mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
@ 2026-08-17  6:27 Hao Ge
  2026-08-17  6:27 ` [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Hao Ge @ 2026-08-17  6:27 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton; +Cc: linux-kernel, linux-mm

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/20260812054105.102637-3-hao.ge@linux.dev/
[2]: https://lore.kernel.org/all/20260813093421.135230-1-hao.ge@linux.dev/

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled
  2026-08-17  6:27 [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Hao Ge
@ 2026-08-17  6:27 ` Hao Ge
  2026-08-24 16:44   ` Suren Baghdasaryan
  2026-08-17  6:27 ` [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
  2026-08-27  3:39 ` [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Andrew Morton
  2 siblings, 1 reply; 11+ messages in thread
From: Hao Ge @ 2026-08-17  6:27 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton
  Cc: linux-kernel, linux-mm, Hao Ge, Sashiko, stable

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.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: stable@vger.kernel.org
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 lib/codetag.c  | 10 ++++++++--
 mm/alloc_tag.c |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/codetag.c b/lib/codetag.c
index a9cda4c962a3..a0b600720afc 100644
--- a/lib/codetag.c
+++ b/lib/codetag.c
@@ -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);
diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 52aece27b00e..80eccaca665a 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -975,6 +975,10 @@ 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 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock
  2026-08-17  6:27 [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Hao Ge
  2026-08-17  6:27 ` [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
@ 2026-08-17  6:27 ` Hao Ge
  2026-08-24 16:46   ` Suren Baghdasaryan
  2026-08-27  3:39 ` [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Andrew Morton
  2 siblings, 1 reply; 11+ messages in thread
From: Hao Ge @ 2026-08-17  6:27 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton
  Cc: linux-kernel, linux-mm, Hao Ge, Sashiko, stable

shutdown_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         <- blocks

Move remove_proc_entry() to a workqueue.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 mm/alloc_tag.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 80eccaca665a..b1d48532a25a 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled
  2026-08-17  6:27 ` [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
@ 2026-08-24 16:44   ` Suren Baghdasaryan
  0 siblings, 0 replies; 11+ messages in thread
From: Suren Baghdasaryan @ 2026-08-24 16:44 UTC (permalink / raw)
  To: Hao Ge
  Cc: Kent Overstreet, Andrew Morton, linux-kernel, linux-mm, Sashiko, stable

On Sun, Aug 16, 2026 at 11:26 PM 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 -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.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Cc: stable@vger.kernel.org
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Hao Ge <hao.ge@linux.dev>

Acked-by: Suren Baghdasaryan <surenb@google.com>

> ---
>  lib/codetag.c  | 10 ++++++++--
>  mm/alloc_tag.c |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/codetag.c b/lib/codetag.c
> index a9cda4c962a3..a0b600720afc 100644
> --- a/lib/codetag.c
> +++ b/lib/codetag.c
> @@ -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);
> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 52aece27b00e..80eccaca665a 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -975,6 +975,10 @@ 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 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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock
  2026-08-17  6:27 ` [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
@ 2026-08-24 16:46   ` Suren Baghdasaryan
  0 siblings, 0 replies; 11+ messages in thread
From: Suren Baghdasaryan @ 2026-08-24 16:46 UTC (permalink / raw)
  To: Hao Ge
  Cc: Kent Overstreet, Andrew Morton, linux-kernel, linux-mm, Sashiko, stable

On Sun, Aug 16, 2026 at 11:27 PM Hao Ge <hao.ge@linux.dev> wrote:
>
> shutdown_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         <- blocks
>
> Move remove_proc_entry() to a workqueue.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>

Acked-by: Suren Baghdasaryan <surenb@google.com>

> ---
>  mm/alloc_tag.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 80eccaca665a..b1d48532a25a 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
  2026-08-17  6:27 [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Hao Ge
  2026-08-17  6:27 ` [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
  2026-08-17  6:27 ` [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
@ 2026-08-27  3:39 ` Andrew Morton
  2026-08-28  3:11   ` Hao Ge
  2 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2026-08-27  3:39 UTC (permalink / raw)
  To: Hao Ge; +Cc: Suren Baghdasaryan, Kent Overstreet, linux-kernel, linux-mm

On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <hao.ge@linux.dev> 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/allocinfo readers.
> 

Thanks.  AI review asked two questions.  One pertinent to your
alterations and one pertinent to Suren ;)

	https://sashiko.dev/#/patchset/20260817062726.106511-1-hao.ge@linux.dev

I'll queue the patchset for 7.3-rc1, with a note-to-self.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
  2026-08-27  3:39 ` [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Andrew Morton
@ 2026-08-28  3:11   ` Hao Ge
  2026-09-15  2:50     ` Hao Ge
  0 siblings, 1 reply; 11+ messages in thread
From: Hao Ge @ 2026-08-28  3:11 UTC (permalink / raw)
  To: Andrew Morton, Suren Baghdasaryan; +Cc: Kent Overstreet, linux-kernel, linux-mm

Hi Andrew

On 2026/8/27 11:39, Andrew Morton wrote:
> On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <hao.ge@linux.dev> 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/allocinfo readers.
>>
> 
> Thanks.  AI review asked two questions.  One pertinent to your
> alterations and one pertinent to Suren ;)
> 
> 	https://sashiko.dev/#/patchset/20260817062726.106511-1-hao.ge@linux.dev
> 
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
  2026-08-28  3:11   ` Hao Ge
@ 2026-09-15  2:50     ` Hao Ge
  2026-09-15 14:36       ` Suren Baghdasaryan
  0 siblings, 1 reply; 11+ messages in thread
From: Hao Ge @ 2026-09-15  2:50 UTC (permalink / raw)
  To: Andrew Morton, Suren Baghdasaryan; +Cc: Kent Overstreet, linux-kernel, linux-mm

Hi Suren and Andrew


Update the status of this issue surfaced by Sashiko.


On 2026/8/28 11:11, Hao Ge wrote:
> Hi Andrew
> 
> On 2026/8/27 11:39, Andrew Morton wrote:
>> On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <hao.ge@linux.dev> 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/allocinfo readers.
>>>
>>
>> Thanks.  AI review asked two questions.  One pertinent to your
>> alterations and one pertinent to Suren ;)
>>
>> 	https://sashiko.dev/#/patchset/20260817062726.106511-1-hao.ge@linux.dev
>>
>> 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.
> 

Daniel also raised this issue
https://lore.kernel.org/all/675259f9-c093-439c-a411-1937b23ddaa2@linux.dev/

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.

> 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?
>

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/20260908092412.115953-1-hao.ge@linux.dev/

This is because Sashiko keeps flagging this percpu leak.
https://lore.kernel.org/all/20260908094736.2B1A61F00A3A@smtp.kernel.org/
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

> Thanks
> Best Regards
> Hao
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
  2026-09-15  2:50     ` Hao Ge
@ 2026-09-15 14:36       ` Suren Baghdasaryan
  0 siblings, 0 replies; 11+ messages in thread
From: Suren Baghdasaryan @ 2026-09-15 14:36 UTC (permalink / raw)
  To: Hao Ge; +Cc: Andrew Morton, Kent Overstreet, linux-kernel, linux-mm

On Mon, Sep 14, 2026 at 7:49 PM Hao Ge <hao.ge@linux.dev> wrote:
>
> Hi Suren and Andrew
>
>
> Update the status of this issue surfaced by Sashiko.
>
>
> On 2026/8/28 11:11, Hao Ge wrote:
> > Hi Andrew
> >
> > On 2026/8/27 11:39, Andrew Morton wrote:
> >> On Mon, 17 Aug 2026 14:27:24 +0800 Hao Ge <hao.ge@linux.dev> 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/allocinfo readers.
> >>>
> >>
> >> Thanks.  AI review asked two questions.  One pertinent to your
> >> alterations and one pertinent to Suren ;)
> >>
> >>      https://sashiko.dev/#/patchset/20260817062726.106511-1-hao.ge@linux.dev
> >>
> >> 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.
> >
>
> Daniel also raised this issue
> https://lore.kernel.org/all/675259f9-c093-439c-a411-1937b23ddaa2@linux.dev/
>
> 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.
>
> > 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?
> >
>
> 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/20260908092412.115953-1-hao.ge@linux.dev/
>
> This is because Sashiko keeps flagging this percpu leak.
> https://lore.kernel.org/all/20260908094736.2B1A61F00A3A@smtp.kernel.org/
> 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.

>
> Thanks
> Best Regards
> Hao
>
> > Thanks
> > Best Regards
> > Hao
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
@ 2026-08-17  6:27 Hao Ge
  0 siblings, 0 replies; 11+ messages in thread
From: Hao Ge @ 2026-08-17  6:27 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton; +Cc: linux-kernel, linux-mm

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/20260812054105.102637-3-hao.ge@linux.dev/
[2]: https://lore.kernel.org/all/20260813093421.135230-1-hao.ge@linux.dev/

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
@ 2026-08-17  6:26 Hao Ge
  0 siblings, 0 replies; 11+ messages in thread
From: Hao Ge @ 2026-08-17  6:26 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton; +Cc: linux-kernel, linux-mm

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/20260812054105.102637-3-hao.ge@linux.dev/
[2]: https://lore.kernel.org/all/20260813093421.135230-1-hao.ge@linux.dev/

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-15 14:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-17  6:27 [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Hao Ge
2026-08-17  6:27 ` [PATCH 1/2] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
2026-08-24 16:44   ` Suren Baghdasaryan
2026-08-17  6:27 ` [PATCH 2/2] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
2026-08-24 16:46   ` Suren Baghdasaryan
2026-08-27  3:39 ` [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling() Andrew Morton
2026-08-28  3:11   ` Hao Ge
2026-09-15  2:50     ` Hao Ge
2026-09-15 14:36       ` Suren Baghdasaryan
  -- strict thread matches above, loose matches on Subject: below --
2026-08-17  6:27 Hao Ge
2026-08-17  6:26 Hao Ge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®