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

[Suren Baghdasaryan <surenb@google.com>]

On Sun, Jan 11, 2026 at 11:50 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Sat, Jan 10, 2026 at 6:34 PM Kent Overstreet
> <kent.overstreet@linux.dev> 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?
> >
> > My reading of early_param() would seem to indicate that
> > setup_early_mem_profiling() is getting called at the appropriate time -
> > and then additionally a second time by do_sysctl_args(), which then
> > becomes a noop.
> >
> > So the only bug would seem to be that the sysctl is not writeable in
> > debug mode? There's an easier fix for that one...
>
> Sorry for the delay.
> That's not a bug. We want this sysctrl to be read-only when the debug
> option is enabled. Otherwise if user toggles mem_profiling sysctrl off
> and then on again, all allocations that were made between these events
> will be missing their tags and our debug mechanism will generate
> warnings for each such occurrence when freeing these allocations.
> I'll look closer into this warning. Maybe we can suppress it when the
> read-only sysctrl is already set to the value being assigned to it?

I think the easiest way to fix this warning is to detect when the
modification is being done by do_sysctl_args() and return success, as
it's a no-op anyway (the same value was already assigned via
early_param). Something like this:

 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) {
+#ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
+                /* User can't toggle profiling while debugging */
+                if (current->mm)
+                        return -EACCES;
+
+                /*
+                 * 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().
+                 */
+                return 0;
+#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,
         },
 };

WDYT?