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

[PATCH v2] 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")
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: David Stevens <stevensd@google.com>
---
v2:
  - Added missing includes reported by Lorenzo and kernel test robot
  - Added Acked-by

 include/linux/memcontrol.h |  2 ++
 mm/memcontrol.c            | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 8170bb8066a2..4a5ef0aba475 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -23,6 +23,7 @@
 #include <linux/writeback.h>
 #include <linux/page-flags.h>
 #include <linux/shrinker.h>
+#include <linux/irq_work_types.h>
 
 struct mem_cgroup;
 struct obj_cgroup;
@@ -219,6 +220,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..0e8b302ca9ad 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -62,6 +62,7 @@
 #include <linux/seq_buf.h>
 #include <linux/sched/isolation.h>
 #include <linux/kmemleak.h>
+#include <linux/irq_work.h>
 #include "internal.h"
 #include "swap_table.h"
 #include <net/sock.h>
@@ -2360,6 +2361,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 +2758,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 +4138,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 +4347,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.979.g7e5102b832-goog

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

[Johannes Weiner]

On Fri, Sep 04, 2026 at 10:31:45AM -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")
> Acked-by: Michal Hocko <mhocko@suse.com>
> Signed-off-by: David Stevens <stevensd@google.com>

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

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

[Shakeel Butt]

On Fri, Sep 04, 2026 at 10:31:45AM -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")
> Acked-by: Michal Hocko <mhocko@suse.com>
> Signed-off-by: David Stevens <stevensd@google.com>
> ---
> v2:
>   - Added missing includes reported by Lorenzo and kernel test robot
>   - Added Acked-by
> 
>  include/linux/memcontrol.h |  2 ++
>  mm/memcontrol.c            | 13 ++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 8170bb8066a2..4a5ef0aba475 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -23,6 +23,7 @@
>  #include <linux/writeback.h>
>  #include <linux/page-flags.h>
>  #include <linux/shrinker.h>
> +#include <linux/irq_work_types.h>
>  
>  struct mem_cgroup;
>  struct obj_cgroup;
> @@ -219,6 +220,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..0e8b302ca9ad 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -62,6 +62,7 @@
>  #include <linux/seq_buf.h>
>  #include <linux/sched/isolation.h>
>  #include <linux/kmemleak.h>
> +#include <linux/irq_work.h>
>  #include "internal.h"
>  #include "swap_table.h"
>  #include <net/sock.h>
> @@ -2360,6 +2361,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 +2758,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 +4138,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 +4347,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);

On RT kernels, this will put rcu grace period here while we are holding the
cgroup_mutex. Easy fix would be to use IRQ_WORK_INIT_HARD instead of
init_irq_work() in mem_cgroup_alloc.

Something like:
	memcg->high_irq_work = IRQ_WORK_INIT_HARD(high_irq_work_func);

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

[David Stevens]

On Fri, Sep 4, 2026 at 12:03 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Fri, Sep 04, 2026 at 10:31:45AM -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")
> > Acked-by: Michal Hocko <mhocko@suse.com>
> > Signed-off-by: David Stevens <stevensd@google.com>
> > ---
> > v2:
> >   - Added missing includes reported by Lorenzo and kernel test robot
> >   - Added Acked-by
> >
> >  include/linux/memcontrol.h |  2 ++
> >  mm/memcontrol.c            | 13 ++++++++++++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 8170bb8066a2..4a5ef0aba475 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -23,6 +23,7 @@
> >  #include <linux/writeback.h>
> >  #include <linux/page-flags.h>
> >  #include <linux/shrinker.h>
> > +#include <linux/irq_work_types.h>
> >
> >  struct mem_cgroup;
> >  struct obj_cgroup;
> > @@ -219,6 +220,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..0e8b302ca9ad 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -62,6 +62,7 @@
> >  #include <linux/seq_buf.h>
> >  #include <linux/sched/isolation.h>
> >  #include <linux/kmemleak.h>
> > +#include <linux/irq_work.h>
> >  #include "internal.h"
> >  #include "swap_table.h"
> >  #include <net/sock.h>
> > @@ -2360,6 +2361,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 +2758,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 +4138,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 +4347,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);
>
> On RT kernels, this will put rcu grace period here while we are holding the
> cgroup_mutex. Easy fix would be to use IRQ_WORK_INIT_HARD instead of
> init_irq_work() in mem_cgroup_alloc.
>
> Something like:
>         memcg->high_irq_work = IRQ_WORK_INIT_HARD(high_irq_work_func);
>

This executes as part of css_free_rwork_fn(), so it isn't under cgroup_mutex.

-David

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

[Shakeel Butt]

On Fri, Sep 04, 2026 at 03:15:54PM -0700, David Stevens wrote:
> On Fri, Sep 4, 2026 at 12:03 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > On Fri, Sep 04, 2026 at 10:31:45AM -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")
> > > Acked-by: Michal Hocko <mhocko@suse.com>
> > > Signed-off-by: David Stevens <stevensd@google.com>
> > > ---
> > > v2:
> > >   - Added missing includes reported by Lorenzo and kernel test robot
> > >   - Added Acked-by
> > >
> > >  include/linux/memcontrol.h |  2 ++
> > >  mm/memcontrol.c            | 13 ++++++++++++-
> > >  2 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 8170bb8066a2..4a5ef0aba475 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/writeback.h>
> > >  #include <linux/page-flags.h>
> > >  #include <linux/shrinker.h>
> > > +#include <linux/irq_work_types.h>
> > >
> > >  struct mem_cgroup;
> > >  struct obj_cgroup;
> > > @@ -219,6 +220,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..0e8b302ca9ad 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -62,6 +62,7 @@
> > >  #include <linux/seq_buf.h>
> > >  #include <linux/sched/isolation.h>
> > >  #include <linux/kmemleak.h>
> > > +#include <linux/irq_work.h>
> > >  #include "internal.h"
> > >  #include "swap_table.h"
> > >  #include <net/sock.h>
> > > @@ -2360,6 +2361,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 +2758,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 +4138,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 +4347,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);
> >
> > On RT kernels, this will put rcu grace period here while we are holding the
> > cgroup_mutex. Easy fix would be to use IRQ_WORK_INIT_HARD instead of
> > init_irq_work() in mem_cgroup_alloc.
> >
> > Something like:
> >         memcg->high_irq_work = IRQ_WORK_INIT_HARD(high_irq_work_func);
> >
> 
> This executes as part of css_free_rwork_fn(), so it isn't under cgroup_mutex.

Oh yes, css_free_rwork_fn does not take cgroup_mutex. Though synchronize_rcu()
is still something to avoid but not a blocker.

You can add:

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>