include/linux/memcontrol.h | 1 + mm/memcontrol.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
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-googOn 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>
This is in mm-unstable and breaks the build for me :)
In general - please don't rely on implicit header includes, if you're using
functions from a header that's not included, add the include because your local
config might happen to import it but another one might be broken.
$ make -j $(nproc) LLVM=1 mm/memcontrol.o
DESCEND objtool
CC arch/x86/kernel/asm-offsets.s
UPD include/generated/asm-offsets.h
CC kernel/sched/rq-offsets.s
UPD include/generated/rq-offsets.h
CALL scripts/checksyscalls.sh
CC mm/memcontrol.o
mm/memcontrol.c:2859:6: error: call to undeclared function 'irq_work_queue'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
2859 | irq_work_queue(&memcg->high_irq_work);
| ^
mm/memcontrol.c:4234:2: error: call to undeclared function 'init_irq_work'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
4234 | init_irq_work(&memcg->high_irq_work, high_irq_work_func);
| ^
mm/memcontrol.c:4443:2: error: call to undeclared function 'irq_work_sync'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
4443 | irq_work_sync(&memcg->high_irq_work);
| ^
3 errors generated.
make[3]: *** [scripts/Makefile.build:290: mm/memcontrol.o] Error 1
make[2]: *** [scripts/Makefile.build:551: mm] Error 2
make[1]: *** [/home/lorenzo/kerndev/kernels/mm/Makefile:2229: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2
> ---
> 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
You need to add:
#include <linux/irq_work.h> in this file.
> @@ -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
>
>
--
Cheers, Lorenzo
On Fri, Sep 4, 2026 at 9:25 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> 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>
>
> This is in mm-unstable and breaks the build for me :)
>
> In general - please don't rely on implicit header includes, if you're using
> functions from a header that's not included, add the include because your local
> config might happen to import it but another one might be broken.
Sorry about that, I'll be sure to be more careful in the future. I've
sent out a v2 with the missing includes.
-David
On Fri, Sep 04, 2026 at 10:33:32AM -0700, David Stevens wrote:
> On Fri, Sep 4, 2026 at 9:25 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> 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>
> >
> > This is in mm-unstable and breaks the build for me :)
> >
> > In general - please don't rely on implicit header includes, if you're using
> > functions from a header that's not included, add the include because your local
> > config might happen to import it but another one might be broken.
>
> Sorry about that, I'll be sure to be more careful in the future. I've
> sent out a v2 with the missing includes.
Thanks!
The kernel headers are a real mess so it's entirely understandable :>) (I think
Matthew is doing some work to improve things on this, or at least some of them).
>
> -David
--
Cheers, Lorenzo
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.
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
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.
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
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.
On Tue 01-09-26 16:59:46, Johannes Weiner wrote: > 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. Right. I was asking because so far we are trying these allocations as more or less trusted (we do allow them to breach the high limit without any pushback). If there are known scenarios where this could run away then we might need to re-evaluate that. Async reclaim might be just too late in those cases. Anway... > 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. Fair enough. A jump through irq work is not that bad from the complexity POV. So you've convinced me Acked-by: Michal Hocko <mhocko@suse.com> Thanks! -- Michal Hocko SUSE Labs
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
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
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>
© 2016 - 2026 Red Hat, Inc.