alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
Subsystems (5)
Hao Ge
reviewerv2 cover alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
Two fixes for issues reported by sashiko:
1. percpu counter leak on modules loaded after profiling is disabled.
2. AB-BA deadlock between module load and /proc/allocinfo readers.
Changes since the RFC [2]:
- The "move codetag section placement decision to layout_sections()" patch is dropped from this series. Its retry path depends on [1], so it will be sent as part of that patchset instead.
- Patch 1 now returns -EOPNOTSUPP instead of introducing CODETAG_MODULE_LOAD/CODETAG_MODULE_EXCLUDED defines (suggested by Suren).
- Reported-by tags added.
Patch 1 skips percpu counter allocation when profiling is disabled, so the module loads without its tags instead of leaking counters. Patch 2 defers remove_proc_entry() to a workqueue.
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/
Hao Ge (2):
alloc_tag: skip percpu counter allocation when profiling is disabled
alloc_tag: remove /proc/allocinfo outside of mod_lock
lib/codetag.c | 10 ++++++++-- mm/alloc_tag.c | 14 ++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-)
--
2.25.1
Hao Ge
reviewerPATCH v2 1/2 alloc_tag: skip percpu counter allocation when profiling is disabled
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 -EOPNOTSUPP from load_module() when profiling is off:codetag_module_init() drops the module's cmod, no counters are allocated and the module loads without its tags. codetag_unload_module() now always calls free_section_mem(), since a module whose module_load() returned -EOPNOTSUPP is not in the idr but may still hold a reserved section.
--- lib/codetag.c | 10 ++++++++-- mm/alloc_tag.c | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)
lib/codetag.c +8 -2
@@ -240,7 +240,9 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)if (err <0 ) {kfree (cmod); -return err; +/* -EOPNOTSUPP means we can load the module without its tag. */ +if (err != -EOPNOTSUPP ) +return err; }return 0 ;
@@ -388,7 +390,11 @@ void codetag_unload_module(struct module *mod) ++cttype->content_id; }up_write (&cttype->mod_lock); -if (found && cttype->desc.free_section_mem) +/* +* A module whose module_load() returned -EOPNOTSUPP is not +* in the idr but may still hold reserved section memory. +*/ +if (cttype->desc.free_section_mem) cttype->desc.free_section_mem (mod, true); }mutex_unlock (&codetag_lock);
mm/alloc_tag.c +4 -0
@@ -975,6 +975,10 @@ static int load_module(struct module *mod, struct codetag *start, struct codetagstruct alloc_tag *stop_tag;struct alloc_tag *tag; +/* Profiling disabled: load the module without its tags. */ +if (!mem_profiling_support) +return -EOPNOTSUPP ; +/* percpu counters for core allocations are already statically allocated */ if (!mod)return 0 ;
Signature · 1 line
2.25.1
Hao Ge
reviewerPATCH v2 2/2 alloc_tag: remove /proc/allocinfo outside of mod_lock
In reply to Hao Geshutdown_mem_profiling() calls remove_proc_entry() from reserve_module_tags(), which runs under mod_lock held for write. remove_proc_entry() waits for readers, and a reader takes mod_lock for read in allocinfo_start():
CPU0 (insmod) CPU1 (read /proc/allocinfo)
---------------- ----------------------------
reserve_module_tags()
down_write(&mod_lock) [held]
use_pde() [in_use++]
allocinfo_start()
down_read(&mod_lock) <- blocks
shutdown_mem_profiling()
remove_proc_entry()
wait for in_use == 0 <- blocksMove remove_proc_entry() to a workqueue.
--- mm/alloc_tag.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
mm/alloc_tag.c +9 -1
@@ -15,6 +15,7 @@#include <linux/seq_file.h> #include <linux/string_choices.h> #include <linux/vmalloc.h> +#include <linux/workqueue.h> #include <linux/kmemleak.h> #include <uapi/linux/alloc_tag.h>
@@ -591,6 +592,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)put_page_tag_ref (handle_new); } +static void remove_allocinfo_file (struct work_struct *work) +{ +remove_proc_entry (ALLOCINFO_FILE_NAME ,NULL ); +} + +static DECLARE_WORK(remove_allocinfo_work, remove_allocinfo_file); +static void shutdown_mem_profiling (bool remove_file) {if (mem_alloc_profiling_enabled ())
@@ -600,7 +608,7 @@ static void shutdown_mem_profiling(bool remove_file)return ;if (remove_file) -remove_proc_entry (ALLOCINFO_FILE_NAME ,NULL ); +schedule_work (&remove_allocinfo_work); mem_profiling_support = false; }
Signature · 1 line
2.25.1
Re: [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled
In reply to Hao GeQuoted 14 lines · Hao Ge
After
shutdown_mem_profiling()clearsmem_profiling_support,needs_section_mem()returns false, so later modules have their codetag section placed as regular data and never enter thealloc_tagmaple tree.codetag_load_module()still calledload_module(), which allocated a percpu counter for every tag;release_module_tags()could not find these modules on unload, so the counters leaked.Return
-EOPNOTSUPPfromload_module()when profiling is off:codetag_module_init()drops the module's cmod, no counters are allocated and the module loads without its tags.codetag_unload_module()now always callsfree_section_mem(), since a module whosemodule_load()returned-EOPNOTSUPPis not in the idr but may still hold a reserved section.
Quoted 5 lines
Reported-by Sashiko <[email protected]>Fixes 4835f747d3ed ("alloc_tag: support for page allocation tag compression")Suggested-by Suren Baghdasaryan <[email protected]>Signed-off-by Hao Ge <[email protected]>
Quoted 51 lines
--- lib/codetag.c | 10 ++++++++-- mm/alloc_tag.c | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-)lib/codetag.c +8
-2@@ -240,7 +240,9 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)if (err <0 ) {kfree (cmod); -return err; +/* -EOPNOTSUPP means we can load the module without its tag. */ +if (err != -EOPNOTSUPP ) +return err; }return 0 ;@@ -388,7 +390,11 @@ void codetag_unload_module(struct module *mod) ++cttype->content_id; }up_write (&cttype->mod_lock); -if (found && cttype->desc.free_section_mem) +/* +* A module whose module_load() returned -EOPNOTSUPP is not +* in the idr but may still hold reserved section memory. +*/ +if (cttype->desc.free_section_mem) cttype->desc.free_section_mem (mod, true); }mutex_unlock (&codetag_lock);mm/alloc_tag.c +4
-1@@ -975,6 +975,10 @@ static int load_module(struct module *mod, struct codetag *start, struct codetagstruct alloc_tag *stop_tag;struct alloc_tag *tag; +/* Profiling disabled: load the module without its tags. */ +if (!mem_profiling_support) +return -EOPNOTSUPP ; +/* percpu counters for core allocations are already statically allocated */ if (!mod)return 0 ; --2.25.1
Re: [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock
In reply to Hao GeQuoted 17 lines · Hao Ge
shutdown_mem_profiling()callsremove_proc_entry()fromreserve_module_tags(), which runs undermod_lockheld for write.remove_proc_entry()waits for readers, and a reader takesmod_lockfor read inallocinfo_start():CPU0 (insmod) CPU1 (read
/proc/allocinfo)
---------------- ----------------------------
reserve_module_tags()down_write(&mod_lock) [held] use_pde() [in_use++] allocinfo_start() down_read(&mod_lock) <- blocks shutdown_mem_profiling() remove_proc_entry() wait for in_use == 0 <- blocks
Quoted 6 lines
Move
remove_proc_entry()to a workqueue.Reported-by Sashiko <[email protected]>Fixes 4835f747d3ed ("alloc_tag: support for page allocation tag compression")Signed-off-by Hao Ge <[email protected]>
Quoted 42 lines
--- mm/alloc_tag.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)mm/alloc_tag.c +9
-2@@ -15,6 +15,7 @@#include <linux/seq_file.h> #include <linux/string_choices.h> #include <linux/vmalloc.h> +#include <linux/workqueue.h> #include <linux/kmemleak.h> #include <uapi/linux/alloc_tag.h> @@ -591,6 +592,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)put_page_tag_ref (handle_new); } +static void remove_allocinfo_file (struct work_struct *work) +{ +remove_proc_entry (ALLOCINFO_FILE_NAME ,NULL ); +} + +static DECLARE_WORK(remove_allocinfo_work, remove_allocinfo_file); +static void shutdown_mem_profiling (bool remove_file) {if (mem_alloc_profiling_enabled ())@@ -600,7 +608,7 @@ static void shutdown_mem_profiling(bool remove_file)return ;if (remove_file) -remove_proc_entry (ALLOCINFO_FILE_NAME ,NULL ); +schedule_work (&remove_allocinfo_work); mem_profiling_support = false; } --2.25.1
Andrew Morton
maintainerQuoted 5 lines · On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <[email protected]> wrote:
Two fixes for issues reported by sashiko:
1. percpu counter leak on modules loaded after profiling is disabled.
2. AB-BA deadlock between module load and/proc/allocinforeaders.
Thanks. AI review asked two questions. One pertinent to your alterations and one pertinent to Suren ;)
https://sashiko.dev/#/patchset/[email protected]
I'll queue the patchset for 7.3-rc1, with a note-to-self.
Hao Ge
reviewerHi Andrew
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <[email protected]> wrote:
Two fixes for issues reported by sashiko:
1. percpu counter leak on modules loaded after profiling is disabled.
2. AB-BA deadlock between module load and/proc/allocinforeaders.
Quoted 6 lines
Thanks. AI review asked two questions. One pertinent to your alterations and one pertinent to Suren ;)
https://sashiko.dev/#/patchset/[email protected]I'll queue the patchset for 7.3-rc1, with a note-to-self.
Thanks for the heads up on the sashiko review questions.
The question on patch 1 (codetag_load_module() error handling) has two parts.
The lost error code issue is already fixed; I sent the patch and you queued it. (Thanks).
For the rollback part:
I've also seen Sashiko flag this same issue on another of my patches. At the moment this case can't actually happen, alloc_tag is our only registered codetag type, and codetag_module_init() cleans up its cmod from the idr on every failure path, so nothing gets left behind.
That said, if we ever add a second codetag type down the line, the problem Sashiko spotted will become real. I will follow up later to refine this logic and make it more robust.
The question on patch 2 (async /proc/allocinfo removal racing with alloc_tag_init() failure):
When I first read it, I think the window is unreachable. It requires alloc_tag_init() to fail after proc_create() succeeded, and a process to open and read /proc/allocinfo in the gap between schedule_work() and the work running on system_wq.
But CONFIG_MEM_ALLOC_PROFILING is a bool, so when enabled alloc_tag is always built in and it cannot be a loadable module. Its module_init(alloc_tag_init) runs inside do_initcalls(), before /init is exec'd. Failures inside alloc_tag_init() are already very unlikely to happen. When the failure happens, no normal userspace exists yet.
That said, I realised the fix would actually be quite simple, we could just move proc_create() to the end of alloc_tag_init().
I am not entirely sure whether we should do this though.
Suren, what is your opinion?
Thanks
Best Regards
Hao
Hao Ge
reviewerHi Suren and Andrew
Update the status of this issue surfaced by Sashiko.
Quoted 28 lines · Hao Ge
Hi Andrew
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <[email protected]> wrote:
Two fixes for issues reported by sashiko:
1. percpu counter leak on modules loaded after profiling is disabled.
2. AB-BA deadlock between module load and/proc/allocinforeaders.Quoted 6 lines
Thanks. AI review asked two questions. One pertinent to your alterations and one pertinent to Suren ;)
https://sashiko.dev/#/patchset/[email protected]I'll queue the patchset for 7.3-rc1, with a note-to-self.
Thanks for the heads up on the sashiko review questions.
The question on patch 1 (
codetag_load_module()error handling) has two parts.The lost error code issue is already fixed; I sent the patch and you queued it. (Thanks).
For the rollback part:
Quoted 9 lines
I've also seen Sashiko flag this same issue on another of my patches. At the moment this case can't actually happen,
alloc_tagis our only registered codetag type, andcodetag_module_init()cleans up its cmod from the idr on every failure path, so nothing gets left behind.That said, if we ever add a second codetag type down the line, the problem Sashiko spotted will become real. I will follow up later to refine this logic and make it more robust.
Daniel also raised this issue
https://lore.kernel.org/all/[email protected]/
I do have the relevant fix ready locally. I plan to hold off on the next batch until we close out this recent chain of fixes. I'll bother you all again when the time comes.
Quoted 14 lines
The question on patch 2 (async
/proc/allocinforemoval racing withalloc_tag_init()failure):When I first read it, I think the window is unreachable. It requires
alloc_tag_init()to fail afterproc_create()succeeded, and a process to open and read/proc/allocinfoin the gap betweenschedule_work()and the work running onsystem_wq.But
CONFIG_MEM_ALLOC_PROFILINGis a bool, so when enabledalloc_tagis always built in and it cannot be a loadable module. Itsmodule_init(alloc_tag_init)runs insidedo_initcalls(), before /init is exec'd. Failures insidealloc_tag_init()are already very unlikely to happen. When the failure happens, no normal userspace exists yet.
Quoted 6 lines
That said, I realised the fix would actually be quite simple, we could just move
proc_create()to the end ofalloc_tag_init().
I am not entirely sure whether we should do this though.Suren, what is your opinion?
I've been thinking about this quite a bit lately. Defensive programming is always welcome — there might be edge cases I haven't considered, or scenarios that could trigger this down the line. Furthermore, if alloc_tag initialization fails, the corresponding sysctl entry serves little purpose.
Besides, I've decided to fold these two patches into this series:
https://lore.kernel.org/all/[email protected]/
This is because Sashiko keeps flagging this percpu leak.
https://lore.kernel.org/all/[email protected]/
And patch 1 addresses exactly this issue.
We'll fold these two patches into that series and let Sashiko run another round of review.
Please kindly help review the folded V10 version.
Thanks
Best Regards
Hao
Quoted 4 lines
Thanks
Best Regards
Hao
Suren Baghdasaryan
maintainerQuoted 83 lines · Hao Ge
Hi Suren and Andrew
Update the status of this issue surfaced by Sashiko.
Quoted 28 lines · Hao Ge
Hi Andrew
Quoted 8 lines · Andrew Morton
Quoted 5 lines · On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <[email protected]> wrote:
Two fixes for issues reported by sashiko:
1. percpu counter leak on modules loaded after profiling is disabled.
2. AB-BA deadlock between module load and/proc/allocinforeaders.Quoted 6 lines
Thanks. AI review asked two questions. One pertinent to your alterations and one pertinent to Suren ;)
https://sashiko.dev/#/patchset/[email protected]I'll queue the patchset for 7.3-rc1, with a note-to-self.
Thanks for the heads up on the sashiko review questions.
The question on patch 1 (
codetag_load_module()error handling) has two parts.The lost error code issue is already fixed; I sent the patch and you queued it. (Thanks).
For the rollback part:
Quoted 9 lines
I've also seen Sashiko flag this same issue on another of my patches. At the moment this case can't actually happen,
alloc_tagis our only registered codetag type, andcodetag_module_init()cleans up its cmod from the idr on every failure path, so nothing gets left behind.That said, if we ever add a second codetag type down the line, the problem Sashiko spotted will become real. I will follow up later to refine this logic and make it more robust.
Daniel also raised this issue
https://lore.kernel.org/all/[email protected]/I do have the relevant fix ready locally. I plan to hold off on the next batch until we close out this recent chain of fixes. I'll bother you all again when the time comes.
Quoted 14 lines
The question on patch 2 (async
/proc/allocinforemoval racing withalloc_tag_init()failure):When I first read it, I think the window is unreachable. It requires
alloc_tag_init()to fail afterproc_create()succeeded, and a process to open and read/proc/allocinfoin the gap betweenschedule_work()and the work running onsystem_wq.But
CONFIG_MEM_ALLOC_PROFILINGis a bool, so when enabledalloc_tagis always built in and it cannot be a loadable module. Itsmodule_init(alloc_tag_init)runs insidedo_initcalls(), before /init is exec'd. Failures insidealloc_tag_init()are already very unlikely to happen. When the failure happens, no normal userspace exists yet.Quoted 6 lines
That said, I realised the fix would actually be quite simple, we could just move
proc_create()to the end ofalloc_tag_init().
I am not entirely sure whether we should do this though.Suren, what is your opinion?
I've been thinking about this quite a bit lately. Defensive programming is always welcome — there might be edge cases I haven't considered, or scenarios that could trigger this down the line. Furthermore, if
alloc_taginitialization fails, the corresponding sysctl entry serves little purpose.Besides, I've decided to fold these two patches into this series:
https://lore.kernel.org/all/[email protected]/
Quoted 8 lines
This is because Sashiko keeps flagging this percpu leak.
https://lore.kernel.org/all/[email protected]/
And patch 1 addresses exactly this issue.We'll fold these two patches into that series and let Sashiko run another round of review.
Please kindly help review the folded V10 version.
Hi Hao,
Sorry for not reviewing earlier version of your patchset and thank you for pursuing this. I saw your v10 submission and will start reviewing it. Due to time limitations, completing the review might take me a couple of days.
Thanks,
Suren.
Quoted 9 lines
Thanks
Best Regards
HaoQuoted 4 lines
Thanks
Best Regards
Hao
Andrew Morton
maintainerQuoted 10 lines · On Tue, 15 Sep 2026 07:36:27 -0700 Suren Baghdasaryan <[email protected]> wrote:
Quoted 9 lines · Hao Ge
Hi Suren and Andrew
Update the status of this issue surfaced by Sashiko.
round of review.
Quoted 7 lines
Quoted 1 line
Please kindly help review the folded V10 version.
Hi Hao,
Sorry for not reviewing earlier version of your patchset and thank you for pursuing this. I saw your v10 submission and will start reviewing it. Due to time limitations, completing the review might take me a couple of days.
As I understand it, this 2-patch series "alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()" will be replaced by the series "alloc_tag and module codetag section fixes" when that's ready.
So I'll drop this series from mm.git.