On 2023/3/3 0:47, SeongJae Park wrote:
Hi Kefeng,
On Thu, 2 Mar 2023 22:49:26 +0800 Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:
Omit two lines by converting if(!folio_isolate_lru()) to
if(folio_isolate_lru()).
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
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
