diff mbox series

mm: reduced code declaration

Message ID 1608287569-4689-1-git-send-email-yanghui.def@gmail.com (mailing list archive)
State New, archived
Headers show
Series mm: reduced code declaration | expand

Commit Message

hui yang Dec. 18, 2020, 10:32 a.m. UTC
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(-)

Comments

Andrew Morton Dec. 18, 2020, 6:02 p.m. UTC | #1
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.
Chris Down Dec. 19, 2020, 1:14 a.m. UTC | #2
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
>
>
diff mbox series

Patch

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;