Re: [PATCH] alloc_tag: remove sysctl prefix from mem_profiling boot parameter

[Suren Baghdasaryan <surenb@google.com>]

On Tue, Jan 13, 2026 at 11:25 PM <ranxiaokai627@163.com> wrote:
>
> >On Mon, Jan 12, 2026 at 7:50 PM Kent Overstreet
> ><kent.overstreet@linux.dev> wrote:
> >>
> >> On Tue, Jan 13, 2026 at 03:27:35AM +0000, ranxiaokai627@163.com wrote:
> >> > >On Fri, Jan 09, 2026 at 06:24:19AM +0000, ranxiaokai627@163.com wrote:
> >> > >> From: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> >> > >>
> >> > >> Boot parameters prefixed with "sysctl." are processed separately
> >> > >> during the final stage of system initialization via kernel_init()->
> >> > >> do_sysctl_args(). Since mem_profiling support should be parsed
> >> > >> in early boot stage, it is unsuitable for centralized handling
> >> > >> in do_sysctl_args().
> >> > >> Also, when CONFIG_MEM_ALLOC_PROFILING_DEBUG is enabled,
> >> > >> the sysctl.vm.mem_profiling entry is not writable and will cause
> >> > >> a warning. To prevent duplicate processing of sysctl.vm.mem_profiling,
> >> > >> rename the boot parameter to "mem_profiling".
> >> > >>
> >> > >> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> >> > >
> >> > >How was this observed/detected?
> >> >
> >> > Actually no kernel bug or funtional defect was observed through testing.
> >> > Via code reading, i found after commit [1],
> >> > boot parameters prefixed with sysctl is processed redundantly.
> >
> >I was able to reproduce the warning by enabling
> >CONFIG_MEM_ALLOC_PROFILING,
> >CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
> >CONFIG_MEM_ALLOC_PROFILING_DEBUG, CONFIG_SYSCTL and setting
> >CONFIG_CMDLINE="1".
> >The fix I posted eliminates that warning. Ran, you can post my
> >suggestion yourself with me as Suggested-by or I can post it with you
> >as Reported-by. Let me know your preference.
>
> I think this version is better.
>
> [PATCH] alloc_tag: fix rw permission issue when handling boot parameter
>
> Boot parameters prefixed with "sysctl." are processed
> during the final stage of system initialization via kernel_init()->
> do_sysctl_args(). When CONFIG_MEM_ALLOC_PROFILING_DEBUG is enabled,
> the sysctl.vm.mem_profiling entry is not writable and will cause
> a warning.
>
> Before run_init_process(), system initialization executes in kernel
> thread context. Use current->mm to distinguish sysctl writes during
> do_sysctl_args() from user-space triggered ones.
>
> And when the proc_handler is from do_sysctl_args(), always return success
> because the same value was already set by setup_early_mem_profiling()
> and this eliminates a permission denied warning.
>
> Signed-off-by: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> Suggested-by: Suren Baghdasaryan <surenb@google.

Please fix the above tag, it should end with ">" instead of "." and
send a v2 as a separate email, not a reply to the original thread.
Otherwise LGTM. Feel free to add:

Acked-by: Suren Baghdasaryan <surenb@google.com>

> ---
>  lib/alloc_tag.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 846a5b5b44a4..00ae4673a271 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -776,8 +776,22 @@ EXPORT_SYMBOL(page_alloc_tagging_ops);
>  static int proc_mem_profiling_handler(const struct ctl_table *table, int write,
>                                       void *buffer, size_t *lenp, loff_t *ppos)
>  {
> -       if (!mem_profiling_support && write)
> -               return -EINVAL;
> +       if (write) {
> +               /*
> +                * Call from do_sysctl_args() which is a no-op since the same
> +                * value was already set by setup_early_mem_profiling.
> +                * Return success to avoid warnings from do_sysctl_args().
> +                */
> +               if (!current->mm)
> +                       return 0;
> +
> +#ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
> +               /* User can't toggle profiling while debugging */
> +               return -EACCES;
> +#endif
> +               if (!mem_profiling_support)
> +                       return -EINVAL;
> +       }
>
>         return proc_do_static_key(table, write, buffer, lenp, ppos);
>  }
> @@ -787,11 +801,7 @@ static const struct ctl_table memory_allocation_profiling_sysctls[] = {
>         {
>                 .procname       = "mem_profiling",
>                 .data           = &mem_alloc_profiling_key,
> -#ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
> -               .mode           = 0444,
> -#else
>                 .mode           = 0644,
> -#endif
>                 .proc_handler   = proc_mem_profiling_handler,
>         },
>  };
> --
> 2.25.1
>
>
> >> When bcachefs was in the kernel, I spent an inordinate amount of time in
> >> code reviews trying to convince people that yes, they really do need to
> >> be testing their code.
> >>
> >> Strangely enough, I have never had this issue with project contributors
> >> who did not come to the project by way of the kernel community... :)
>