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

[SeongJae Park <sj@kernel.org>]

Hello Asier,

On Sat, 14 Mar 2026 09:38:12 +0000 <gutierrez.asier@huawei-partners.com> wrote:

> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> 
> 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@huawei-partners.com>
> ---
>  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.
'''

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?

[1] https://sashiko.dev/#/patchset/20260314093813.1359737-1-gutierrez.asier%40huawei-partners.com


Thanks,
SJ