* [PATCH -next] mm/damon/paddr: minor refactor of damon_pa_pageout() @ 2023-03-02 14:49 Kefeng Wang 2023-03-02 16:47 ` SeongJae Park 0 siblings, 1 reply; 3+ messages in thread From: Kefeng Wang @ 2023-03-02 14:49 UTC (permalink / raw) To: SeongJae Park, Andrew Morton; +Cc: linux-mm, linux-kernel, damon, Kefeng Wang Omit two lines by converting if(!folio_isolate_lru()) to if(folio_isolate_lru()). Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/damon/paddr.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c index 6c655d9b5639..a557f3c9300f 100644 --- a/mm/damon/paddr.c +++ b/mm/damon/paddr.c @@ -246,14 +246,12 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s) folio_clear_referenced(folio); folio_test_clear_young(folio); - if (!folio_isolate_lru(folio)) { - folio_put(folio); - continue; + if (folio_isolate_lru(folio)) { + if (folio_test_unevictable(folio)) + folio_putback_lru(folio); + else + list_add(&folio->lru, &folio_list); } - if (folio_test_unevictable(folio)) - folio_putback_lru(folio); - else - list_add(&folio->lru, &folio_list); folio_put(folio); } applied = reclaim_pages(&folio_list); -- 2.35.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -next] mm/damon/paddr: minor refactor of damon_pa_pageout() 2023-03-02 14:49 [PATCH -next] mm/damon/paddr: minor refactor of damon_pa_pageout() Kefeng Wang @ 2023-03-02 16:47 ` SeongJae Park 2023-03-03 1:42 ` Kefeng Wang 0 siblings, 1 reply; 3+ messages in thread From: SeongJae Park @ 2023-03-02 16:47 UTC (permalink / raw) To: Kefeng Wang; +Cc: SeongJae Park, Andrew Morton, linux-mm, linux-kernel, damon Hi Kefeng, On Thu, 2 Mar 2023 22:49:26 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > Omit two lines by converting if(!folio_isolate_lru()) to > if(folio_isolate_lru()). > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > mm/damon/paddr.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c > index 6c655d9b5639..a557f3c9300f 100644 > --- a/mm/damon/paddr.c > +++ b/mm/damon/paddr.c > @@ -246,14 +246,12 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s) > > folio_clear_referenced(folio); > folio_test_clear_young(folio); > - if (!folio_isolate_lru(folio)) { > - folio_put(folio); > - continue; > + if (folio_isolate_lru(folio)) { > + if (folio_test_unevictable(folio)) > + folio_putback_lru(folio); > + else > + list_add(&folio->lru, &folio_list); > } > - if (folio_test_unevictable(folio)) > - folio_putback_lru(folio); > - else > - list_add(&folio->lru, &folio_list); > folio_put(folio); > } This looks ok to me, thank you for your effort! Nevertheless, I don't like increasing depth as much as number of lines. The more-than-3-levels-of-indentation[1] warning is not always right, but this 4 levels of indentation is not making me so exciting. What do you think about adding put_continue label and using it, not only here, but also above part? For example: --- a/mm/damon/paddr.c +++ b/mm/damon/paddr.c @@ -239,21 +239,18 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s) if (!folio) continue; - if (damos_pa_filter_out(s, folio)) { - folio_put(folio); - continue; - } + if (damos_pa_filter_out(s, folio)) + goto put_continue; folio_clear_referenced(folio); folio_test_clear_young(folio); - if (!folio_isolate_lru(folio)) { - folio_put(folio); - continue; - } + if (!folio_isolate_lru(folio)) + goto put_continue; if (folio_test_unevictable(folio)) folio_putback_lru(folio); else list_add(&folio->lru, &folio_list); +put_continue: folio_put(folio); } applied = reclaim_pages(&folio_list); It omits three lines. $ git diff --stat mm/damon/paddr.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) [1] https://docs.kernel.org/process/coding-style.html#indentation Thanks, SJ > applied = reclaim_pages(&folio_list); > -- > 2.35.3 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -next] mm/damon/paddr: minor refactor of damon_pa_pageout() 2023-03-02 16:47 ` SeongJae Park @ 2023-03-03 1:42 ` Kefeng Wang 0 siblings, 0 replies; 3+ messages in thread From: Kefeng Wang @ 2023-03-03 1:42 UTC (permalink / raw) To: SeongJae Park; +Cc: Andrew Morton, linux-mm, linux-kernel, damon On 2023/3/3 0:47, SeongJae Park wrote: > Hi Kefeng, > > On Thu, 2 Mar 2023 22:49:26 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > >> Omit two lines by converting if(!folio_isolate_lru()) to >> if(folio_isolate_lru()). >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> --- >> mm/damon/paddr.c | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c >> index 6c655d9b5639..a557f3c9300f 100644 >> --- a/mm/damon/paddr.c >> +++ b/mm/damon/paddr.c >> @@ -246,14 +246,12 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s) >> >> folio_clear_referenced(folio); >> folio_test_clear_young(folio); >> - if (!folio_isolate_lru(folio)) { >> - folio_put(folio); >> - continue; >> + if (folio_isolate_lru(folio)) { >> + if (folio_test_unevictable(folio)) >> + folio_putback_lru(folio); >> + else >> + list_add(&folio->lru, &folio_list); >> } >> - if (folio_test_unevictable(folio)) >> - folio_putback_lru(folio); >> - else >> - list_add(&folio->lru, &folio_list); >> folio_put(folio); >> } > > This looks ok to me, thank you for your effort! Nevertheless, I don't like > increasing depth as much as number of lines. The > more-than-3-levels-of-indentation[1] warning is not always right, but this 4 > levels of indentation is not making me so exciting. > > What do you think about adding put_continue label and using it, not only here, > but also above part? For example: > > --- a/mm/damon/paddr.c > +++ b/mm/damon/paddr.c > @@ -239,21 +239,18 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s) > if (!folio) > continue; > > - if (damos_pa_filter_out(s, folio)) { > - folio_put(folio); > - continue; > - } > + if (damos_pa_filter_out(s, folio)) > + goto put_continue; > > folio_clear_referenced(folio); > folio_test_clear_young(folio); > - if (!folio_isolate_lru(folio)) { > - folio_put(folio); > - continue; > - } > + if (!folio_isolate_lru(folio)) > + goto put_continue; > if (folio_test_unevictable(folio)) > folio_putback_lru(folio); > else > list_add(&folio->lru, &folio_list); > +put_continue: > folio_put(folio); > } > applied = reclaim_pages(&folio_list); > > It omits three lines. It looks good, will update, thanks > > $ git diff --stat > mm/damon/paddr.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > [1] https://docs.kernel.org/process/coding-style.html#indentation > > > Thanks, > SJ > >> applied = reclaim_pages(&folio_list); >> -- >> 2.35.3 >> >> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-03 2:00 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-02 14:49 [PATCH -next] mm/damon/paddr: minor refactor of damon_pa_pageout() Kefeng Wang 2023-03-02 16:47 ` SeongJae Park 2023-03-03 1:42 ` Kefeng Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.