Re: [RFC PATCH v2 1/1] mm/damon: support MADV_COLLAPSE via DAMOS_COLLAPSE scheme action

[Gutierrez Asier]

Hi SJ,

First of all, I just noticed that this was sent as v2 RFC, while
it should be v1. Bear in mind that the next series will also be
v2.

On 3/17/2026 3:32 AM, SeongJae Park wrote:
> Hello Asier,
> 
> On Mon, 16 Mar 2026 18:38:05 +0000 <gutierrez.asier@xxxxxxxxxxxxxxxxxxx> wrote:
> 
>> From: Asier Gutierrez <gutierrez.asier@xxxxxxxxxxxxxxxxxxx>
>>
>> This patch set introces a new action: DAMOS_COLLAPSE.
>>
>> For DAMOS_HUGEPAGE and DAMOS_NOHUGEPAGE to work, khugepaged should be
>> working, since it relies on hugepage_madvise to add a new slot. This
>> slot should be picked up by khugepaged and eventually collapse (or
>> not, if we are using DAMOS_NOHUGEPAGE) the pages. If THP is not
>> enabled, khugepaged will not be working, and therefore no collapse
>> will happen.
>>
>> DAMOS_COLLAPSE eventually calls madvise_collapse, which will collapse
>> the address range synchronously.
>>
>> This new action may be required to support autotuning with hugepage as
>> a goal.
> 
> Above all makes sense.  Thank you for posting this patch.
> 
> Do you have some test results that you can also share together?  It would be
> nice if it can demonstrate the benefit of DAMOS_COLLAPSE over DAMOS_HUGEPAGE.
I will run some tests and benchmarks.
> 
>>
>> [1] https://lore.kernel.org/lkml/20260314165156.86647-1-sj@xxxxxxxxxx/
> 
> Seems the above link is just added by a mistake?  If not, please clarify.
Yes, it looks like I copied the wrong link.
>>
>> Signed-off-by: Asier Gutierrez <gutierrez.asier@xxxxxxxxxxxxxxxxxxx>
>> Reviewed-by: SeongJae Park <sj@xxxxxxxxxx>
>> ---
>>  Documentation/mm/damon/design.rst | 4 ++++
>>  include/linux/damon.h             | 1 +
>>  mm/damon/sysfs-schemes.c          | 4 ++++
>>  mm/damon/vaddr.c                  | 3 +++
>>  4 files changed, 12 insertions(+)
> [...]
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index 3a441fbca170..6720dc70c487 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -140,6 +140,7 @@ enum damos_action {
>>  	DAMOS_PAGEOUT,
>>  	DAMOS_HUGEPAGE,
>>  	DAMOS_NOHUGEPAGE,
>> +	DAMOS_COLLAPSE,
>>  	DAMOS_LRU_PRIO,
>>  	DAMOS_LRU_DEPRIO,
>>  	DAMOS_MIGRATE_HOT,
> 
> sashiko.dev adds [1] below comments.  Let me also add my comments in line.
> 
> : This isn't a bug, but should a kernel-doc entry for @DAMOS_COLLAPSE be added
> : to the comment block above this enum?
> 
> Makes sense.  'make htmldocs' may complain otherwise.  Asier, could you please
> add the kernel-doc comment for DAMOS_COLLAPSE in the next spin?
OK, I will split this patch into 2: one with the code and the other one with
the documentation.
> : 
> : Also, does inserting DAMOS_COLLAPSE here shift the integer values of the
> : subsequent enum entries like DAMOS_STAT?
> : 
> : The DAMON sysfs selftest script (tools/testing/selftests/damon/sysfs.py) uses
> : a hardcoded dictionary action_val to map string names to their integer enum
> : values.
> : 
> : If the enum values shift, the test's assertion:
> : 
> : assert_true(dump['action'] == action_val[scheme.action])
> : 
> : might fail when checking the struct memory via drgn. Could the python test
> : dictionary be updated to reflect the new values, or could the new action be
> : added at the end of the enum list?
> 
> There is no test that uses DAMOS actions that defined after DAMOS_NOHUGEPAGE,
> so no real test will break.  But this is a good point.  It would be better to
> update the hard-coded value together.  Asier, could you also update the
> 'action_val' dict of assert_scheme_committed() function in
> tools/testing/selftets/damon/sysfs.py for the updated enum value in the next
> version?
OK, I will do it.
> 
> [1] https://sashiko.dev/#/patchset/20260316183805.2090297-1-gutierrez.asier@xxxxxxxxxxxxxxxxxxx
> 
> 
> Thanks,
> SJ
> 
> [...]
> 
Thanks for the feedback.

-- 
Asier Gutierrez
Huawei