Re: [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE

[Gutierrez Asier]

Hi SJ,

On 3/14/2026 7:51 PM, SeongJae Park wrote:
> Hello Asier,
> 
> On Sat, 14 Mar 2026 09:38:12 +0000 <gutierrez.asier@xxxxxxxxxxxxxxxxxxx> wrote:
> 
>> From: Asier Gutierrez <gutierrez.asier@xxxxxxxxxxxxxxxxxxx>
>>
>> MADV_HUGEPAGE and MADV_NOHUGEPAGE are guarded and they are not
>> available when compiling the kernel without TRANSPARENT_HUGEPAGE
>> option. Therefore, DAMOS_HUGEPAGE and DAMOS_NOHUGEPAGE shouldn't be
>> available to the user either.
> 
> That makes many sense to me.  But, I'd prefer having ifdef as minimum as
> possible because ifdef increases amount of code that I need to maintain per
> different configurations.  I understand use of ifdef can make the resulting
> kernel image more optimized.  But, unless the benefit of optimization is
> obviously big enough, I'd prefer simplicity of the code in general.
> 
> When TRASPARENT_HUGEPAGE is off, DAMOS_[NO]HUGEPAGE will just silently fail.  I
> think that's not a bad user experience if the silently fail is kindly informed.
> How about updating document for that, but keeping the code as is?
> 
>>
>> Signed-off-by: Asier Gutierrez <gutierrez.asier@xxxxxxxxxxxxxxxxxxx>
>> ---
>>  Documentation/mm/damon/design.rst | 6 ++++--
>>  include/linux/damon.h             | 2 ++
>>  mm/damon/sysfs-schemes.c          | 2 ++
>>  mm/damon/vaddr.c                  | 2 ++
>>  4 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
>> index 29fff20b3c2a..207219ee10d4 100644
>> --- a/Documentation/mm/damon/design.rst
>> +++ b/Documentation/mm/damon/design.rst
>> @@ -460,9 +460,11 @@ that supports each action are as below.
>>   - ``pageout``: Reclaim the region.
>>     Supported by ``vaddr``, ``fvaddr`` and ``paddr`` operations set.
>>   - ``hugepage``: Call ``madvise()`` for the region with ``MADV_HUGEPAGE``.
>> -   Supported by ``vaddr`` and ``fvaddr`` operations set.
>> +   Supported by ``vaddr`` and ``fvaddr`` operations set when
>> +   TRANSPARENT_HUGEPAGE is enabled.
>>   - ``nohugepage``: Call ``madvise()`` for the region with ``MADV_NOHUGEPAGE``.
>> -   Supported by ``vaddr`` and ``fvaddr`` operations set.
>> +   Supported by ``vaddr`` and ``fvaddr`` operations set when
>> +   TRANSPARENT_HUGEPAGE is enabled.
> 
> I like the above change.  I'd suggest make it just describe the current
> behavior in a better way.  E.g., "Supported by vaddr and fvaddr.  When
> CONFIG_TRANSPARENT_HUGEPAG is disabled, however, the application of the action
> will just fail always.
> 
>>   - ``lru_prio``: Prioritize the region on its LRU lists.
>>     Supported by ``paddr`` operations set.
>>   - ``lru_deprio``: Deprioritize the region on its LRU lists.
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index 3a441fbca170..b3fcbe84b5d7 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -138,8 +138,10 @@ enum damos_action {
>>  	DAMOS_WILLNEED,
>>  	DAMOS_COLD,
>>  	DAMOS_PAGEOUT,
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>  	DAMOS_HUGEPAGE,
>>  	DAMOS_NOHUGEPAGE,
>> +#endif
>>  	DAMOS_LRU_PRIO,
>>  	DAMOS_LRU_DEPRIO,
>>  	DAMOS_MIGRATE_HOT,
> 
> Also, sashiko.dev commented [1] some conrens as below:
> 
> '''
> Does adding this #ifdef shift the integer values of subsequent actions like
> DAMOS_STAT when CONFIG_TRANSPARENT_HUGEPAGE is disabled?
> The kernel selftest tools/testing/selftests/damon/sysfs.py uses drgn to read
> the internal damos->action enum value and expects hardcoded integer values
> (such as 'stat': 9).
> Additionally, modifying enum layouts based on kernel configuration can break
> BPF and drgn tracing tools that rely on stable DWARF/BTF types.
> '''
> 
That's a good catch, I didn't think about it.
> There is no selftest using DAMOS action that defined after DAMOS_HUGEPAGE, so
> this change should be _safe_.  But the code does have a hard-coded mapping of
> DAMOS action to value, in sysfs.py file under the DAMON selftests directory.
> Maintaining it correctly with future tests for different configuration may be
> challenging.
> 
> So I'd again prefer not adding ifdef.
> 
> [...]
> 
> To summarize again, how about fixing the document but keeping the code as is?
If the expected behaviour is to silently fail, I agree, documenting this
should be the best option. I will submit a new patch with the documentation
update.
> 
> [1] https://sashiko.dev/#/patchset/20260314093813.1359737-1-gutierrez.asier%40huawei-partners.com
> 
> 
> Thanks,
> SJ
> 

-- 
Asier Gutierrez
Huawei