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