On Sat, Aug 15, 2026 at 3:45 AM Petr Pavlu <[email protected]> wrote:
>
> On 8/12/26 7:41 AM, Hao Ge wrote:
> > In reserve_module_tags(), the tag overflow check is gated on
> > mem_alloc_profiling_enabled():
> >
> > if (mem_alloc_profiling_enabled() && !tags_addressable())
> >
> > If profiling is toggled off at runtime and a module is loaded whose
> > tags exceed the compressed-mode limit, shutdown_mem_profiling() is
> > skipped. vm_module_tags_populate() still maps memory for the tags and
> > the module loads successfully, but the total tag count now exceeds what
> > NR_UNUSED_PAGEFLAG_BITS can address.
> >
> > Once profiling is re-enabled, ref_to_idx() computes each tag's index
> > as its position in the alloc_tag array. update_page_tag_ref() masks
> > it to alloc_tag_ref_mask before storing in page->flags. Indices
> > beyond the mask are truncated and idx_to_ref() resolves them to wrong
> > tags.
> >
> > This silently corrupts /proc/allocinfo: allocated pages get attributed
> > to the wrong call sites, so the statistics it reports are wrong.
> >
> > mem_alloc_profiling_enabled() and mem_profiling_compressed are
> > independent. Once compressed mode is established at boot, it stays
> > active regardless of runtime toggles of mem_profiling.
> >
> > Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down
> > profiling, release the reservation, and return -EAGAIN so that
> > layout_and_allocate() retries with profiling disabled: codetag sections
> > are then placed as regular module data and the module loads without
> > profiling rather than being rejected entirely.
>
> When the described overflow occurs, why should codetag sections be
> placed as regular module data? Will the codetag support use them in any
> way, or do they simply waste space? Is the issue that alloc_hooks()
> creates relocations pointing into .codetag.alloc_tags?
Correct, alloc_hooks() will have references into .codetag.alloc_tags.
With mem_profiling_support=false they should technically never be used
but I don't think it's a good idea to skip .codetag.alloc_tags section
allocation and to leave dangling pointers. Also the case described
here is an outlier, so optimizing it would not yield much benefit.
>
> >
> > Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag
> > compression")
> > Cc: [email protected]
> > Suggested-by: Suren Baghdasaryan <[email protected]>
> > Signed-off-by: Hao Ge <[email protected]>
> > ---
> > kernel/module/main.c | 25 +++++++++++++++++++++++--
> > mm/alloc_tag.c | 8 +++++---
> > 2 files changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index 46dd8d25a605..ed26f167be84 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -2944,6 +2944,7 @@ static struct module *layout_and_allocate(struct
> > load_info *info, int flags)
> > {
> > struct module *mod;
> > int err;
> > + unsigned long frob_size[MOD_MEM_NUM_TYPES];
>
> frob_size is used to store values of module_memory::size, which has type
> `unsigned int`. The types should match.
>
> >
> > /* Allow arches to frob section contents and sizes. */
> > err = module_frob_arch_sections(info->hdr, info->sechdrs,
> > @@ -2966,18 +2967,38 @@ static struct module *layout_and_allocate(struct
> > load_info *info, int flags)
> > */
> > module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
> >
> > + /*
> > + * Save the sizes reserved by module_frob_arch_sections() so they can
> > + * be restored if we retry below.
> > + */
> > + for_each_mod_mem_type(type)
> > + frob_size[type] = info->mod->mem[type].size;
> > +
> > /*
> > * Determine total sizes, and put offsets in sh_entsize. For now
> > * this is done generically; there doesn't appear to be any
> > * special cases for the architectures.
> > */
> > +retry:
> > layout_sections(info->mod, info);
> > layout_symtab(info->mod, info);
> >
> > /* Allocate and move to the final place */
> > err = move_module(info->mod, info);
> > - if (err)
> > - return ERR_PTR(err);
> > + if (err) {
> > + if (err != -EAGAIN)
> > + return ERR_PTR(err);
>
> The move_module() logic is non-trivial. -EAGAIN could be returned by
> other code, now or in the future.
That's a good point.
>
> > + /*
> > + * -EAGAIN means profiling was disabled but the module
> > + * can still load without it. Reset state and retry.
> > + */
> > + rewrite_section_headers(info, flags);
> > + for_each_mod_mem_type(type)
> > + info->mod->mem[type].size = frob_size[type];
> > + info->sechdrs[info->index.sym].sh_flags &= ~(unsigned
> > long)SHF_ALLOC;
> > + info->sechdrs[info->index.str].sh_flags &= ~(unsigned
> > long)SHF_ALLOC;
>
> Why is it necessary to reset SHF_ALLOC for .symtab and .strtab here?
I believe layout_symtab() sets that bit and to retry we need to reset
it. But I might be wrong here.
>
> > + goto retry;
> > + }
> >
> > /* Module has been copied to its final place now: return it. */
> > mod = (void *)info->sechdrs[info->index.mod].sh_addr;
>
> I'm not sure this is the best approach. It's complex logic for what
> appears to be an edge case related to a debugging facility. It will have
> the usual problem of error paths not getting enough testing and breaking
> subtly over time.
>
> An alternative could be to reset SHF_ALLOC on the codetag section to
> remove it from further processing and have relocations that point to
> this section resolve to something else. It seems that alloc_hooks_tag()
> could tolerate this, since it only needs to reference the associated
> alloc_tag when mem_alloc_profiling_enabled() is true and that gets
> disabled by reserve_module_tags() on the overflow.
Hmm, yeah if we redirect the references into .codetag.alloc_tags, that
would be much better.
>
> It is also not an ideal approach, but I feel it could be less intrusive
> to the module loader. I can put together a prototype if needed.
If your approach does not cause module loading to fail when we disable
profiling, then that sounds like a good idea. If it's not too much
trouble, could you please send an RFC?
>
> --
> Thanks,
> Petr