[PATCH] memcg: Don't call schedule_work when no spinning is allowed

[PATCH] memcg: Don't call schedule_work when no spinning is allowed

[David Stevens]

Memcg charging can be done from any context, but calling schedule_work()
isn't safe from an NMI. If memory.high is breached from a context where
spinning isn't allowed, use irq_work to schedule the reclaim work.

Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
Signed-off-by: David Stevens <stevensd@google.com>
---
 include/linux/memcontrol.h |  1 +
 mm/memcontrol.c            | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 8170bb8066a2..036d973ceca6 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -219,6 +219,7 @@ struct mem_cgroup {
 	spinlock_t	 peaks_lock;
 
 	/* Range enforcement for interrupt charges */
+	struct irq_work high_irq_work;
 	struct work_struct high_work;
 
 #ifdef CONFIG_ZSWAP
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6dc4888a90f3..5e2f749067cb 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2360,6 +2360,11 @@ static void high_work_func(struct work_struct *work)
 	reclaim_high(memcg, MEMCG_CHARGE_BATCH, GFP_KERNEL);
 }
 
+static void high_irq_work_func(struct irq_work *work)
+{
+	schedule_work(&container_of(work, struct mem_cgroup, high_irq_work)->high_work);
+}
+
 /*
  * Clamp the maximum sleep time per allocation batch to 2 seconds. This is
  * enough to still cause a significant slowdown in most cases, while still
@@ -2752,7 +2757,10 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 		/* Don't bother a random interrupted task */
 		if (!in_task()) {
 			if (mem_high) {
-				schedule_work(&memcg->high_work);
+				if (allow_spinning)
+					schedule_work(&memcg->high_work);
+				else
+					irq_work_queue(&memcg->high_irq_work);
 				break;
 			}
 			continue;
@@ -4129,6 +4137,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 		goto fail;
 
 	INIT_WORK(&memcg->high_work, high_work_func);
+	init_irq_work(&memcg->high_irq_work, high_irq_work_func);
 	vmpressure_init(&memcg->vmpressure);
 	INIT_LIST_HEAD(&memcg->memory_peaks);
 	INIT_LIST_HEAD(&memcg->swap_peaks);
@@ -4337,6 +4346,7 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
 		static_branch_dec(&memcg_bpf_enabled_key);
 
 	vmpressure_cleanup(&memcg->vmpressure);
+	irq_work_sync(&memcg->high_irq_work);
 	cancel_work_sync(&memcg->high_work);
 	memcg1_remove_from_trees(memcg);
 	free_shrinker_info(memcg);

base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Andrew Morton]

On Mon, 31 Aug 2026 16:43:39 -0700 David Stevens <stevensd@google.com> wrote:

> Memcg charging can be done from any context, but calling schedule_work()
> isn't safe from an NMI.

Who does that.  bpf, IIRC?

> If memory.high is breached from a context where
> spinning isn't allowed, use irq_work to schedule the reclaim work.
> 
> Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")

Can anything actually hit this?  IOW, should we backport it?

> Signed-off-by: David Stevens <stevensd@google.com>

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Shakeel Butt]

On Mon, Aug 31, 2026 at 04:43:39PM -0700, David Stevens wrote:
> Memcg charging can be done from any context, but calling schedule_work()
> isn't safe from an NMI. If memory.high is breached from a context where
> spinning isn't allowed, use irq_work to schedule the reclaim work.
> 
> Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
> Signed-off-by: David Stevens <stevensd@google.com>

Did you hit this issue or just code inspection? I assume this is the
done_restock code path.

