From: Hao Ge <hao.ge@linux.dev>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Luis Chamberlain <mcgrof@kernel.org>,
Petr Pavlu <petr.pavlu@suse.com>,
Daniel Gomez <da.gomez@kernel.org>,
Sami Tolvanen <samitolvanen@google.com>,
Aaron Tomlin <atomlin@atomlin.com>,
linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled
Date: Mon, 17 Aug 2026 13:05:36 +0800 [thread overview]
Message-ID: <e19d1b50-4d10-4833-8770-5003a0593518@linux.dev> (raw)
In-Reply-To: <CAJuCfpHAzhCC1Sx3uJr7kbhBzc7RFqwLhMyjo04hqZz=HBJH8Q@mail.gmail.com>
Hi Suren
On 2026/8/15 14:15, Suren Baghdasaryan wrote:
> On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> 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 CODETAG_MODULE_EXCLUDED from load_module() when profiling is off:
>> codetag_module_init() drops the module's cmod and no counters are
>> allocated. codetag_unload_module() now always calls free_section_mem(),
>> since an excluded module may still hold a reserved section.
>>
>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>
> Thanks for the fix. I think it could be done simpler, see below.
Agree, will do in the next version.
>
>> ---
>> include/linux/codetag.h | 4 ++++
>> lib/codetag.c | 8 +++++---
>> mm/alloc_tag.c | 8 ++++++--
>> 3 files changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/codetag.h b/include/linux/codetag.h
>> index a25a085c2df1..88081c618673 100644
>> --- a/include/linux/codetag.h
>> +++ b/include/linux/codetag.h
>> @@ -52,6 +52,10 @@ struct codetag_type_desc {
>> #endif
>> };
>>
>> +/* module_load() return values */
>> +#define CODETAG_MODULE_LOAD 0 /* module loads with its tags */
>> +#define CODETAG_MODULE_EXCLUDED 1 /* module loads without its tags */
>
> I see no reason for adding these special values. You could simply
> return -ENOTSUP when profiling is disabled.
>
>> +
>> struct codetag_iterator {
>> struct codetag_type *cttype;
>> struct codetag_module *cmod;
>> diff --git a/lib/codetag.c b/lib/codetag.c
>> index a9cda4c962a3..8506ecab9ea7 100644
>> --- a/lib/codetag.c
>> +++ b/lib/codetag.c
>> @@ -238,9 +238,10 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
>> }
>> up_write(&cttype->mod_lock);
>>
>> - if (err < 0) {
>> + if (err) {
>> + /* Error or excluded: cmod is dropped, free it. */
>
> IIUC here you want to call kfree() if profiling got disabled. If you
> return -ENOTSUP instead of CODETAG_MODULE_EXCLUDED then this condition
> does not need to change.
>
>> kfree(cmod);
>> - return err;
>> + return err < 0 ? err : 0;
>
> Here you can do:
> if (err && err != -ENOTSUP)
> return err;
> return 0;
>
>> }
>>
>> return 0;
>> @@ -388,7 +389,8 @@ void codetag_unload_module(struct module *mod)
>> ++cttype->content_id;
>> }
>> up_write(&cttype->mod_lock);
>> - if (found && cttype->desc.free_section_mem)
>> + /* an excluded module may still hold 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 0a7b657fe2de..461fa87fbb0b 100644
>> --- a/mm/alloc_tag.c
>> +++ b/mm/alloc_tag.c
>> @@ -977,9 +977,13 @@ 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 but exclude its tags. */
>> + if (!mem_profiling_support)
>> + return CODETAG_MODULE_EXCLUDED;
>
> Return -ENOTSUP here.
>
>
>
>> +
>> /* percpu counters for core allocations are already statically allocated */
>> if (!mod)
>> - return 0;
>> + return CODETAG_MODULE_LOAD;
>>
>> start_tag = ct_to_alloc_tag(start);
>> stop_tag = ct_to_alloc_tag(stop);
>> @@ -1002,7 +1006,7 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
>> */
>> kmemleak_ignore_percpu(tag->counters);
>> }
>> - return 0;
>> + return CODETAG_MODULE_LOAD;
>> }
>>
>> static void replace_module(struct module *mod, struct module *new_mod)
>> --
>> 2.25.1
>>
next prev parent reply other threads:[~2026-08-17 5:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 9:34 [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling() Hao Ge
2026-08-13 9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
2026-08-15 6:15 ` Suren Baghdasaryan
2026-08-17 5:05 ` Hao Ge [this message]
2026-08-13 9:34 ` [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() Hao Ge
2026-08-15 6:21 ` Suren Baghdasaryan
2026-08-17 5:37 ` Hao Ge
2026-08-13 9:34 ` [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
2026-08-15 6:51 ` Suren Baghdasaryan
2026-08-17 5:41 ` Hao Ge
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e19d1b50-4d10-4833-8770-5003a0593518@linux.dev \
--to=hao.ge@linux.dev \
--cc=akpm@linux-foundation.org \
--cc=atomlin@atomlin.com \
--cc=da.gomez@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-modules@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=petr.pavlu@suse.com \
--cc=samitolvanen@google.com \
--cc=surenb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox