Re: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()
Suren Baghdasaryan <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-modules |
|---|---|
| Message-ID | <CAJuCfpHBWFZh5yHHK01CT1i-z=OP74kzGobfYskkqKgBBLx_EQ@mail.gmail.com> |
On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <[email protected]> wrote: > > codetag_needs_module_section() is called twice per codetag section, once > in layout_sections() and once in move_module(), and both depend on > mem_profiling_support, which changes without a lock. If profiling is > disabled between the two calls, layout excludes the section (offset 0) > while move copies it as normal memory to offset 0: > > CPU0 (insmod A) CPU1 (insmod B) > ---------------- ---------------- > layout_sections() > needs_section_mem() == true > sh_entsize: type, offset = 0 > reserve_module_tags() overflows > shutdown_mem_profiling() > mem_profiling_support = false > move_module() > needs_section_mem() == false > offset = sh_entsize & MASK = 0 > memcpy(mod->mem[type].base + 0, ...) > -> overwrites the first section there > > Record the decision in layout_sections() in sh_entsize using a > MOD_MEM_CODETAG type, and have move_module() use that instead of asking > again. > > reserve_module_tags() returns -EAGAIN if profiling was disabled after > layout, so the loader retries and places the section as normal memory. > > Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression") > Signed-off-by: Hao Ge <[email protected]> > --- > include/linux/module.h | 11 +++++++++++ > kernel/module/main.c | 17 ++++++----------- > mm/alloc_tag.c | 8 ++++++++ > 3 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 7566815fabbe..a02016528e1d 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -328,6 +328,17 @@ enum mod_mem_type { > MOD_INVALID = -1, > }; > > +/* > + * If CONFIG_CODE_TAGGING is on, modules get a .codetag section. > + * codetag_needs_module_section() says where it goes: the usual > + * mod->mem[], or off to the codetag region. > + * > + * Mark the codetag-region ones with MOD_MEM_NUM_TYPES. > + * It's just past the real types, so it doesn't index into mod->mem[] > + * and for_each_mod_mem_type() skips it. > + */ > +#define MOD_MEM_CODETAG MOD_MEM_NUM_TYPES Ok, it feels a bit hacky but it's probably the simplest way to mark codetag regions. > + > #define mod_mem_type_is_init(type) \ > ((type) == MOD_INIT_TEXT || \ > (type) == MOD_INIT_DATA || \ > diff --git a/kernel/module/main.c b/kernel/module/main.c > index ed26f167be84..2337bf604f58 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -1728,11 +1728,8 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i > * preallocated contiguous memory. > */ > if (codetag_needs_module_section(mod, sname, s->sh_size)) { > - /* > - * s->sh_entsize won't be used but populate the > - * type field to avoid confusion. > - */ > - s->sh_entsize = ((unsigned long)(type) & SH_ENTSIZE_TYPE_MASK) > + s->sh_entsize = ((unsigned long)MOD_MEM_CODETAG > + & SH_ENTSIZE_TYPE_MASK) > << SH_ENTSIZE_TYPE_SHIFT; > continue; > } > @@ -2815,11 +2812,10 @@ static int move_module(struct module *mod, struct load_info *info) > continue; > > sname = info->secstrings + shdr->sh_name; > - /* > - * Load codetag sections separately as they might still be used > - * after module unload. > - */ > - if (codetag_needs_module_section(mod, sname, shdr->sh_size)) { > + > + enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT; > + > + if (type == MOD_MEM_CODETAG) { > dest = codetag_alloc_module_section(mod, sname, shdr->sh_size, > arch_mod_section_prepend(mod, i), shdr->sh_addralign); > if (WARN_ON(!dest)) { > @@ -2832,7 +2828,6 @@ static int move_module(struct module *mod, struct load_info *info) > } > codetag_section_found = true; > } else { > - enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT; > unsigned long offset = shdr->sh_entsize & SH_ENTSIZE_OFFSET_MASK; > > dest = mod->mem[type].base + offset; > diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c > index 461fa87fbb0b..7481180dadd2 100644 > --- a/mm/alloc_tag.c > +++ b/mm/alloc_tag.c > @@ -893,6 +893,14 @@ static void *reserve_module_tags(struct module *mod, unsigned long size, > if (size < sizeof(struct alloc_tag)) > return ERR_PTR(-EINVAL); > > + /* > + * Profiling may have been disabled by a concurrent module load. > + * Return -EAGAIN so the loader retries with profiling off, laying > + * the section out as ordinary module memory. > + */ > + if (!mem_profiling_support) > + return ERR_PTR(-EAGAIN); I think this requires your patch [1] from another patchset to work correctly, correct? If so, I would suggest sending this patch as part of that patchset since there is a dependency. [1] https://lore.kernel.org/all/[email protected]/ > + > /* > * align is always power of 2, so we can use IS_ALIGNED and ALIGN. > * align 0 or 1 means no alignment, to simplify set to 1. > -- > 2.25.1 >