Re: [PATCH v2 0/2] alloc_tag: fix a leak and a deadlock around shutdown_mem_profiling()
From: Suren Baghdasaryan
Date: Tue Sep 15 2026 - 11:16:15 EST
On Mon, Sep 14, 2026 at 7:49 PM Hao Ge <hao.ge@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx
> >>
> >> 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@xxxxxxxxx/
>
> 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@xxxxxxxxx/
>
> This is because Sashiko keeps flagging this percpu leak.
> https://lore.kernel.org/all/20260908094736.2B1A61F00A3A@xxxxxxxxxxxxxxx/
> 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
> >