Replace the if/else chain of sysfs_streq() calls in defrag_store()
with sysfs_match_string() and a defrag_mode_strings[] table.
Introduce enum defrag_mode and defrag_flags[] array mapping each mode
to its corresponding transparent_hugepage_flag. The store function now
loops over defrag_flags[], setting the bit for the selected mode and
clearing the others. When mode is DEFRAG_NEVER (index 4), no index
in the 4-element defrag_flags[] matches, so all flags are cleared.
Note that the enum ordering (always, defer, defer+madvise, madvise,
never) differs from the original if/else chain order in defrag_store()
(always, defer+madvise, defer, madvise, never). This is intentional to
match the display order used by defrag_show().
This is a follow-up cleanup to commit 522dfb4ba71f ("mm: huge_memory:
refactor anon_enabled_store() with change_anon_orders()") which applied
the same sysfs_match_string() pattern to anon_enabled_store().
Signed-off-by: Breno Leitao <leitao@debian.org>
---
mm/huge_memory.c | 60 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3fc02913b63e3..4843e2154038f 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -421,6 +421,29 @@ ssize_t single_hugepage_flag_store(struct kobject *kobj,
return count;
}
+enum defrag_mode {
+ DEFRAG_ALWAYS = 0,
+ DEFRAG_DEFER = 1,
+ DEFRAG_DEFER_MADVISE = 2,
+ DEFRAG_MADVISE = 3,
+ DEFRAG_NEVER = 4,
+};
+
+static const char * const defrag_mode_strings[] = {
+ [DEFRAG_ALWAYS] = "always",
+ [DEFRAG_DEFER] = "defer",
+ [DEFRAG_DEFER_MADVISE] = "defer+madvise",
+ [DEFRAG_MADVISE] = "madvise",
+ [DEFRAG_NEVER] = "never",
+};
+
+static const enum transparent_hugepage_flag defrag_flags[] = {
+ [DEFRAG_ALWAYS] = TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
+ [DEFRAG_DEFER] = TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG,
+ [DEFRAG_DEFER_MADVISE] = TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG,
+ [DEFRAG_MADVISE] = TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG,
+};
+
static ssize_t defrag_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
{
@@ -448,34 +471,19 @@ static ssize_t defrag_store(struct kobject *kobj,
struct kobj_attribute *attr,
const char *buf, size_t count)
{
- if (sysfs_streq(buf, "always")) {
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
- set_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
- } else if (sysfs_streq(buf, "defer+madvise")) {
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
- set_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
- } else if (sysfs_streq(buf, "defer")) {
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
- set_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
- } else if (sysfs_streq(buf, "madvise")) {
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
- set_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
- } else if (sysfs_streq(buf, "never")) {
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG, &transparent_hugepage_flags);
- clear_bit(TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG, &transparent_hugepage_flags);
- } else
+ int mode, m;
+
+ mode = sysfs_match_string(defrag_mode_strings, buf);
+ if (mode < 0)
return -EINVAL;
+ for (m = 0; m < ARRAY_SIZE(defrag_flags); m++) {
+ if (m == mode)
+ set_bit(defrag_flags[m], &transparent_hugepage_flags);
+ else
+ clear_bit(defrag_flags[m], &transparent_hugepage_flags);
+ }
+
return count;
}
static struct kobj_attribute defrag_attr = __ATTR_RW(defrag);
--
2.52.0On Sat, Mar 21, 2026 at 12:06 AM Breno Leitao <leitao@debian.org> wrote:
>
> Replace the if/else chain of sysfs_streq() calls in defrag_store()
> with sysfs_match_string() and a defrag_mode_strings[] table.
>
> Introduce enum defrag_mode and defrag_flags[] array mapping each mode
> to its corresponding transparent_hugepage_flag. The store function now
> loops over defrag_flags[], setting the bit for the selected mode and
> clearing the others. When mode is DEFRAG_NEVER (index 4), no index
> in the 4-element defrag_flags[] matches, so all flags are cleared.
>
> Note that the enum ordering (always, defer, defer+madvise, madvise,
> never) differs from the original if/else chain order in defrag_store()
> (always, defer+madvise, defer, madvise, never). This is intentional to
> match the display order used by defrag_show().
>
> This is a follow-up cleanup to commit 522dfb4ba71f ("mm: huge_memory:
> refactor anon_enabled_store() with change_anon_orders()") which applied
> the same sysfs_match_string() pattern to anon_enabled_store().
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
LGTM,
Reviewed-by: Barry Song <baohua@kernel.org>
On 2026/3/21 00:05, Breno Leitao wrote:
> Replace the if/else chain of sysfs_streq() calls in defrag_store()
> with sysfs_match_string() and a defrag_mode_strings[] table.
>
> Introduce enum defrag_mode and defrag_flags[] array mapping each mode
> to its corresponding transparent_hugepage_flag. The store function now
> loops over defrag_flags[], setting the bit for the selected mode and
> clearing the others. When mode is DEFRAG_NEVER (index 4), no index
> in the 4-element defrag_flags[] matches, so all flags are cleared.
>
> Note that the enum ordering (always, defer, defer+madvise, madvise,
> never) differs from the original if/else chain order in defrag_store()
> (always, defer+madvise, defer, madvise, never). This is intentional to
> match the display order used by defrag_show().
>
> This is a follow-up cleanup to commit 522dfb4ba71f ("mm: huge_memory:
> refactor anon_enabled_store() with change_anon_orders()") which applied
> the same sysfs_match_string() pattern to anon_enabled_store().
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
Thanks.
Tested-by: Lance Yang <lance.yang@linux.dev>
With David's comments addressed, feel free to add:
Reviewed-by: Lance Yang <lance.yang@linux.dev>
Cheers,
Lance
On 3/20/26 17:05, Breno Leitao wrote:
> Replace the if/else chain of sysfs_streq() calls in defrag_store()
> with sysfs_match_string() and a defrag_mode_strings[] table.
>
> Introduce enum defrag_mode and defrag_flags[] array mapping each mode
> to its corresponding transparent_hugepage_flag. The store function now
> loops over defrag_flags[], setting the bit for the selected mode and
> clearing the others. When mode is DEFRAG_NEVER (index 4), no index
> in the 4-element defrag_flags[] matches, so all flags are cleared.
>
> Note that the enum ordering (always, defer, defer+madvise, madvise,
> never) differs from the original if/else chain order in defrag_store()
> (always, defer+madvise, defer, madvise, never). This is intentional to
> match the display order used by defrag_show().
>
> This is a follow-up cleanup to commit 522dfb4ba71f ("mm: huge_memory:
> refactor anon_enabled_store() with change_anon_orders()") which applied
> the same sysfs_match_string() pattern to anon_enabled_store().
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> mm/huge_memory.c | 60 ++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3fc02913b63e3..4843e2154038f 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -421,6 +421,29 @@ ssize_t single_hugepage_flag_store(struct kobject *kobj,
> return count;
> }
>
> +enum defrag_mode {
> + DEFRAG_ALWAYS = 0,
> + DEFRAG_DEFER = 1,
> + DEFRAG_DEFER_MADVISE = 2,
> + DEFRAG_MADVISE = 3,
> + DEFRAG_NEVER = 4,
> +};
These numbers should get assigned as default (C standard) I think, and
can be dropped.
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
On Thu, Apr 02, 2026 at 10:21:30AM +0200, David Hildenbrand (Arm) wrote:
> On 3/20/26 17:05, Breno Leitao wrote:
> > +enum defrag_mode {
> > + DEFRAG_ALWAYS = 0,
> > + DEFRAG_DEFER = 1,
> > + DEFRAG_DEFER_MADVISE = 2,
> > + DEFRAG_MADVISE = 3,
> > + DEFRAG_NEVER = 4,
> > +};
>
> These numbers should get assigned as default (C standard) I think, and
> can be dropped.
You're correct that they would be assigned by default. However, I've
explicitly set them to clarify that we iterate over these enum values
using an integer and require these specific values.
This was discussed in the previous patch series:
https://lore.kernel.org/all/20260308140530.9ab6d1445d0936467eab4aef@linux-foundation.org/
Please let me know if it should be removed here, and I will update the
patch,
Thanks for the review,
--breno
On 4/2/26 15:42, Breno Leitao wrote:
> On Thu, Apr 02, 2026 at 10:21:30AM +0200, David Hildenbrand (Arm) wrote:
>> On 3/20/26 17:05, Breno Leitao wrote:
>>> +enum defrag_mode {
>>> + DEFRAG_ALWAYS = 0,
>>> + DEFRAG_DEFER = 1,
>>> + DEFRAG_DEFER_MADVISE = 2,
>>> + DEFRAG_MADVISE = 3,
>>> + DEFRAG_NEVER = 4,
>>> +};
>>
>> These numbers should get assigned as default (C standard) I think, and
>> can be dropped.
>
> You're correct that they would be assigned by default. However, I've
> explicitly set them to clarify that we iterate over these enum values
> using an integer and require these specific values.
>
> This was discussed in the previous patch series:
>
> https://lore.kernel.org/all/20260308140530.9ab6d1445d0936467eab4aef@linux-foundation.org/
>
> Please let me know if it should be removed here, and I will update the
> patch,
It's ok to leave it. It's just ... unnecessary :)
What I saw in the past is that we highlight it by only setting the "= 0"
on the first entry.
--
Cheers,
David
On Thu, Apr 02, 2026 at 03:45:25PM +0200, David Hildenbrand (Arm) wrote:
> On 4/2/26 15:42, Breno Leitao wrote:
> > On Thu, Apr 02, 2026 at 10:21:30AM +0200, David Hildenbrand (Arm) wrote:
> >> On 3/20/26 17:05, Breno Leitao wrote:
> >>> +enum defrag_mode {
> >>> + DEFRAG_ALWAYS = 0,
> >>> + DEFRAG_DEFER = 1,
> >>> + DEFRAG_DEFER_MADVISE = 2,
> >>> + DEFRAG_MADVISE = 3,
> >>> + DEFRAG_NEVER = 4,
> >>> +};
> >>
> >> These numbers should get assigned as default (C standard) I think, and
> >> can be dropped.
> >
> > You're correct that they would be assigned by default. However, I've
> > explicitly set them to clarify that we iterate over these enum values
> > using an integer and require these specific values.
> >
> > This was discussed in the previous patch series:
> >
> > https://lore.kernel.org/all/20260308140530.9ab6d1445d0936467eab4aef@linux-foundation.org/
> >
> > Please let me know if it should be removed here, and I will update the
> > patch,
>
> It's ok to leave it. It's just ... unnecessary :)
>
> What I saw in the past is that we highlight it by only setting the "= 0"
> on the first entry.
Understood. You're right that I was overly verbose here.
Thanks for the review,
--breno
© 2016 - 2026 Red Hat, Inc.