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

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

[Sang-Heon Jeon]

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.

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
So, Is it means that DAMOS_STAT is only supported by paddr now?
If my understanding is correct, then we need to fix Design documentation
and comments both.

However, since it will be fixed soon. Keeping current status is also
affordable. So could you check this point as well? 

---

 include/linux/damon.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index cddf82eaac4e..0870e9f48b0f 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -128,7 +128,7 @@ struct damon_access_report {
  *
  * @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.
  * @DAMOS_HUGEPAGE:	Call ``madvise()`` for the region with MADV_HUGEPAGE.
  * @DAMOS_NOHUGEPAGE:	Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
  * @DAMOS_LRU_PRIO:	Prioritize the region on its LRU lists.
@@ -146,8 +146,8 @@ struct damon_access_report {
  * The support of each action is up to running &struct damon_operations.
  * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
  * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
- * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
- * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
+ * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
+ * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.
  */
 enum damos_action {
 	DAMOS_WILLNEED,
-- 
2.43.0

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

[SeongJae Park]

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.

Thank you for catching this and fixing!

> 
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> So, Is it means that DAMOS_STAT is only supported by paddr now?

No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
on vaddr, but extending the support for DAMOS filters.

Thank you for checking the thread and asking this question, though.  I see this
as an important signal of another rooms to improve on the documentation.

> If my understanding is correct, then we need to fix Design documentation
> and comments both.
> 
> However, since it will be fixed soon. Keeping current status is also
> affordable. So could you check this point as well? 
> 
> ---
> 
>  include/linux/damon.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index cddf82eaac4e..0870e9f48b0f 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -128,7 +128,7 @@ struct damon_access_report {

Seems you are using damon/next as your baseline.  If there is no real
dependencies to damon/next, please use mm-new as your baseline.  For more
contexts about that, please refer to the DAMON maintainer-profile[1].

>   *
>   * @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!

>   * @DAMOS_HUGEPAGE:	Call ``madvise()`` for the region with MADV_HUGEPAGE.
>   * @DAMOS_NOHUGEPAGE:	Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
>   * @DAMOS_LRU_PRIO:	Prioritize the region on its LRU lists.
> @@ -146,8 +146,8 @@ struct damon_access_report {
>   * The support of each action is up to running &struct damon_operations.
>   * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
>   * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
> - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
> - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> + * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
> + * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.

Thank you for updating this!

But, now I find this place is having outdated information that partially
duplicated with the DAMON design document[2].  I think the design document is
better to maintain this kind of details.  What about keeping the high level
information but giving a reference to the documentation for details, for
example, like below?

    @@ -143,11 +143,9 @@ struct damon_access_report {
      * @DAMOS_STAT:                Do nothing but count the stat.
      * @NR_DAMOS_ACTIONS:  Total number of DAMOS actions
      *
    - * The support of each action is up to running &struct damon_operations.
    - * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
    - * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
    - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
    - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
    + * The support of each action is up to running &struct damon_operations.  Refer
    + * to 'Operation Action' section of Documentation/mm/damon/design.rst for
    + * status of the supports.
      */

Thanks,
SJ


[1] https://origin.kernel.org/doc/html/next/mm/damon/maintainer-profile.html#scm-trees
[2] https://origin.kernel.org/doc/html/next/mm/damon/design.html#operation-action

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

[Honggyu Kim]

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.
> 
> Thank you for catching this and fixing!
> 
>>
>> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>> ---
>> I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
>> So, Is it means that DAMOS_STAT is only supported by paddr now?
> 
> No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
> on vaddr, but extending the support for DAMOS filters.
> 
> Thank you for checking the thread and asking this question, though.  I see this
> as an important signal of another rooms to improve on the documentation.
> 
>> If my understanding is correct, then we need to fix Design documentation
>> and comments both.
>>
>> However, since it will be fixed soon. Keeping current status is also
>> affordable. So could you check this point as well?
>>
>> ---
>>
>>   include/linux/damon.h | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index cddf82eaac4e..0870e9f48b0f 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -128,7 +128,7 @@ struct damon_access_report {
> 
> Seems you are using damon/next as your baseline.  If there is no real
> dependencies to damon/next, please use mm-new as your baseline.  For more
> contexts about that, please refer to the DAMON maintainer-profile[1].
> 
>>    *
>>    * @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.

Thanks,
Honggyu

>>    * @DAMOS_HUGEPAGE:  Call ``madvise()`` for the region with MADV_HUGEPAGE.
>>    * @DAMOS_NOHUGEPAGE:        Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
>>    * @DAMOS_LRU_PRIO:  Prioritize the region on its LRU lists.
>> @@ -146,8 +146,8 @@ struct damon_access_report {
>>    * The support of each action is up to running &struct damon_operations.
>>    * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
>>    * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
>> - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
>> - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
>> + * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
>> + * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.
> 
> Thank you for updating this!
> 
> But, now I find this place is having outdated information that partially
> duplicated with the DAMON design document[2].  I think the design document is
> better to maintain this kind of details.  What about keeping the high level
> information but giving a reference to the documentation for details, for
> example, like below?
> 
>      @@ -143,11 +143,9 @@ struct damon_access_report {
>        * @DAMOS_STAT:                Do nothing but count the stat.
>        * @NR_DAMOS_ACTIONS:  Total number of DAMOS actions
>        *
>      - * The support of each action is up to running &struct damon_operations.
>      - * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
>      - * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
>      - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
>      - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
>      + * The support of each action is up to running &struct damon_operations.  Refer
>      + * to 'Operation Action' section of Documentation/mm/damon/design.rst for
>      + * status of the supports.
>        */
> 
> Thanks,
> SJ
> 
> 
> [1] https://origin.kernel.org/doc/html/next/mm/damon/maintainer-profile.html#scm-trees
> [2] https://origin.kernel.org/doc/html/next/mm/damon/design.html#operation-action

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

[Sang-Heon Jeon]

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.
> >
> > Thank you for catching this and fixing!
> >
> >>
> >> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> >> ---
> >> I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> >> So, Is it means that DAMOS_STAT is only supported by paddr now?
> >
> > No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
> > on vaddr, but extending the support for DAMOS filters.
> >
> > Thank you for checking the thread and asking this question, though.  I see this
> > as an important signal of another rooms to improve on the documentation.
> >
> >> If my understanding is correct, then we need to fix Design documentation
> >> and comments both.
> >>
> >> However, since it will be fixed soon. Keeping current status is also
> >> affordable. So could you check this point as well?
> >>
> >> ---
> >>
> >>   include/linux/damon.h | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/damon.h b/include/linux/damon.h
> >> index cddf82eaac4e..0870e9f48b0f 100644
> >> --- a/include/linux/damon.h
> >> +++ b/include/linux/damon.h
> >> @@ -128,7 +128,7 @@ struct damon_access_report {
> >
> > Seems you are using damon/next as your baseline.  If there is no real
> > dependencies to damon/next, please use mm-new as your baseline.  For more
> > contexts about that, please refer to the DAMON maintainer-profile[1].
> >
> >>    *
> >>    * @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.

My intention was just to synchronize with the Design documentation.

So how about changing the description to `Page out the region`, Would
this be also confusing?
I feel like it would be clearer than using word "reclaim"

> Thanks,
> Honggyu
>
> >>    * @DAMOS_HUGEPAGE:  Call ``madvise()`` for the region with MADV_HUGEPAGE.
> >>    * @DAMOS_NOHUGEPAGE:        Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
> >>    * @DAMOS_LRU_PRIO:  Prioritize the region on its LRU lists.
> >> @@ -146,8 +146,8 @@ struct damon_access_report {
> >>    * The support of each action is up to running &struct damon_operations.
> >>    * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
> >>    * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
> >> - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
> >> - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> >> + * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
> >> + * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.
> >
> > Thank you for updating this!
> >
> > But, now I find this place is having outdated information that partially
> > duplicated with the DAMON design document[2].  I think the design document is
> > better to maintain this kind of details.  What about keeping the high level
> > information but giving a reference to the documentation for details, for
> > example, like below?
> >
> >      @@ -143,11 +143,9 @@ struct damon_access_report {
> >        * @DAMOS_STAT:                Do nothing but count the stat.
> >        * @NR_DAMOS_ACTIONS:  Total number of DAMOS actions
> >        *
> >      - * The support of each action is up to running &struct damon_operations.
> >      - * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
> >      - * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
> >      - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
> >      - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> >      + * The support of each action is up to running &struct damon_operations.  Refer
> >      + * to 'Operation Action' section of Documentation/mm/damon/design.rst for
> >      + * status of the supports.
> >        */
> >
> > Thanks,
> > SJ
> >
> >
> > [1] https://origin.kernel.org/doc/html/next/mm/damon/maintainer-profile.html#scm-trees
> > [2] https://origin.kernel.org/doc/html/next/mm/damon/design.html#operation-action
>

Best Regards.
Sang-Heon Jeon

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

[SeongJae Park]

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?

> 
> My intention was just to synchronize with the Design documentation.
> 
> So how about changing the description to `Page out the region`, Would
> this be also confusing?
> I feel like it would be clearer than using word "reclaim"

In my opinion, "reclaim" is good.


Thanks,
SJ

[...]

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

[Honggyu Kim]

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.

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

The replated commit is as follows.
mm/migrate: add sysfs interface to enable reclaim migration
https://github.com/torvalds/linux/commit/20b51af15e014cac63b58a4f8b8b323ac35bccce

> 
>>
>> My intention was just to synchronize with the Design documentation.
>>
>> So how about changing the description to `Page out the region`, Would
>> this be also confusing?
>> I feel like it would be clearer than using word "reclaim"

I don't have a good idea but it looks like recusive explanation.

> 
> In my opinion, "reclaim" is good.

I wish there could be better term that distinguishes between swap out and
demotion.  In my understanding "reclaim" includes both swap out and demotion.

I also found that man page explanation about MADV_PAGEOUT is "reclaim these 
pages".  If this is correct, then maybe demotion isn't included in "reclaim".

Thanks,
Honggyu

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

[SeongJae Park]

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?

> 
> 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?

```
static bool can_demote(int nid, struct scan_control *sc,
		       struct mem_cgroup *memcg)
{
	int demotion_nid;

	if (!numa_demotion_enabled)
		return false;
```

It returns "false" when demotion_enabled is "off" (unset).  Am I reading
something wrong...?

> 
> The replated commit is as follows.
> mm/migrate: add sysfs interface to enable reclaim migration
> https://github.com/torvalds/linux/commit/20b51af15e014cac63b58a4f8b8b323ac35bccce
> 
> > 
> >>
> >> My intention was just to synchronize with the Design documentation.
> >>
> >> So how about changing the description to `Page out the region`, Would
> >> this be also confusing?
> >> I feel like it would be clearer than using word "reclaim"
> 
> I don't have a good idea but it looks like recusive explanation.
> 
> > 
> > In my opinion, "reclaim" is good.
> 
> I wish there could be better term that distinguishes between swap out and
> demotion.  In my understanding "reclaim" includes both swap out and demotion.

"Reclaim" also includes writeback operations.

I agree we have many rooms to improve in terms of terminologies.  But, I'd
argue we don't need to have only 1:1 mapping terminologies.  Othrwise, maybe we
don't need any documentation at all but just code.  "Reclaim" is a good general
terminology for describing an effort to get free pages on a memory domain (NUMA
node, zone, etc), in my opinion.

To be honest, btw, I'm not a fan of "promote/demote", and that was one of the
reasons I insisted "DAMOS_MIGRATE_{HOT,COLD}" instead of
"DAMOS_{PROMOTE,DEMOTE}".

> 
> I also found that man page explanation about MADV_PAGEOUT is "reclaim these 
> pages".  If this is correct, then maybe demotion isn't included in "reclaim".

I interpret the term "reclaim" on the man page with a flexibility including my
above definition, and hence I don't think this is contradicting in a very bad
way against what I'm understanding.  That is, I interpret the documentation
says MADV_PAGEOUT can also do demote pages under certain conditions.

I didn't write the documentation, so I may be completely wrong.  I also
frustratingly lost my resource to validate the main question of this discussion
(whether DAMOS_PAGEOUT can also do demotion or not), for now.  I'd like to
continue this discussion based on code rather than a documentation that _might_
be right or wrong, and preferrably based on real tests if you or I have a good
testing setup.


Thanks,
SJ

[...]

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

[Honggyu Kim]

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().
https://github.com/torvalds/linux/blob/v6.16/mm/vmscan.c#L2237

Inside shrink_folio_list(), it calls can_demote() and it returns false even if 
demotion_enabled is true.
https://github.com/torvalds/linux/blob/v6.16/mm/vmscan.c#L352-L353

> 
> ```
> static bool can_demote(int nid, struct scan_control *sc,
>                         struct mem_cgroup *memcg)
> {
>          int demotion_nid;
> 
>          if (!numa_demotion_enabled)
>                  return false;
> ```
> 
> It returns "false" when demotion_enabled is "off" (unset).  Am I reading
> something wrong...?

The full implementation of can_demote is as follows.  It checks whether
"no_demotion" is set.

static bool can_demote(int nid, struct scan_control *sc,
		       struct mem_cgroup *memcg)
{
	int demotion_nid;

	if (!numa_demotion_enabled)
		return false;
	if (sc && sc->no_demotion)
		return false;

	demotion_nid = next_demotion_node(nid);
	if (demotion_nid == NUMA_NO_NODE)
		return false;

	/* If demotion node isn't in the cgroup's mems_allowed, fall back */
	return mem_cgroup_node_allowed(memcg, demotion_nid);
}

Hope this is helpful.

> 
>>
>> The replated commit is as follows.
>> mm/migrate: add sysfs interface to enable reclaim migration
>> https://github.com/torvalds/linux/commit/20b51af15e014cac63b58a4f8b8b323ac35bccce
>>
>>>
>>>>
>>>> My intention was just to synchronize with the Design documentation.
>>>>
>>>> So how about changing the description to `Page out the region`, Would
>>>> this be also confusing?
>>>> I feel like it would be clearer than using word "reclaim"
>>
>> I don't have a good idea but it looks like recusive explanation.
>>
>>>
>>> In my opinion, "reclaim" is good.
>>
>> I wish there could be better term that distinguishes between swap out and
>> demotion.  In my understanding "reclaim" includes both swap out and demotion.
> 
> "Reclaim" also includes writeback operations.
> 
> I agree we have many rooms to improve in terms of terminologies.  But, I'd
> argue we don't need to have only 1:1 mapping terminologies.  Othrwise, maybe we
> don't need any documentation at all but just code.  "Reclaim" is a good general
> terminology for describing an effort to get free pages on a memory domain (NUMA
> node, zone, etc), in my opinion.
> 
> To be honest, btw, I'm not a fan of "promote/demote", and that was one of the
> reasons I insisted "DAMOS_MIGRATE_{HOT,COLD}" instead of
> "DAMOS_{PROMOTE,DEMOTE}".

I do agree, I feel the current memory tiering system doesn't represent the
tiered memory topology so promote/demote might not be good terms for now.

> 
>>
>> I also found that man page explanation about MADV_PAGEOUT is "reclaim these
>> pages".  If this is correct, then maybe demotion isn't included in "reclaim".
> 
> I interpret the term "reclaim" on the man page with a flexibility including my
> above definition, and hence I don't think this is contradicting in a very bad
> way against what I'm understanding.  That is, I interpret the documentation
> says MADV_PAGEOUT can also do demote pages under certain conditions.
> 
> I didn't write the documentation, so I may be completely wrong.  I also
> frustratingly lost my resource to validate the main question of this discussion
> (whether DAMOS_PAGEOUT can also do demotion or not), for now.  I'd like to
> continue this discussion based on code rather than a documentation that _might_
> be right or wrong, and preferrably based on real tests if you or I have a good
> testing setup.

The code I showed above explains well enough about this.

Thanks,
Honggyu

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

[SeongJae Park]

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.

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?


Thanks,
SJ

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

[Honggyu Kim]

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

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

[Sang-Heon Jeon]

Hi, SeongJae and Honggyu,

On Sun, Aug 3, 2025 at 2:41 PM Honggyu Kim <honggyu.kim@sk.com> wrote:
>
>
>
> 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

Because of the above thread, I got to know the details more clearly.
Thank you guys!
When the discussion of "reclaim" finishes, I'll make v2 patch as soon
as possible.

However, I want to talk about a slightly different topic. How about
adding support demotion to DAMOS operation action?
Maybe we can add another action type or change implementation of DAMOS_PAGEOUT.

IMHO, I think we should first check whether it's possible to set
no_demotion in the `madvise` -> `foilo` flow we're using.
Since I'm still quite new to these things, I'd like to check whether
my idea and direction are correct.

I can't thank you all enough for your kindness :)

Best Regards.
Sang-Heon Jeon

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

[SeongJae Park]

On Sun, 3 Aug 2025 22:22:05 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> Hi, SeongJae and Honggyu,
> 
> On Sun, Aug 3, 2025 at 2:41 PM Honggyu Kim <honggyu.kim@sk.com> wrote:
> >
> >
> >
> > 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.

I was thinking in the first way, and apparently I was simply wrong.  Now I
think the second one is more correct, at least for {DAMOS,MADV}_PAGEOUT.

Yet another way to do the reclaim would be memory.reclaim cgroup file.  Seems
in the case, demotion can happen since user_proactive_reclaim() doesn't set
scan_control->no_demotion.

I didn't read the code, but apparently memory pressure-based reactive
traditional reclaim logic would also do demotion?

So, apparently "reclaim" can mean at least the two definitions.

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

I think that could be a nice discussion.  Looking forward to.  As I previously
mentioned, I don't think we need to have only 1:1 mapping terminologies, though
I have no strong opinion here.

Regardless of terms, I agree there are many rooms to improve on our
documentation.  At least DAMOS_PAGEOUT documentation may better to clarify what
you pointed out.

> > Thanks,
> > Honggyu
> 
> Because of the above thread, I got to know the details more clearly.
> Thank you guys!
> When the discussion of "reclaim" finishes, I'll make v2 patch as soon
> as possible.

I don't think we need to block v2 of this patch for the discussion.  What about
adding a note about the demotion behavior on the details comment second, e.g.,

--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -150,6 +150,8 @@ struct damon_access_report {
  * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
  * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
  * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
+ *
+ * Note that DAMOS_PAGEOUT doesn't trigger demotions.
  */
 enum damos_action {
        DAMOS_WILLNEED,

> 
> However, I want to talk about a slightly different topic. How about
> adding support demotion to DAMOS operation action?
> Maybe we can add another action type or change implementation of DAMOS_PAGEOUT.

We actually tried to implement DAMOS_DEMOTE, but eventually decided[1] to have
a more general action called DAMOS_MIGRATE_COLD.

Do you think there are use cases where DAMOS_MIGRATE_COLD cannot cover?

> 
> IMHO, I think we should first check whether it's possible to set
> no_demotion in the `madvise` -> `foilo` flow we're using.
> Since I'm still quite new to these things, I'd like to check whether
> my idea and direction are correct.
> 
> I can't thank you all enough for your kindness :)

The pleasure is mine :)

> 
> Best Regards.
> Sang-Heon Jeon
> 

[1] https://lore.kernel.org/all/20240614030010.751-1-honggyu.kim@sk.com/


Thanks,
SJ

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

[Sang-Heon Jeon]

On Mon, Aug 4, 2025 at 2:42 AM SeongJae Park <sj@kernel.org> wrote:
>
> On Sun, 3 Aug 2025 22:22:05 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> > Hi, SeongJae and Honggyu,
> >
> > On Sun, Aug 3, 2025 at 2:41 PM Honggyu Kim <honggyu.kim@sk.com> wrote:
> > >
> > >
> > >
> > > 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.
>
> I was thinking in the first way, and apparently I was simply wrong.  Now I
> think the second one is more correct, at least for {DAMOS,MADV}_PAGEOUT.
>
> Yet another way to do the reclaim would be memory.reclaim cgroup file.  Seems
> in the case, demotion can happen since user_proactive_reclaim() doesn't set
> scan_control->no_demotion.
>
> I didn't read the code, but apparently memory pressure-based reactive
> traditional reclaim logic would also do demotion?
>
> So, apparently "reclaim" can mean at least the two definitions.
>
> > >
> > > 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.
>
> I think that could be a nice discussion.  Looking forward to.  As I previously
> mentioned, I don't think we need to have only 1:1 mapping terminologies, though
> I have no strong opinion here.
>
> Regardless of terms, I agree there are many rooms to improve on our
> documentation.  At least DAMOS_PAGEOUT documentation may better to clarify what
> you pointed out.
>
> > > Thanks,
> > > Honggyu
> >
> > Because of the above thread, I got to know the details more clearly.
> > Thank you guys!
> > When the discussion of "reclaim" finishes, I'll make v2 patch as soon
> > as possible.
>
> I don't think we need to block v2 of this patch for the discussion.  What about
> adding a note about the demotion behavior on the details comment second, e.g.,

I see. That sounds reasonable because we can revisit after the
discussion finishes.

> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -150,6 +150,8 @@ struct damon_access_report {
>   * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
>   * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
>   * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> + *
> + * Note that DAMOS_PAGEOUT doesn't trigger demotions.
>   */
>  enum damos_action {
>         DAMOS_WILLNEED,
>
> >
> > However, I want to talk about a slightly different topic. How about
> > adding support demotion to DAMOS operation action?
> > Maybe we can add another action type or change implementation of DAMOS_PAGEOUT.
>
> We actually tried to implement DAMOS_DEMOTE, but eventually decided[1] to have
> a more general action called DAMOS_MIGRATE_COLD.
>
> Do you think there are use cases where DAMOS_MIGRATE_COLD cannot cover?

Thanks for your comments. Above suggestion was based on my lack of
knowledge of DAMON.
I tracked linked RFC threads; now I understand that DAMOS_MIGRATE_COLD
supports both demotion or lateral migration, depending on the
environment.

Maybe there could be a new use case, but I think now it's not at the moment.
It's just my bad :( let's just skip that suggestion.

> >
> > IMHO, I think we should first check whether it's possible to set
> > no_demotion in the `madvise` -> `foilo` flow we're using.
> > Since I'm still quite new to these things, I'd like to check whether
> > my idea and direction are correct.
> >
> > I can't thank you all enough for your kindness :)
>
> The pleasure is mine :)
>
> >
> > Best Regards.
> > Sang-Heon Jeon
> >
>
> [1] https://lore.kernel.org/all/20240614030010.751-1-honggyu.kim@sk.com/
>
>
> Thanks,
> SJ

Best Regards.
Sang-Heon Jeon

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

[Honggyu Kim]

Hi SeongJae,

On 8/4/2025 2:42 AM, SeongJae Park wrote:
> On Sun, 3 Aug 2025 22:22:05 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> 
>> Hi, SeongJae and Honggyu,
>>
>> On Sun, Aug 3, 2025 at 2:41 PM Honggyu Kim <honggyu.kim@sk.com> wrote:
>>>
>>>
>>>
>>> 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:

[...snip...]

>>>>> 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.
> 
> I was thinking in the first way, and apparently I was simply wrong.  Now I
> think the second one is more correct, at least for {DAMOS,MADV}_PAGEOUT.
> 
> Yet another way to do the reclaim would be memory.reclaim cgroup file.  Seems
> in the case, demotion can happen since user_proactive_reclaim() doesn't set
> scan_control->no_demotion.

If so, we can't limit the meaning of reclaim into only pageout.

> 
> I didn't read the code, but apparently memory pressure-based reactive
> traditional reclaim logic would also do demotion?

Not sure about "traditional" reclaim logic but demotion logic was introduced not
a long time ago.  The meaning of demotion is meaningful only for tiered memory
system.  I remember tiered memory concept was introduced only about 2+ years
ago.

> 
> So, apparently "reclaim" can mean at least the two definitions.

Yeah, I would like to clarify the definition and make our function names clearer
for better readability.

> 
>>>
>>> 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.
> 
> I think that could be a nice discussion.  Looking forward to.  As I previously
> mentioned, I don't think we need to have only 1:1 mapping terminologies, though
> I have no strong opinion here.
> 
> Regardless of terms, I agree there are many rooms to improve on our
> documentation.  At least DAMOS_PAGEOUT documentation may better to clarify what
> you pointed out.

I don't have strong objection now even if DAMOS_PAGEOUT uses the term "reclaim"
for now.  I think it will take some time so will leave it for now because I
cannot put this into my top priority list due to other works.

> 
>>> Thanks,
>>> Honggyu
>>
>> Because of the above thread, I got to know the details more clearly.
>> Thank you guys!
>> When the discussion of "reclaim" finishes, I'll make v2 patch as soon
>> as possible.
> 
> I don't think we need to block v2 of this patch for the discussion.  What about
> adding a note about the demotion behavior on the details comment second, e.g.,

Agreed.

> 
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -150,6 +150,8 @@ struct damon_access_report {
>    * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
>    * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
>    * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> + *
> + * Note that DAMOS_PAGEOUT doesn't trigger demotions.
>    */
>   enum damos_action {
>          DAMOS_WILLNEED,
> 
>>
>> However, I want to talk about a slightly different topic. How about
>> adding support demotion to DAMOS operation action?
>> Maybe we can add another action type or change implementation of DAMOS_PAGEOUT.
> 
> We actually tried to implement DAMOS_DEMOTE, but eventually decided[1] to have
> a more general action called DAMOS_MIGRATE_COLD.
> 
> Do you think there are use cases where DAMOS_MIGRATE_COLD cannot cover?
> 
>>
>> IMHO, I think we should first check whether it's possible to set
>> no_demotion in the `madvise` -> `foilo` flow we're using.
>> Since I'm still quite new to these things, I'd like to check whether
>> my idea and direction are correct.
>>
>> I can't thank you all enough for your kindness :)
> 
> The pleasure is mine :)

mine too :)

Thanks,
Honggyu

> 
>>
>> Best Regards.
>> Sang-Heon Jeon
>>
> 
> [1] https://lore.kernel.org/all/20240614030010.751-1-honggyu.kim@sk.com/
> 
> 
> Thanks,
> SJ

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

[Sang-Heon Jeon]

Hi, SeongJae

On Fri, Aug 1, 2025 at 2:58 AM SeongJae Park <sj@kernel.org> 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.
>
> Thank you for catching this and fixing!

:)

> >
> > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > ---
> > I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> > So, Is it means that DAMOS_STAT is only supported by paddr now?
>
> No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
> on vaddr, but extending the support for DAMOS filters.
>
> Thank you for checking the thread and asking this question, though.  I see this
> as an important signal of another rooms to improve on the documentation.

Thanks for your kind review. I really don't want to bother you
anymore. But I would like to check my understanding.

So the basic operation of stat is covered by outside of
`damon_pa_apply_scheme` and `damon_va_apply_scheme`.
If it's not too much trouble, could you point me to where they are?

And some extension stat (maybe it only renewed that filter exists) is
covered inside of upper functions. Is it also right point?

> > If my understanding is correct, then we need to fix Design documentation
> > and comments both.
> >
> > However, since it will be fixed soon. Keeping current status is also
> > affordable. So could you check this point as well?
> >
> > ---
> >
> >  include/linux/damon.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index cddf82eaac4e..0870e9f48b0f 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -128,7 +128,7 @@ struct damon_access_report {
>
> Seems you are using damon/next as your baseline.  If there is no real
> dependencies to damon/next, please use mm-new as your baseline.  For more
> contexts about that, please refer to the DAMON maintainer-profile[1].

I see. I'll keep this in my mind. Thank you for telling me!

> >   *
> >   * @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!
>
> >   * @DAMOS_HUGEPAGE:  Call ``madvise()`` for the region with MADV_HUGEPAGE.
> >   * @DAMOS_NOHUGEPAGE:        Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
> >   * @DAMOS_LRU_PRIO:  Prioritize the region on its LRU lists.
> > @@ -146,8 +146,8 @@ struct damon_access_report {
> >   * The support of each action is up to running &struct damon_operations.
> >   * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
> >   * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
> > - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
> > - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> > + * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
> > + * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.
>
> Thank you for updating this!
>
> But, now I find this place is having outdated information that partially
> duplicated with the DAMON design document[2].  I think the design document is
> better to maintain this kind of details.  What about keeping the high level
> information but giving a reference to the documentation for details, for
> example, like below?

I totally agree with you. Unexpected duplication sometimes leads to
unexpected expiration like this one
So, I think that I need to send v2 patch, If that's right, which way
would you prefer?
reply to this thread or make a new thread?

>     @@ -143,11 +143,9 @@ struct damon_access_report {
>       * @DAMOS_STAT:                Do nothing but count the stat.
>       * @NR_DAMOS_ACTIONS:  Total number of DAMOS actions
>       *
>     - * The support of each action is up to running &struct damon_operations.
>     - * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
>     - * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
>     - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
>     - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
>     + * The support of each action is up to running &struct damon_operations.  Refer
>     + * to 'Operation Action' section of Documentation/mm/damon/design.rst for
>     + * status of the supports.
>       */
>
> Thanks,
> SJ
>
>
> [1] https://origin.kernel.org/doc/html/next/mm/damon/maintainer-profile.html#scm-trees
> [2] https://origin.kernel.org/doc/html/next/mm/damon/design.html#operation-action

Best Regards.
Sang-Heon Jeon

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

[SeongJae Park]

On Sat, 2 Aug 2025 00:34:30 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> Hi, SeongJae
> 
> On Fri, Aug 1, 2025 at 2:58 AM SeongJae Park <sj@kernel.org> 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.
> >
> > Thank you for catching this and fixing!
> 
> :)
> 
> > >
> > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > ---
> > > I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> > > So, Is it means that DAMOS_STAT is only supported by paddr now?
> >
> > No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
> > on vaddr, but extending the support for DAMOS filters.
> >
> > Thank you for checking the thread and asking this question, though.  I see this
> > as an important signal of another rooms to improve on the documentation.
> 
> Thanks for your kind review. I really don't want to bother you
> anymore. But I would like to check my understanding.

No worry, questions about DAMON never bother me.

> 
> So the basic operation of stat is covered by outside of
> `damon_pa_apply_scheme` and `damon_va_apply_scheme`.
> If it's not too much trouble, could you point me to where they are?

It is covered by damos_update_stat() in core.c file.

> 
> And some extension stat (maybe it only renewed that filter exists) is
> covered inside of upper functions. Is it also right point?

Depending on what you mean with "upper functions".

damos_apply_scheme() in DAMON core layer calls
damon_operationss->apply_scheme() callback of the current operations set.  The
callback may handle DAMOS filters or not.  The callback of paddr.c handles the
filters for DAMOS_STAT.  The callback of vaddr.c does not, and Pan is extending
it to handle filters.

[...]
> So, I think that I need to send v2 patch, If that's right, which way
> would you prefer?
> reply to this thread or make a new thread?

I prefer making a new thread :)


Thanks,
SJ

[...]

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

[Sang-Heon Jeon]

On Sat, Aug 2, 2025 at 2:02 AM SeongJae Park <sj@kernel.org> wrote:
>
> On Sat, 2 Aug 2025 00:34:30 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> > Hi, SeongJae
> >
> > On Fri, Aug 1, 2025 at 2:58 AM SeongJae Park <sj@kernel.org> 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.
> > >
> > > Thank you for catching this and fixing!
> >
> > :)
> >
> > > >
> > > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > ---
> > > > I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> > > > So, Is it means that DAMOS_STAT is only supported by paddr now?
> > >
> > > No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
> > > on vaddr, but extending the support for DAMOS filters.
> > >
> > > Thank you for checking the thread and asking this question, though.  I see this
> > > as an important signal of another rooms to improve on the documentation.
> >
> > Thanks for your kind review. I really don't want to bother you
> > anymore. But I would like to check my understanding.
>
> No worry, questions about DAMON never bother me.
>
> >
> > So the basic operation of stat is covered by outside of
> > `damon_pa_apply_scheme` and `damon_va_apply_scheme`.
> > If it's not too much trouble, could you point me to where they are?
>
> It is covered by damos_update_stat() in core.c file.

Oh, I see, Now I can understand the flow more deeply :)

> >
> > And some extension stat (maybe it only renewed that filter exists) is
> > covered inside of upper functions. Is it also right point?
>
> Depending on what you mean with "upper functions".
>
> damos_apply_scheme() in DAMON core layer calls
> damon_operationss->apply_scheme() callback of the current operations set.  The
> callback may handle DAMOS filters or not.  The callback of paddr.c handles the
> filters for DAMOS_STAT.  The callback of vaddr.c does not, and Pan is extending
> it to handle filters.

Oh, I see. Thanks to your help, Now i figure out the flow from
sysfs(or directly set) to damon_select_ops().

> [...]
> > So, I think that I need to send v2 patch, If that's right, which way
> > would you prefer?
> > reply to this thread or make a new thread?
>
> I prefer making a new thread :)
>

When the discussion of "reclaim" finishes, I'll make v2 patch as soon
as possible.

> Thanks,
> SJ
>
> [...]

Thanks for your kindness.

Best Regards.
Sang-Heon Jeon

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

[SeongJae Park]

On Sun, 3 Aug 2025 21:44:10 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

[...]
> When the discussion of "reclaim" finishes, I'll make v2 patch as soon
> as possible.

As mentioned on the other reply, I don't think the v2 should be blocked for the
discussion, unless Honggyu or others object.


Thanks,
SJ

[...]