public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
* [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE
@ 2026-03-14  9:38 gutierrez.asier
  2026-03-14 16:51 ` SeongJae Park
  0 siblings, 1 reply; 3+ messages in thread
From: gutierrez.asier @ 2026-03-14  9:38 UTC (permalink / raw)
  To: gutierrez.asier, artem.kuzin, stepanov.anatoly, wangkefeng.wang,
	yanquanmin1, zuoze1, damon, sj, akpm, linux-mm, linux-kernel

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.

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.
  - ``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,
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 5186966dafb3..05e57c7ca2b6 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2033,6 +2033,7 @@ static struct damos_sysfs_action_name damos_sysfs_action_names[] = {
 		.action = DAMOS_PAGEOUT,
 		.name = "pageout",
 	},
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	{
 		.action = DAMOS_HUGEPAGE,
 		.name = "hugepage",
@@ -2041,6 +2042,7 @@ static struct damos_sysfs_action_name damos_sysfs_action_names[] = {
 		.action = DAMOS_NOHUGEPAGE,
 		.name = "nohugepage",
 	},
+#endif
 	{
 		.action = DAMOS_LRU_PRIO,
 		.name = "lru_prio",
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b069dbc7e3d2..19c488660027 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -897,12 +897,14 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
 	case DAMOS_PAGEOUT:
 		madv_action = MADV_PAGEOUT;
 		break;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	case DAMOS_HUGEPAGE:
 		madv_action = MADV_HUGEPAGE;
 		break;
 	case DAMOS_NOHUGEPAGE:
 		madv_action = MADV_NOHUGEPAGE;
 		break;
+#endif
 	case DAMOS_MIGRATE_HOT:
 	case DAMOS_MIGRATE_COLD:
 		return damos_va_migrate(t, r, scheme, sz_filter_passed);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE
  2026-03-14  9:38 [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE gutierrez.asier
@ 2026-03-14 16:51 ` SeongJae Park
  2026-03-16  7:03   ` Gutierrez Asier
  0 siblings, 1 reply; 3+ messages in thread
From: SeongJae Park @ 2026-03-14 16:51 UTC (permalink / raw)
  To: gutierrez.asier
  Cc: SeongJae Park, artem.kuzin, stepanov.anatoly, wangkefeng.wang,
	yanquanmin1, zuoze1, damon, akpm, linux-mm, linux-kernel

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE
  2026-03-14 16:51 ` SeongJae Park
@ 2026-03-16  7:03   ` Gutierrez Asier
  0 siblings, 0 replies; 3+ messages in thread
From: Gutierrez Asier @ 2026-03-16  7:03 UTC (permalink / raw)
  To: SeongJae Park
  Cc: artem.kuzin, stepanov.anatoly, wangkefeng.wang, yanquanmin1,
	zuoze1, damon, akpm, linux-mm, linux-kernel

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-16  7:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14  9:38 [PATCH v1] mm/damon: guard THP-specific DAMOS actions with CONFIG_TRANSPARENT_HUGEPAGE gutierrez.asier
2026-03-14 16:51 ` SeongJae Park
2026-03-16  7:03   ` Gutierrez Asier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox