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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.