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

[Suren Baghdasaryan]

On Thu, Sep 17, 2026 at 7:36 PM Hao Ge <hao.ge@xxxxxxxxx> wrote:
>
> Hi Suren
>
> On 2026/9/18 08:53, Suren Baghdasaryan wrote:
> > On Mon, Sep 14, 2026 at 11:59 PM Hao Ge <hao.ge@xxxxxxxxx> 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@xxxxxxxxxx>
> >> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> Suggested-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> >> Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> >
> > 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?

Ok, fair enough. I think your current fix is the simplest way to
resolve this issue, so let's keep it simple.

Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>

>
> Thanks
> Best Regards
> Hao
>
> >> Signed-off-by: Hao Ge <hao.ge@xxxxxxxxx>
> >> ---
> >>  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
> >>