Re: [PATCH v10 5/6] alloc_tag: skip percpu counter allocation when profiling is disabled

[Hao Ge <hao.ge@linux.dev>]

Hi Suren

On 2026/9/18 08:53, Suren Baghdasaryan wrote:
> On Mon, Sep 14, 2026 at 11:59 PM Hao Ge <hao.ge@linux.dev> wrote:
>>
>> After shutdown_mem_profiling() clears mem_profiling_support,
>> needs_section_mem() returns false, so later modules have their codetag
>> section placed as regular data and never enter the alloc_tag maple tree.
>> codetag_load_module() still called load_module(), which allocated a percpu
>> counter for every tag; release_module_tags() could not find these modules
>> on unload, so the counters leaked.
>>
>> Return -EOPNOTSUPP from load_module() when profiling is off:
>> codetag_module_init() drops the module's cmod, no counters are allocated
>> and the module loads without its tags. codetag_unload_module() now always
>> calls free_section_mem(), since a module whose module_load() returned
>> -EOPNOTSUPP is not in the idr but may still hold a reserved section.
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Suren Baghdasaryan <surenb@google.com>
>> Acked-by: Suren Baghdasaryan <surenb@google.com>
> 
> Yeah, I guess I didn't think about alternatives...
> 
> When mem_profiling_support=false, why can't load_module() skip
> allocating percpu counters, zero out the whole area between start_tag
> and stop_tag and return 0 as success (mem_profiling_support is not
> enabled, so we acted accordingly)?
> Module will be added into maple tree and later codetag_unload_module()
> will find the module, call release_module_tags() (which is a NOOP
> because tag->counter==NULL) and finally call free_section_mem(). This
> seems more natural than special-casing with EOPNOTSUPP. WDYT?
> 

I see, but the module would then sit in the idr with NULL-counter
tags that become visible to readers which today never expect that state.

alloc_tag_top_users calls codetag_next_ct() to find the next codetag
and alloc_tag_read() would do per_cpu_ptr(NULL) for each cpu - plain
pointer arithmetic onto a wild address, an oops straight out of the
OOM report. 

https://elixir.bootlin.com/linux/v7.3-rc3/source/mm/alloc_tag.c#L503

So if the codetag section within this codetag_type ends up meaningless,
is there a reason we still return success? We will still maintain cttype->count
and allocate a slot for this module inside mod_idr.

I'd rather keep the -EOPNOTSUPP version (your original suggestion),
also because the comments in load_module() and codetag_module_init()
already document this scenario.

WDYT?

Thanks
Best Regards
Hao

>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>>  lib/codetag.c  | 10 ++++++++--
>>  mm/alloc_tag.c |  4 ++++
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/codetag.c b/lib/codetag.c
>> index a9cda4c962a3..a0b600720afc 100644
>> --- a/lib/codetag.c
>> +++ b/lib/codetag.c
>> @@ -240,7 +240,9 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
>>
>>         if (err < 0) {
>>                 kfree(cmod);
>> -               return err;
>> +               /* -EOPNOTSUPP means we can load the module without its tag. */
>> +               if (err != -EOPNOTSUPP)
>> +                       return err;
>>         }
>>
>>         return 0;
>> @@ -388,7 +390,11 @@ void codetag_unload_module(struct module *mod)
>>                         ++cttype->content_id;
>>                 }
>>                 up_write(&cttype->mod_lock);
>> -               if (found && cttype->desc.free_section_mem)
>> +               /*
>> +                * A module whose module_load() returned -EOPNOTSUPP is not
>> +                * in the idr but may still hold reserved section memory.
>> +                */
>> +               if (cttype->desc.free_section_mem)
>>                         cttype->desc.free_section_mem(mod, true);
>>         }
>>         mutex_unlock(&codetag_lock);
>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
>> index 5836803898ad..1ca0409b492b 100644
>> --- a/mm/alloc_tag.c
>> +++ b/mm/alloc_tag.c
>> @@ -988,6 +988,10 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
>>         struct alloc_tag *stop_tag;
>>         struct alloc_tag *tag;
>>
>> +       /* Profiling disabled: load the module without its tags. */
>> +       if (!mem_profiling_support)
>> +               return -EOPNOTSUPP;
>> +
>>         /* percpu counters for core allocations are already statically allocated */
>>         if (!mod)
>>                 return 0;
>> --
>> 2.25.1
>>