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:05 UTC|newest]
Thread overview: 13+ 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-13 9:49 ` sashiko-bot
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-13 9:54 ` sashiko-bot
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-13 9:57 ` sashiko-bot
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.