> ---
>  include/linux/memcontrol.h |  1 +
>  mm/memcontrol.c            | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 8170bb8066a2..036d973ceca6 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -219,6 +219,7 @@ struct mem_cgroup {
>  	spinlock_t	 peaks_lock;
>  
>  	/* Range enforcement for interrupt charges */
> +	struct irq_work high_irq_work;

Instead of adding more complexity, let's just return if we can not spin on
done_restock path.

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[David Stevens]

On Mon, Aug 31, 2026 at 5:10 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Mon, Aug 31, 2026 at 04:43:39PM -0700, David Stevens wrote:
> > Memcg charging can be done from any context, but calling schedule_work()
> > isn't safe from an NMI. If memory.high is breached from a context where
> > spinning isn't allowed, use irq_work to schedule the reclaim work.
> >
> > Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
> > Signed-off-by: David Stevens <stevensd@google.com>
>
> Did you hit this issue or just code inspection? I assume this is the
> done_restock code path.

I just found this via code inspection. I spent a little bit trying to
trigger it for real, but the only way I managed was by writing a hacky
driver absuing alloc_pages_nolock().

> > ---
> >  include/linux/memcontrol.h |  1 +
> >  mm/memcontrol.c            | 12 +++++++++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 8170bb8066a2..036d973ceca6 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -219,6 +219,7 @@ struct mem_cgroup {
> >       spinlock_t       peaks_lock;
> >
> >       /* Range enforcement for interrupt charges */
> > +     struct irq_work high_irq_work;
>
> Instead of adding more complexity, let's just return if we can not spin on
> done_restock path.
>

There would be no guarantee that memcg reclaim would ever be
triggered, which also would also stop MEMCG_HIGH events from being
generated. Overall that seems a more serious than just dropping
userspace notifications like is done for MEMCG_MAX.

That said, it is very much an edge case. I can send a patch with the
simpler fix if dropping the events is preferred.

-David

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Michal Hocko]

On Mon 31-08-26 18:04:57, David Stevens wrote:
> On Mon, Aug 31, 2026 at 5:10 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > On Mon, Aug 31, 2026 at 04:43:39PM -0700, David Stevens wrote:
> > > Memcg charging can be done from any context, but calling schedule_work()
> > > isn't safe from an NMI. If memory.high is breached from a context where
> > > spinning isn't allowed, use irq_work to schedule the reclaim work.
> > >
> > > Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
> > > Signed-off-by: David Stevens <stevensd@google.com>
> >
> > Did you hit this issue or just code inspection? I assume this is the
> > done_restock code path.
> 
> I just found this via code inspection. I spent a little bit trying to
> trigger it for real, but the only way I managed was by writing a hacky
> driver absuing alloc_pages_nolock().

Then this is not really a fix but rather a new feature.

> > > ---
> > >  include/linux/memcontrol.h |  1 +
> > >  mm/memcontrol.c            | 12 +++++++++++-
> > >  2 files changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 8170bb8066a2..036d973ceca6 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -219,6 +219,7 @@ struct mem_cgroup {
> > >       spinlock_t       peaks_lock;
> > >
> > >       /* Range enforcement for interrupt charges */
> > > +     struct irq_work high_irq_work;
> >
> > Instead of adding more complexity, let's just return if we can not spin on
> > done_restock path.
> >
> 
> There would be no guarantee that memcg reclaim would ever be
> triggered, which also would also stop MEMCG_HIGH events from being
> generated. Overall that seems a more serious than just dropping
> userspace notifications like is done for MEMCG_MAX.
> 
> That said, it is very much an edge case. I can send a patch with the
> simpler fix if dropping the events is preferred.

Yes, let's go simpler before making this more complex without any actual
user.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[David Stevens]

On Tue, Sep 1, 2026 at 1:18 AM Michal Hocko <mhocko@suse.com> wrote:
>
> On Mon 31-08-26 18:04:57, David Stevens wrote:
> > On Mon, Aug 31, 2026 at 5:10 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> > >
> > > On Mon, Aug 31, 2026 at 04:43:39PM -0700, David Stevens wrote:
> > > > Memcg charging can be done from any context, but calling schedule_work()
> > > > isn't safe from an NMI. If memory.high is breached from a context where
> > > > spinning isn't allowed, use irq_work to schedule the reclaim work.
> > > >
> > > > Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
> > > > Signed-off-by: David Stevens <stevensd@google.com>
> > >
> > > Did you hit this issue or just code inspection? I assume this is the
> > > done_restock code path.
> >
> > I just found this via code inspection. I spent a little bit trying to
> > trigger it for real, but the only way I managed was by writing a hacky
> > driver absuing alloc_pages_nolock().
>
> Then this is not really a fix but rather a new feature.

I spent a bit longer looking, and this can be hit via bpf_arena_alloc_pages().

-David

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Johannes Weiner]

On Mon, Aug 31, 2026 at 06:04:57PM -0700, David Stevens wrote:
> On Mon, Aug 31, 2026 at 5:10 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > On Mon, Aug 31, 2026 at 04:43:39PM -0700, David Stevens wrote:
> > > Memcg charging can be done from any context, but calling schedule_work()
> > > isn't safe from an NMI. If memory.high is breached from a context where
> > > spinning isn't allowed, use irq_work to schedule the reclaim work.
> > >
> > > Fixes: 3ac4638a734a ("memcg: make memcg_rstat_updated nmi safe")
> > > Signed-off-by: David Stevens <stevensd@google.com>
> >
> > Did you hit this issue or just code inspection? I assume this is the
> > done_restock code path.
> 
> I just found this via code inspection. I spent a little bit trying to
> trigger it for real, but the only way I managed was by writing a hacky
> driver absuing alloc_pages_nolock().
> 
> > > ---
> > >  include/linux/memcontrol.h |  1 +
> > >  mm/memcontrol.c            | 12 +++++++++++-
> > >  2 files changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 8170bb8066a2..036d973ceca6 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -219,6 +219,7 @@ struct mem_cgroup {
> > >       spinlock_t       peaks_lock;
> > >
> > >       /* Range enforcement for interrupt charges */
> > > +     struct irq_work high_irq_work;
> >
> > Instead of adding more complexity, let's just return if we can not spin on
> > done_restock path.
> >
> 
> There would be no guarantee that memcg reclaim would ever be
> triggered, which also would also stop MEMCG_HIGH events from being
> generated. Overall that seems a more serious than just dropping
> userspace notifications like is done for MEMCG_MAX.

I'm leaning that way too. It's an indefinite error, and it's a freely
programmable surface.

IMO, a few lines of relatively straight-forward, self-explanatory code
is better than code that needs a comment and leaves a problem that
somebody in the future might run into.

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Michal Hocko]

On Tue 01-09-26 10:25:52, Johannes Weiner wrote:
> On Mon, Aug 31, 2026 at 06:04:57PM -0700, David Stevens wrote:
[...]
> > There would be no guarantee that memcg reclaim would ever be
> > triggered, which also would also stop MEMCG_HIGH events from being
> > generated. Overall that seems a more serious than just dropping
> > userspace notifications like is done for MEMCG_MAX.
> 
> I'm leaning that way too. It's an indefinite error, and it's a freely
> programmable surface.

I am really curious about the indefinite error side of things. It has
been my understanding that these NMI safe charges are a) rare and b)
there is userspace running so eventually any discrepancies would
resolve so the excess is temporary.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] memcg: Don't call schedule_work when no spinning is allowed

[Johannes Weiner]

On Tue, Sep 01, 2026 at 05:42:30PM +0200, Michal Hocko wrote:
> On Tue 01-09-26 10:25:52, Johannes Weiner wrote:
> > On Mon, Aug 31, 2026 at 06:04:57PM -0700, David Stevens wrote:
> [...]
> > > There would be no guarantee that memcg reclaim would ever be
> > > triggered, which also would also stop MEMCG_HIGH events from being
> > > generated. Overall that seems a more serious than just dropping
> > > userspace notifications like is done for MEMCG_MAX.
> > 
> > I'm leaning that way too. It's an indefinite error, and it's a freely
> > programmable surface.
> 
> I am really curious about the indefinite error side of things. It has
> been my understanding that these NMI safe charges are a) rare and b)
> there is userspace running so eventually any discrepancies would
> resolve so the excess is temporary.

So I think the question is what limits the error in both space and
time. When you say it's rare and userspace fixes it, it basically
means the answer is: luck of the common case.

But that doesn't help the worst case that can be triggered.

Like I said, if we need to have code to handle that !allow_spinning
case anyway, I'd rather just have a few lines of working code than a
(lengthy) comment explaining the luck of the common case.