Re: [PATCH] mm/damon: update expired description of damos_action

[Honggyu Kim <honggyu.kim@sk.com>]

On 8/3/2025 2:30 PM, SeongJae Park wrote:
> On Sun, 3 Aug 2025 13:43:03 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
> 
>> Hi SeongJae,
>>
>> On 8/3/2025 1:22 PM, SeongJae Park wrote:
>>> On Sun, 3 Aug 2025 11:03:12 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
>>>
>>>> Hi SeongJae and Sang-Heon,
>>>>
>>>> On 8/2/2025 1:50 AM, SeongJae Park wrote:
>>>>> On Sat, 2 Aug 2025 01:11:09 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>>>>>
>>>>>> Hi, Honggyu
>>>>>>
>>>>>> On Fri, Aug 1, 2025 at 8:35 PM Honggyu Kim <honggyu.kim@sk.com> wrote:
>>>>>>>
>>>>>>> Hi Sang-Heon and SeongJae,
>>>>>>>
>>>>>>> On 8/1/2025 2:58 AM, SeongJae Park wrote:
>>>>>>>> Hello Sang-Heon,
>>>>>>>>
>>>>>>>> On Thu, 31 Jul 2025 22:22:30 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>>>>>>>>
>>>>>>>>> Nowadays, damos operation actions support more various operation set.
>>>>>>>>> But comments(also, generated documentation) doesn't updated.
>>>>>>>>> So, fix the comments with current support status.
>>>>> [...]
>>>>>>>>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>>>>> [...]
>>>>>>>>>       * @DAMOS_WILLNEED:  Call ``madvise()`` for the region with MADV_WILLNEED.
>>>>>>>>>       * @DAMOS_COLD:              Call ``madvise()`` for the region with MADV_COLD.
>>>>>>>>> - * @DAMOS_PAGEOUT:   Call ``madvise()`` for the region with MADV_PAGEOUT.
>>>>>>>>> + * @DAMOS_PAGEOUT:   Reclaim the region.
>>>>>>>>
>>>>>>>> Nice!
>>>>>>>
>>>>>>> But doesn't it make confusion about whether this pages out to disk or does
>>>>>>> demotion to the lower tier memory?  It's because PAGEOUT action doesn't do
>>>>>>> demotion, but it looks "reclaim" includes pageout and demotion together in my
>>>>>>> understanding since /sys/kernel/mm/numa/demotion_enabled was introduced.
>>>>>
>>>>> To my understanding, DAMOS_PAGEOUT can also do demotion when demotion_enabled
>>>>> is set.  Am I missing something?
>>>>
>>>> Actually no, please see below.
>>>
>>> I'm unsure to what point you are saying "no".  Are you saying DAMOS_PAGEOUT can
>>> also do demotion when demotion_enabled is set?  Or not?  Could you please
>>> clarify, and add more explanations about why you think so?
>>
>> I checked it again and found I pointed out in the wrong place. Please see below.
>>
>>>
>>>>
>>>> do_demote_pass in shrink_folio_list()
>>>> https://github.com/torvalds/linux/blob/v6.16/mm/vmscan.c#L1122
>>>>
>>>> The do_demote_pass is used here.
>>>> https://github.com/torvalds/linux/blob/v6.16/mm/vmscan.c#L1293-L1302
>>>>
>>>> can_demote() implementation returns false when demotion_enabled is on.
>>>> https://github.com/torvalds/linux/blob/v6.16/mm/vmscan.c#L350-L351
>>>
>>> I'm again get confused.  Isn't it opposite?
>>
>> The thing is that DAMOS_PAGEOUT call sequence is as follows.
>>
>>     DAMOS_PAGEOUT
>>     -> damon_pa_pageout
>>     -> reclaim_pages
>>     -> reclaim_folio_list
>>
>> In reclaim_folio_list(), it sets "no_demotion = 1" in scan_control, then invokes
>> shrink_folio_list().
> 
> Thank you, this clarifies.  DAMOS_PAGEOUT doesn't demote pages even if
> demotion_enabled is set.  Thank you for enlightening me.

Thank you too. Glad to hear that.

> 
> So, "reclaim" means "reclaim".  shrink_folio_list() can do demotions when
> demotion_enabled is set.  I hence still don't think this patch is saying
> something very wrong, and how it could be improved.  Do you have more specific
> change suggestions for this patch for an improvment?

I would just like to make the term "reclaim" clearer and we may be able to 
define what "reclaim" is.  I think we can choose between the following two
different definitions.

Definition 1. "reclaim" includes "pageout" and "demotion".
In this case, we better clarify all the other documents that mentions about
those terms.

Definition 2. "reclaim" only includes "pageout", but "demotion" is out of scope.
In this case, shrink_folio_list just do pageout, but "demotion" is only
exceptional case so we can say the "demotion" escapes from "reclaim" logic.

We might have to clarify the term "reclaim" for those cases whether it includes 
"demotion" or not.  We might have to discuss with other mm developers together.

Thanks,
Honggyu