* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).