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 |
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.
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 --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;