Re: [RFC PATCH v6 2/2] module: allocate codetag sections before the regular module layout

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

Hi Petr

Thanks for review.

On 2026/9/1 23:04, Petr Pavlu wrote:
> On 8/31/26 9:21 AM, Hao Ge wrote:
>> Whether a codetag section goes to the codetag region is decided by
>> layout_sections() and asked again in move_module(). A concurrent
>> load can shut profiling down in between, and move_module() then
>> copies the section to offset 0 of its regular destination,
>> overwriting whatever is there.
>>
>> Decide and allocate in one pass, before the layout. Allocation
>> errors fail the load. On a tag area overflow profiling is already
>> disabled, so -EAGAIN makes the section fall back to regular module
>> data and the module still loads.
>>
>> The overflow and populate failure paths of reserve_module_tags() now
>> release their reservation instead of leaking the maple tree entry.
>> When profiling was toggled off, the overflow check did not run, a
>> module could load with more tags than the page flags can address,
>> and re-enabling profiling then silently corrupted /proc/allocinfo.
>> The check no longer depends on mem_alloc_profiling_enabled().
>>
>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Based-on-a-patch-by: Petr Pavlu <petr.pavlu@suse.com>
>> Cc: Suren Baghdasaryan <surenb@google.com>
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>> Changes against Petr's prototype:
>> - allocate_codetag_sections() returns an error instead of void, and
>>   only -EAGAIN falls back to a regular section. Any other error now
>>   fails the load. The prototype fell back on everything, which can
>>   leave live tags in module memory.
>> - reserve_module_tags() releases its reservation when populate fails
>>   too, that path used to leak the maple tree entry.
>> - codetag_free_module_sections() on the move_module() error path uses
>>   info->mod, the local mod is assigned only after a successful move.
>> - The percpu section is marked only when index.pcpu != 0, otherwise
>>   sechdrs[0] gets marked.
>> - Dropped the SHF_ALLOC check, .codetag.* sections always have it.
>> ---
>>  include/linux/module.h   |   2 +
>>  kernel/module/internal.h |   4 ++
>>  kernel/module/main.c     | 120 ++++++++++++++++++++-------------------
>>  mm/alloc_tag.c           |   9 ++-
>>  4 files changed, 75 insertions(+), 60 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 7566815fabbe..33548daa31a3 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -325,6 +325,8 @@ enum mod_mem_type {
>>  	MOD_INIT_RODATA,
>>  
>>  	MOD_MEM_NUM_TYPES,
>> +
>> +	MOD_STANDALONE = -2,
>>  	MOD_INVALID = -1,
>>  };
>>  
> 
> It might be better to split this patch into two: the first to introduce
> MOD_STANDALONE and use it only for the percpu section, and the second
> with all the codetag-related changes.
> 

OK, will do for the next version.

> The introduction of SH_ENTSIZE_STANDALONE should also allow us to clean
> up the current resetting of SHF_ALLOC for the percpu section, and now
> also for codetag sections. The problem is that find_sec(".data..percpu")

I also like that this keeps SHF_ALLOC semantics closer to the ELF spec.
https://www.sco.com/developers/gabi/latest/ch4.sheader.html

SHF_ALLOC
	The section occupies memory during process execution. Some control sections do
	not reside in the memory image of an object file; this attribute is off for those sections.

(I spent some time reading up on this, so hopefully I've understood it correctly.)

So it feels like we were taking a bit of a shortcut before.
These sections really do satisfy the SHF_ALLOC property by definition.

There were probably other reasons for doing it that way back then, but either‑way
I think adding SH_ENTSIZE_STANDALONE is a much cleaner solution.

> can currently return different results depending on whether it is called
> before layout_and_allocate() or later. In addition, apply_relocations()
> needs a special case for SH_ENTSIZE_STANDALONE, where it could otherwise
> just test SHF_ALLOC.
> 

Yeah, fair point.

> Instead of resetting SHF_ALLOC for percpu/codetag sections, both
> __layout_sections() and move_module() can check for
> SH_ENTSIZE_STANDALONE to determine whether a section is handled
> specially and should be skipped.
> 

Right, While working on the code, I noticed that __layout_sections already has a check for
this.

if ((s->sh_flags & masks[m][0]) != masks[m][0]
    || (s->sh_flags & masks[m][1])
    || s->sh_entsize != ~0UL
    || is_init != module_init_layout_section(sname))
	continue;

s->sh_entsize != ~0UL can be used to handle this.
I'll add a short comment here.

> I think it would be useful to include this change in the first patch
> introducing MOD_STANDALONE, but I'm also ok with the current version.
> I can send a separate patch later to make more use of
> SH_ENTSIZE_STANDALONE in this way.

So cool.

> 
>> @@ -2966,18 +2967,23 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>>  	 */
>>  	module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
>>  
>> -	/*
>> -	 * 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.
>> -	 */
>> +	/* Allow codetag sections to be allocated separately first. */
>> +	err = allocate_codetag_sections(info);
>> +	if (err) {
>> +		codetag_free_module_sections(info->mod);
> 
> The usual convention is that functions clean up after themselves on
> error. That means this codetag_free_module_sections() call should be
> done ideally by allocate_codetag_sections().
> 

Ack.

Thanks
Best Regards
Hao