[PATCH] mm: reduced code declaration

[PATCH] mm: reduced code declaration

[hui yang]

From: YangHui <yanghui.def@gmail.com>

make code more formal.

Signed-off-by: YangHui <yanghui.def@gmail.com>
---
 mm/vmscan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1b8f0e0..1fea0b4 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2425,7 +2425,7 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
 {
 	unsigned long nr[NR_LRU_LISTS];
 	unsigned long targets[NR_LRU_LISTS];
-	unsigned long nr_to_scan;
+	unsigned long nr_to_scan, scan_target;
 	enum lru_list lru;
 	unsigned long nr_reclaimed = 0;
 	unsigned long nr_to_reclaim = sc->nr_to_reclaim;
@@ -2492,12 +2492,12 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
 			break;
 
 		if (nr_file > nr_anon) {
-			unsigned long scan_target = targets[LRU_INACTIVE_ANON] +
+			scan_target = targets[LRU_INACTIVE_ANON] +
 						targets[LRU_ACTIVE_ANON] + 1;
 			lru = LRU_BASE;
 			percentage = nr_anon * 100 / scan_target;
 		} else {
-			unsigned long scan_target = targets[LRU_INACTIVE_FILE] +
+			scan_target = targets[LRU_INACTIVE_FILE] +
 						targets[LRU_ACTIVE_FILE] + 1;
 			lru = LRU_FILE;
 			percentage = nr_file * 100 / scan_target;
-- 
2.7.4

Re: [PATCH] mm: reduced code declaration

[Andrew Morton]

On Fri, 18 Dec 2020 18:32:49 +0800 hui yang <yanghui.def@gmail.com> wrote:

> From: YangHui <yanghui.def@gmail.com>
> 
> make code more formal.
> 
> ...
>
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2425,7 +2425,7 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>  {
>  	unsigned long nr[NR_LRU_LISTS];
>  	unsigned long targets[NR_LRU_LISTS];
> -	unsigned long nr_to_scan;
> +	unsigned long nr_to_scan, scan_target;
>  	enum lru_list lru;
>  	unsigned long nr_reclaimed = 0;
>  	unsigned long nr_to_reclaim = sc->nr_to_reclaim;
> @@ -2492,12 +2492,12 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>  			break;
>  
>  		if (nr_file > nr_anon) {
> -			unsigned long scan_target = targets[LRU_INACTIVE_ANON] +
> +			scan_target = targets[LRU_INACTIVE_ANON] +
>  						targets[LRU_ACTIVE_ANON] + 1;
>  			lru = LRU_BASE;
>  			percentage = nr_anon * 100 / scan_target;
>  		} else {
> -			unsigned long scan_target = targets[LRU_INACTIVE_FILE] +
> +			scan_target = targets[LRU_INACTIVE_FILE] +
>  						targets[LRU_ACTIVE_FILE] + 1;
>  			lru = LRU_FILE;
>  			percentage = nr_file * 100 / scan_target;

I tend to prefer the original code - this change gives scan_target
function-wide scope, which it does not need.  Makes it harder for the
reader to understand where and why this variable is used.

Re: [PATCH] mm: reduced code declaration

[Chris Down]

Hi Hui,

hui yang writes:
>From: YangHui <yanghui.def@gmail.com>
>
>make code more formal.
>
>Signed-off-by: YangHui <yanghui.def@gmail.com>

For future changelogs: please include a concrete description of the problem 
you're trying to solve. "Formal" is meaningless here.

Just to add my agreement with Andrew, in my opinion this makes the code worse 
rather than better. It increases the scope of scan_target, which seems neutral 
to negative, and (if merged) would occlude `git blame` unnecessarily.

>---
> mm/vmscan.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/mm/vmscan.c b/mm/vmscan.c
>index 1b8f0e0..1fea0b4 100644
>--- a/mm/vmscan.c
>+++ b/mm/vmscan.c
>@@ -2425,7 +2425,7 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
> {
> 	unsigned long nr[NR_LRU_LISTS];
> 	unsigned long targets[NR_LRU_LISTS];
>-	unsigned long nr_to_scan;
>+	unsigned long nr_to_scan, scan_target;
> 	enum lru_list lru;
> 	unsigned long nr_reclaimed = 0;
> 	unsigned long nr_to_reclaim = sc->nr_to_reclaim;
>@@ -2492,12 +2492,12 @@ static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
> 			break;
>
> 		if (nr_file > nr_anon) {
>-			unsigned long scan_target = targets[LRU_INACTIVE_ANON] +
>+			scan_target = targets[LRU_INACTIVE_ANON] +
> 						targets[LRU_ACTIVE_ANON] + 1;
> 			lru = LRU_BASE;
> 			percentage = nr_anon * 100 / scan_target;
> 		} else {
>-			unsigned long scan_target = targets[LRU_INACTIVE_FILE] +
>+			scan_target = targets[LRU_INACTIVE_FILE] +
> 						targets[LRU_ACTIVE_FILE] + 1;
> 			lru = LRU_FILE;
> 			percentage = nr_file * 100 / scan_target;
>-- 
>2.7.4
>
>