Re: [PATCH v7 3/4] mm: huge_memory: refactor enabled_store() with set_global_enabled_mode()

Lorenzo Stoakes (Oracle)
2026-03-17T17:15:25Z

On Tue, Mar 17, 2026 at 08:33:58AM -0700, Breno Leitao wrote:
> Refactor enabled_store() to use a new set_global_enabled_mode() helper.
> Introduce a separate enum global_enabled_mode and
> global_enabled_mode_strings[], mirroring the anon_enabled_mode
> pattern from the previous commit.
>
> A separate enum is necessary because the global THP setting does
> not support "inherit", only "always", "madvise", and "never".
> Reusing anon_enabled_mode would leave a NULL gap in the string
> array, causing sysfs_match_string() to stop early and fail to
> match entries after the gap.
>
> The helper uses the same loop pattern as set_anon_enabled_mode(),
> iterating over an array of flag bit positions and using
> test_and_set_bit()/test_and_clear_bit() to track whether the state
> actually changed.
>
> Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> mm/huge_memory.c | 63 ++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index f6af90e6cf05d..b95fde5843399 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -330,30 +330,63 @@ static const char * const anon_enabled_mode_strings[] = {
> [ANON_ENABLED_NEVER] = "never",
> };
>
> +enum global_enabled_mode {
> + GLOBAL_ENABLED_ALWAYS = 0,
> + GLOBAL_ENABLED_MADVISE = 1,
> + GLOBAL_ENABLED_NEVER = 2,
> +};
> +
> +static const char * const global_enabled_mode_strings[] = {
> + [GLOBAL_ENABLED_ALWAYS] = "always",
> + [GLOBAL_ENABLED_MADVISE] = "madvise",
> + [GLOBAL_ENABLED_NEVER] = "never",
> +};
> +
> +static bool set_global_enabled_mode(enum global_enabled_mode mode)
> +{
> + static const unsigned long thp_flags[] = {
> + TRANSPARENT_HUGEPAGE_FLAG,
> + TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
> + };
> + enum global_enabled_mode m;
> + bool changed = false;
> +
> + for (m = 0; m < ARRAY_SIZE(thp_flags); m++) {
> + if (m == mode)
> + changed |= !test_and_set_bit(thp_flags[m],
> + &transparent_hugepage_flags);
> + else
> + changed |= test_and_clear_bit(thp_flags[m],
> + &transparent_hugepage_flags);

LGTM!

> + }
> +
> + return changed;
> +}
> +
> static ssize_t enabled_store(struct kobject *kobj,
> struct kobj_attribute *attr,
> const char *buf, size_t count)
> {
> - ssize_t ret = count;
> + int mode;
>
> - if (sysfs_streq(buf, "always")) {
> - clear_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, &transparent_hugepage_flags);
> - set_bit(TRANSPARENT_HUGEPAGE_FLAG, &transparent_hugepage_flags);
> - } else if (sysfs_streq(buf, "madvise")) {
> - clear_bit(TRANSPARENT_HUGEPAGE_FLAG, &transparent_hugepage_flags);
> - set_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, &transparent_hugepage_flags);
> - } else if (sysfs_streq(buf, "never")) {
> - clear_bit(TRANSPARENT_HUGEPAGE_FLAG, &transparent_hugepage_flags);
> - clear_bit(TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG, &transparent_hugepage_flags);
> - } else
> - ret = -EINVAL;
> + mode = sysfs_match_string(global_enabled_mode_strings, buf);
> + if (mode < 0)
> + return -EINVAL;
>
> - if (ret > 0) {
> + if (set_global_enabled_mode(mode)) {
> int err = start_stop_khugepaged();
> +
> if (err)
> - ret = err;
> + return err;
> + } else {
> + /*
> + * Recalculate watermarks even when the mode didn't
> + * change, as the previous code always called
> + * start_stop_khugepaged() which does this internally.
> + */
> + set_recommended_min_free_kbytes();
> }
> - return ret;
> + return count;
> }
>
> static struct kobj_attribute enabled_attr = __ATTR_RW(enabled);
>
> --
> 2.52.0
>