memcg: make memcg stats irq safe

PATCH In review Shakeel Butt 4 participants 16 messages May 14, 2025 – May 17, 2025

Subsystems (6)

Memory management BPF cgroup MGLRU +2 more subsystems

lore

Share & tools

lore

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #0 lore raw

v2 cover memcg: make memcg stats irq safe

This series converts memcg stats to be irq safe i.e. memcg stats can be updated in any context (task, softirq or hardirq) without disabling the irqs. This is still not nmi-safe on all architectures but after this series converting memcg charging and stats nmi-safe will be easier.

Changes since v1:
-----------------
1. Rebased on mm-new.
2. Cleanups in first patch as suggested by Vlastimil.

Changes since RFC[1]:
--------------------
1. Rebased on next-20250513 (mm tree has some conflicts with cgroup tree).
2. Does not depend of nmi-safe memcg series [2].
3. Made memcg_rstat_updated re-entrant using this_cpu_* ops as suggested by Vlastimil.
4. Fixes some spelling mistakes as suggested by Vlastimil.
5. Rearranged the 6th and 7th patch as suggested by Vlastimil.

Link http://lore.kernel.org/[email protected] [1] open here

Link http://lore.kernel.org/[email protected] [2] open here

Shakeel Butt (7):
memcg: memcg_rstat_updated re-entrant safe against irqs
memcg: move preempt disable to callers of memcg_rstat_updated
memcg: make mod_memcg_state re-entrant safe against irqs
memcg: make count_memcg_events re-entrant safe against irqs
memcg: make __mod_memcg_lruvec_state re-entrant safe against irqs
memcg: no stock lock for cpu hot-unplug
memcg: objcg stock trylock without irq disabling

 include/linux/memcontrol.h |  41 +--------
 mm/memcontrol-v1.c         |   6 +-
 mm/memcontrol.c            | 170 +++++++++++++++----------------------
 mm/swap.c                  |   8 +-
 mm/vmscan.c                |  14 +--
 5 files changed, 86 insertions(+), 153 deletions(-)

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #1 lore raw

PATCH v2 1/7 memcg: memcg_rstat_updated re-entrant safe against irqs

The function memcg_rstat_updated() is used to track the memcg stats updates for optimizing the flushes. At the moment, it is not re-entrant safe and the callers disabled irqs before calling. However to achieve the goal of updating memcg stats without irqs, memcg_rstat_updated() needs to be re-entrant safe against irqs.

This patch makes memcg_rstat_updated() re-entrant safe using this_cpu_* ops. On archs with CONFIG_ARCH_HAS_NMI_SAFE_THIS_CPU_OPS, this patch is also making memcg_rstat_updated() nmi safe.

Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

mm/memcontrol.c +17 ~~-11~~

index 89476a71a18d..2464a58fbf17 100644
@@ -505,8 +505,8 @@ struct memcg_vmstats_percpu {
 	unsigned int			stats_updates;
 
 	/* Cached pointers for fast iteration in memcg_rstat_updated() */
-	struct memcg_vmstats_percpu	*parent;
-	struct memcg_vmstats		*vmstats;
+	struct memcg_vmstats_percpu __percpu	*parent_pcpu;
+	struct memcg_vmstats			*vmstats;
 
 	/* The above should fit a single cacheline for memcg_rstat_updated() */

@@ -588,16 +588,21 @@ static bool memcg_vmstats_needs_flush(struct memcg_vmstats *vmstats)
 
 static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 {
+	struct memcg_vmstats_percpu __percpu *statc_pcpu;
 	struct memcg_vmstats_percpu *statc;
-	int cpu = smp_processor_id();
+	int cpu;
 	unsigned int stats_updates;
 
 	if (!val)
 		return;
 
+	/* Don't assume callers have preemption disabled. */
+	cpu = get_cpu();
+
 	cgroup_rstat_updated(memcg->css.cgroup, cpu);
-	statc = this_cpu_ptr(memcg->vmstats_percpu);
-	for (; statc; statc = statc->parent) {
+	statc_pcpu = memcg->vmstats_percpu;
+	for (; statc_pcpu; statc_pcpu = statc->parent_pcpu) {
+		statc = this_cpu_ptr(statc_pcpu);
 		/*
 		 * If @memcg is already flushable then all its ancestors are
 		 * flushable as well and also there is no need to increase

@@ -606,14 +611,15 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 		if (memcg_vmstats_needs_flush(statc->vmstats))
 			break;
 
-		stats_updates = READ_ONCE(statc->stats_updates) + abs(val);
-		WRITE_ONCE(statc->stats_updates, stats_updates);
+		stats_updates = this_cpu_add_return(statc_pcpu->stats_updates,
+						    abs(val));
 		if (stats_updates < MEMCG_CHARGE_BATCH)
 			continue;
 
+		stats_updates = this_cpu_xchg(statc_pcpu->stats_updates, 0);
 		atomic64_add(stats_updates, &statc->vmstats->stats_updates);
-		WRITE_ONCE(statc->stats_updates, 0);
 	}
+	put_cpu();
 }
 
 static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)

@@ -3691,7 +3697,7 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
 
 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 {
-	struct memcg_vmstats_percpu *statc, *pstatc;
+	struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
 	struct mem_cgroup *memcg;
 	int node, cpu;
 	int __maybe_unused i;

@@ -3722,9 +3728,9 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 
 	for_each_possible_cpu(cpu) {
 		if (parent)
-			pstatc = per_cpu_ptr(parent->vmstats_percpu, cpu);
+			pstatc_pcpu = parent->vmstats_percpu;
 		statc = per_cpu_ptr(memcg->vmstats_percpu, cpu);
-		statc->parent = parent ? pstatc : NULL;
+		statc->parent_pcpu = parent ? pstatc_pcpu : NULL;
 		statc->vmstats = memcg->vmstats;
 	}

Signature · 1 line

2.47.1

Lorenzo Stoakes

2025-05-15 12:47 UTC #2 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Shakeel - This breaks the build in mm-new for me:

CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,

                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:

mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’

   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~

mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?

 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu

mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in

The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:

From 28275e5d054506746d310cf5ebd1fafdb0881dba Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <[email protected]>
Date: Thu, 15 May 2025 13:43:46 +0100
Subject: [PATCH] fix

Signed-off-by Lorenzo Stoakes <[email protected]>

---
 mm/memcontrol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

mm/memcontrol.c +2 -1

index 2464a58fbf17..40fcc2259e5f 100644
@@ -3697,7 +3697,8 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
 
 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 {
-	struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
+	struct memcg_vmstats_percpu *statc;
+	struct memcg_vmstats_percpu __percpu *pstatc_pcpu;
 	struct mem_cgroup *memcg;
 	int node, cpu;
 	int __maybe_unused i;

--
2.49.0

I have duplicated this again at the end of this mail for easy application.

Could we get this fix in or drop the series so the build is fixed for mm-new? Thanks!

Quoted 97 lines · Shakeel Butt

This patch makes memcg_rstat_updated() re-entrant safe using this_cpu_* ops. On archs with CONFIG_ARCH_HAS_NMI_SAFE_THIS_CPU_OPS, this patch is also making memcg_rstat_updated() nmi safe.

Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

mm/memcontrol.c +17 ~~-12~~

index 89476a71a18d..2464a58fbf17 100644
@@ -505,8 +505,8 @@ struct memcg_vmstats_percpu {
 	unsigned int			stats_updates;
 
 	/* Cached pointers for fast iteration in memcg_rstat_updated() */
-	struct memcg_vmstats_percpu	*parent;
-	struct memcg_vmstats		*vmstats;
+	struct memcg_vmstats_percpu __percpu	*parent_pcpu;
+	struct memcg_vmstats			*vmstats;
 
 	/* The above should fit a single cacheline for memcg_rstat_updated() */

@@ -588,16 +588,21 @@ static bool memcg_vmstats_needs_flush(struct memcg_vmstats *vmstats)
 
 static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 {
+	struct memcg_vmstats_percpu __percpu *statc_pcpu;
 	struct memcg_vmstats_percpu *statc;
-	int cpu = smp_processor_id();
+	int cpu;
 	unsigned int stats_updates;
 
 	if (!val)
 		return;
 
+	/* Don't assume callers have preemption disabled. */
+	cpu = get_cpu();
+
 	cgroup_rstat_updated(memcg->css.cgroup, cpu);
-	statc = this_cpu_ptr(memcg->vmstats_percpu);
-	for (; statc; statc = statc->parent) {
+	statc_pcpu = memcg->vmstats_percpu;
+	for (; statc_pcpu; statc_pcpu = statc->parent_pcpu) {
+		statc = this_cpu_ptr(statc_pcpu);
 		/*
 		 * If @memcg is already flushable then all its ancestors are
 		 * flushable as well and also there is no need to increase

@@ -606,14 +611,15 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 		if (memcg_vmstats_needs_flush(statc->vmstats))
 			break;
 
-		stats_updates = READ_ONCE(statc->stats_updates) + abs(val);
-		WRITE_ONCE(statc->stats_updates, stats_updates);
+		stats_updates = this_cpu_add_return(statc_pcpu->stats_updates,
+						    abs(val));
 		if (stats_updates < MEMCG_CHARGE_BATCH)
 			continue;
 
+		stats_updates = this_cpu_xchg(statc_pcpu->stats_updates, 0);
 		atomic64_add(stats_updates, &statc->vmstats->stats_updates);
-		WRITE_ONCE(statc->stats_updates, 0);
 	}
+	put_cpu();
 }
 
 static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)

@@ -3691,7 +3697,7 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
 
 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 {
-	struct memcg_vmstats_percpu *statc, *pstatc;
+	struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
 	struct mem_cgroup *memcg;
 	int node, cpu;
 	int __maybe_unused i;

@@ -3722,9 +3728,9 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 
 	for_each_possible_cpu(cpu) {
 		if (parent)
-			pstatc = per_cpu_ptr(parent->vmstats_percpu, cpu);
+			pstatc_pcpu = parent->vmstats_percpu;
 		statc = per_cpu_ptr(memcg->vmstats_percpu, cpu);
-		statc->parent = parent ? pstatc : NULL;
+		statc->parent_pcpu = parent ? pstatc_pcpu : NULL;
 		statc->vmstats = memcg->vmstats;
 	}
 
--

Quoted 3 lines

2.47.1

----8<----
From 28275e5d054506746d310cf5ebd1fafdb0881dba Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <[email protected]>
Date: Thu, 15 May 2025 13:43:46 +0100
Subject: [PATCH] fix

Signed-off-by Lorenzo Stoakes <[email protected]>

---
 mm/memcontrol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

mm/memcontrol.c +2 -1

index 2464a58fbf17..40fcc2259e5f 100644
@@ -3697,7 +3697,8 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
 
 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 {
-	struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
+	struct memcg_vmstats_percpu *statc;
+	struct memcg_vmstats_percpu __percpu *pstatc_pcpu;
 	struct mem_cgroup *memcg;
 	int node, cpu;
 	int __maybe_unused i;

--
2.49.0

Shakeel Butt

author maintainer

2025-05-15 14:31 UTC #3 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Quoted 31 lines · Lorenzo Stoakes

Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?

Quoted 8 lines

 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:

Which compiler (and version) is this? Thanks for the fix.

Lorenzo Stoakes

2025-05-15 14:53 UTC #4 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Quoted 38 lines · Shakeel Butt

Quoted 36 lines · Lorenzo Stoakes
Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?
 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in

Quoted 5 lines

Quoted 3 lines
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:
Which compiler (and version) is this? Thanks for the fix.

gcc 15, but apparently 13, 14 also fail. It seems independent of config.

Shakeel Butt

author maintainer

2025-05-15 15:22 UTC #5 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Quoted 39 lines · Lorenzo Stoakes

Quoted 38 lines · Shakeel Butt
Quoted 36 lines · Lorenzo Stoakes
Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?
 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:
Which compiler (and version) is this? Thanks for the fix.
gcc 15, but apparently 13, 14 also fail. It seems independent of config.

Thanks, somehow it works with gcc 11.5.0.

Lorenzo Stoakes

2025-05-15 15:29 UTC #6 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Quoted 44 lines · Shakeel Butt

Quoted 43 lines · Lorenzo Stoakes
Quoted 42 lines · Shakeel Butt
Quoted 39 lines · Lorenzo Stoakes
Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?
 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Which compiler (and version) is this? Thanks for the fix.
gcc 15, but apparently 13, 14 also fail. It seems independent of config.
Thanks, somehow it works with gcc 11.5.0.

That is... both extremely bizarre, and VERY gnu... haha

Vlastimil Babka

2025-05-15 14:57 UTC #7 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

In reply to Shakeel Butt

Quoted 38 lines · Shakeel Butt

Quoted 36 lines · Lorenzo Stoakes
Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?
 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in

Quoted 5 lines

Quoted 3 lines
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:
Which compiler (and version) is this? Thanks for the fix.

Hm right I see the same errors with gcc 7, 13, 14, 15 but not with clang.

Shakeel Butt

author maintainer

2025-05-15 15:21 UTC #8 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

Quoted 39 lines · Vlastimil Babka

Quoted 38 lines · Shakeel Butt
Quoted 36 lines · Lorenzo Stoakes
Shakeel - This breaks the build in mm-new for me:
CC mm/pt_reclaim.o
In file included from ./arch/x86/include/asm/rmwcc.h:5,
                 from ./arch/x86/include/asm/bitops.h:18,
                 from ./include/linux/bitops.h:68,
                 from ./include/linux/radix-tree.h:11,
                 from ./include/linux/idr.h:15,
                 from ./include/linux/cgroup-defs.h:13,
                 from mm/memcontrol.c:28:
mm/memcontrol.c: In function ‘mem_cgroup_alloc’:
./arch/x86/include/asm/percpu.h:39:45: error: expected identifier or ‘(’ before ‘__seg_gs’
39 | #define __percpu_seg_override CONCATENATE(__seg_, __percpu_seg)
| ^~~~~~
./include/linux/args.h:25:24: note: in definition of macro ‘__CONCAT’
25 | #define __CONCAT(a, b) a ## b
| ^
./arch/x86/include/asm/percpu.h:39:33: note: in expansion of macro ‘CONCATENATE’
   39 | #define __percpu_seg_override   CONCATENATE(__seg_, __percpu_seg)
      |                                 ^~~~~~~~~~~
./arch/x86/include/asm/percpu.h:93:33: note: in expansion of macro ‘__percpu_seg_override’
   93 | # define __percpu_qual          __percpu_seg_override
      |                                 ^~~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:60:25: note: in expansion of macro ‘__percpu_qual’
   60 | # define __percpu       __percpu_qual BTF_TYPE_TAG(percpu)
      |                         ^~~~~~~~~~~~~
mm/memcontrol.c:3700:45: note: in expansion of macro ‘__percpu’
3700 | struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
| ^~~~~~~~
mm/memcontrol.c:3731:25: error: ‘pstatc_pcpu’ undeclared (first use in this function); did you mean ‘kstat_cpu’?
 3731 |                         pstatc_pcpu = parent->vmstats_percpu;
      |                         ^~~~~~~~~~~
      |                         kstat_cpu
mm/memcontrol.c:3731:25: note: each undeclared identifier is reported only once for each function it appears in

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
The __percpu macro seems to be a bit screwy with comma-delimited decls, as it seems that putting this on its own line fixes this problem:
Which compiler (and version) is this? Thanks for the fix.
Hm right I see the same errors with gcc 7, 13, 14, 15 but not with clang.

It seems to work with gcc 11.5.0, so weird.

Alexei Starovoitov

Tested-by

2025-05-17 00:25 UTC #9 lore raw

Re: [PATCH v2 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs

In reply to Lorenzo Stoakes

Quoted 27 lines · Lorenzo Stoakes

----8<----
From 28275e5d054506746d310cf5ebd1fafdb0881dba Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <[email protected]>
Date: Thu, 15 May 2025 13:43:46 +0100
Subject: [PATCH] fix

Signed-off-by Lorenzo Stoakes <[email protected]>

---
 mm/memcontrol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

mm/memcontrol.c +2 -2

index 2464a58fbf17..40fcc2259e5f 100644
@@ -3697,7 +3697,8 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
 
 static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 {
-       struct memcg_vmstats_percpu *statc, __percpu *pstatc_pcpu;
+       struct memcg_vmstats_percpu *statc;
+       struct memcg_vmstats_percpu __percpu *pstatc_pcpu;
        struct mem_cgroup *memcg;
        int node, cpu;
        int __maybe_unused i;
--

Quoted 1 line

2.49.0

Tested-by Alexei Starovoitov <[email protected]>

Andrew,

Please pick up Lorenzo's fix into mm-new.

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #10 lore raw

PATCH v2 2/7 memcg: move preempt disable to callers of memcg_rstat_updated

In reply to Shakeel Butt

Let's move the explicit preempt disable code to the callers of memcg_rstat_updated and also remove the memcg_stats_lock and related functions which ensures the callers of stats update functions have disabled preemption because now the stats update functions are explicitly disabling preemption.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 74 +++++++++++++------------------------------------
 1 file changed, 19 insertions(+), 55 deletions(-)

mm/memcontrol.c +19 ~~-55~~

index 2464a58fbf17..1750d86012f3 100644
@@ -557,48 +557,22 @@ static u64 flush_last_time;
 
 #define FLUSH_TIME (2UL*HZ)
 
-/*
- * Accessors to ensure that preemption is disabled on PREEMPT_RT because it can
- * not rely on this as part of an acquired spinlock_t lock. These functions are
- * never used in hardirq context on PREEMPT_RT and therefore disabling preemtion
- * is sufficient.
- */
-static void memcg_stats_lock(void)
-{
-	preempt_disable_nested();
-	VM_WARN_ON_IRQS_ENABLED();
-}
-
-static void __memcg_stats_lock(void)
-{
-	preempt_disable_nested();
-}
-
-static void memcg_stats_unlock(void)
-{
-	preempt_enable_nested();
-}
-
-
 static bool memcg_vmstats_needs_flush(struct memcg_vmstats *vmstats)
 {
 	return atomic64_read(&vmstats->stats_updates) >
 		MEMCG_CHARGE_BATCH * num_online_cpus();
 }
 
-static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
+static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val,
+				       int cpu)
 {
 	struct memcg_vmstats_percpu __percpu *statc_pcpu;
 	struct memcg_vmstats_percpu *statc;
-	int cpu;
 	unsigned int stats_updates;
 
 	if (!val)
 		return;
 
-	/* Don't assume callers have preemption disabled. */
-	cpu = get_cpu();
-
 	cgroup_rstat_updated(memcg->css.cgroup, cpu);
 	statc_pcpu = memcg->vmstats_percpu;
 	for (; statc_pcpu; statc_pcpu = statc->parent_pcpu) {

@@ -619,7 +593,6 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 		stats_updates = this_cpu_xchg(statc_pcpu->stats_updates, 0);
 		atomic64_add(stats_updates, &statc->vmstats->stats_updates);
 	}
-	put_cpu();
 }
 
 static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)

@@ -717,6 +690,7 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
 		       int val)
 {
 	int i = memcg_stats_index(idx);
+	int cpu;
 
 	if (mem_cgroup_disabled())
 		return;

@@ -724,12 +698,14 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
 	if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx))
 		return;
 
-	memcg_stats_lock();
+	cpu = get_cpu();
+
 	__this_cpu_add(memcg->vmstats_percpu->state[i], val);
 	val = memcg_state_val_in_pages(idx, val);
-	memcg_rstat_updated(memcg, val);
+	memcg_rstat_updated(memcg, val, cpu);
 	trace_mod_memcg_state(memcg, idx, val);
-	memcg_stats_unlock();
+
+	put_cpu();
 }
 
 #ifdef CONFIG_MEMCG_V1

@@ -758,6 +734,7 @@ static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
 	struct mem_cgroup_per_node *pn;
 	struct mem_cgroup *memcg;
 	int i = memcg_stats_index(idx);
+	int cpu;
 
 	if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx))
 		return;

@@ -765,24 +742,7 @@ static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
 	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
 	memcg = pn->memcg;
 
-	/*
-	 * The caller from rmap relies on disabled preemption because they never
-	 * update their counter from in-interrupt context. For these two
-	 * counters we check that the update is never performed from an
-	 * interrupt context while other caller need to have disabled interrupt.
-	 */
-	__memcg_stats_lock();
-	if (IS_ENABLED(CONFIG_DEBUG_VM)) {
-		switch (idx) {
-		case NR_ANON_MAPPED:
-		case NR_FILE_MAPPED:
-		case NR_ANON_THPS:
-			WARN_ON_ONCE(!in_task());
-			break;
-		default:
-			VM_WARN_ON_IRQS_ENABLED();
-		}
-	}
+	cpu = get_cpu();
 
 	/* Update memcg */
 	__this_cpu_add(memcg->vmstats_percpu->state[i], val);

@@ -791,9 +751,10 @@ static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
 	__this_cpu_add(pn->lruvec_stats_percpu->state[i], val);
 
 	val = memcg_state_val_in_pages(idx, val);
-	memcg_rstat_updated(memcg, val);
+	memcg_rstat_updated(memcg, val, cpu);
 	trace_mod_memcg_lruvec_state(memcg, idx, val);
-	memcg_stats_unlock();
+
+	put_cpu();
 }
 
 /**

@@ -873,6 +834,7 @@ void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
 			  unsigned long count)
 {
 	int i = memcg_events_index(idx);
+	int cpu;
 
 	if (mem_cgroup_disabled())
 		return;

@@ -880,11 +842,13 @@ void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
 	if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx))
 		return;
 
-	memcg_stats_lock();
+	cpu = get_cpu();
+
 	__this_cpu_add(memcg->vmstats_percpu->events[i], count);
-	memcg_rstat_updated(memcg, count);
+	memcg_rstat_updated(memcg, count, cpu);
 	trace_count_memcg_events(memcg, idx, count);
-	memcg_stats_unlock();
+
+	put_cpu();
 }
 
 unsigned long memcg_events(struct mem_cgroup *memcg, int event)

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #11 lore raw

PATCH v2 3/7 memcg: make mod_memcg_state re-entrant safe against irqs

In reply to Shakeel Butt

Let's make mod_memcg_state re-entrant safe against irqs. The only thing needed is to convert the usage of __this_cpu_add() to this_cpu_add(). In addition, with re-entrant safety, there is no need to disable irqs.

mod_memcg_state() is not safe against nmi, so let's add warning if someone tries to call it in nmi context.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 include/linux/memcontrol.h | 20 ++------------------
 mm/memcontrol.c            |  8 ++++----
 2 files changed, 6 insertions(+), 22 deletions(-)

include/linux/memcontrol.h +2 ~~-18~~

index 9ed75f82b858..92861ff3c43f 100644
@@ -903,19 +903,9 @@ struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
 					    struct mem_cgroup *oom_domain);
 void mem_cgroup_print_oom_group(struct mem_cgroup *memcg);
 
-void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
-		       int val);
-
 /* idx can be of type enum memcg_stat_item or node_stat_item */
-static inline void mod_memcg_state(struct mem_cgroup *memcg,
-				   enum memcg_stat_item idx, int val)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	__mod_memcg_state(memcg, idx, val);
-	local_irq_restore(flags);
-}
+void mod_memcg_state(struct mem_cgroup *memcg,
+		     enum memcg_stat_item idx, int val);
 
 static inline void mod_memcg_page_state(struct page *page,
 					enum memcg_stat_item idx, int val)

@@ -1375,12 +1365,6 @@ static inline void mem_cgroup_print_oom_group(struct mem_cgroup *memcg)
 {
 }
 
-static inline void __mod_memcg_state(struct mem_cgroup *memcg,
-				     enum memcg_stat_item idx,
-				     int nr)
-{
-}
-
 static inline void mod_memcg_state(struct mem_cgroup *memcg,
 				   enum memcg_stat_item idx,
 				   int nr)

mm/memcontrol.c +4 -4

index 1750d86012f3..c5a835071610 100644
@@ -681,12 +681,12 @@ static int memcg_state_val_in_pages(int idx, int val)
 }
 
 /**
- * __mod_memcg_state - update cgroup memory statistics
+ * mod_memcg_state - update cgroup memory statistics
  * @memcg: the memory cgroup
  * @idx: the stat item - can be enum memcg_stat_item or enum node_stat_item
  * @val: delta to add to the counter, can be negative
  */
-void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
+void mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
 		       int val)
 {
 	int i = memcg_stats_index(idx);

@@ -700,7 +700,7 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
 
 	cpu = get_cpu();
 
-	__this_cpu_add(memcg->vmstats_percpu->state[i], val);
+	this_cpu_add(memcg->vmstats_percpu->state[i], val);
 	val = memcg_state_val_in_pages(idx, val);
 	memcg_rstat_updated(memcg, val, cpu);
 	trace_mod_memcg_state(memcg, idx, val);

@@ -2920,7 +2920,7 @@ static void drain_obj_stock(struct obj_stock_pcp *stock)
 
 			memcg = get_mem_cgroup_from_objcg(old);
 
-			__mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
+			mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
 			memcg1_account_kmem(memcg, -nr_pages);
 			if (!mem_cgroup_is_root(memcg))
 				memcg_uncharge(memcg, nr_pages);

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #12 lore raw

PATCH v2 4/7 memcg: make count_memcg_events re-entrant safe against irqs

In reply to Shakeel Butt

Let's make count_memcg_events re-entrant safe against irqs. The only thing needed is to convert the usage of __this_cpu_add() to this_cpu_add(). In addition, with re-entrant safety, there is no need to disable irqs. Also add warnings for in_nmi() as it is not safe against nmi context.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 include/linux/memcontrol.h | 21 ++-------------------
 mm/memcontrol-v1.c         |  6 +++---
 mm/memcontrol.c            |  6 +++---
 mm/swap.c                  |  8 ++++----
 mm/vmscan.c                | 14 +++++++-------
 5 files changed, 19 insertions(+), 36 deletions(-)

include/linux/memcontrol.h +2 ~~-19~~

index 92861ff3c43f..f7848f73f41c 100644
@@ -942,19 +942,8 @@ static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
 	local_irq_restore(flags);
 }
 
-void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
-			  unsigned long count);
-
-static inline void count_memcg_events(struct mem_cgroup *memcg,
-				      enum vm_event_item idx,
-				      unsigned long count)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-	__count_memcg_events(memcg, idx, count);
-	local_irq_restore(flags);
-}
+void count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
+			unsigned long count);
 
 static inline void count_memcg_folio_events(struct folio *folio,
 		enum vm_event_item idx, unsigned long nr)

@@ -1418,12 +1407,6 @@ static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
 }
 
 static inline void count_memcg_events(struct mem_cgroup *memcg,
-				      enum vm_event_item idx,
-				      unsigned long count)
-{
-}
-
-static inline void __count_memcg_events(struct mem_cgroup *memcg,
 					enum vm_event_item idx,
 					unsigned long count)
 {

mm/memcontrol-v1.c +3 -3

index 54c49cbfc968..4b94731305b9 100644
@@ -512,9 +512,9 @@ static void memcg1_charge_statistics(struct mem_cgroup *memcg, int nr_pages)
 {
 	/* pagein of a big page is an event. So, ignore page size */
 	if (nr_pages > 0)
-		__count_memcg_events(memcg, PGPGIN, 1);
+		count_memcg_events(memcg, PGPGIN, 1);
 	else {
-		__count_memcg_events(memcg, PGPGOUT, 1);
+		count_memcg_events(memcg, PGPGOUT, 1);
 		nr_pages = -nr_pages; /* for event */
 	}

@@ -689,7 +689,7 @@ void memcg1_uncharge_batch(struct mem_cgroup *memcg, unsigned long pgpgout,
 	unsigned long flags;
 
 	local_irq_save(flags);
-	__count_memcg_events(memcg, PGPGOUT, pgpgout);
+	count_memcg_events(memcg, PGPGOUT, pgpgout);
 	__this_cpu_add(memcg->events_percpu->nr_page_events, nr_memory);
 	memcg1_check_events(memcg, nid);
 	local_irq_restore(flags);

mm/memcontrol.c +3 -3

index c5a835071610..0923072386c2 100644
@@ -825,12 +825,12 @@ void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val)
 }
 
 /**
- * __count_memcg_events - account VM events in a cgroup
+ * count_memcg_events - account VM events in a cgroup
  * @memcg: the memory cgroup
  * @idx: the event item
  * @count: the number of events that occurred
  */
-void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
+void count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
 			  unsigned long count)
 {
 	int i = memcg_events_index(idx);

@@ -844,7 +844,7 @@ void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
 
 	cpu = get_cpu();
 
-	__this_cpu_add(memcg->vmstats_percpu->events[i], count);
+	this_cpu_add(memcg->vmstats_percpu->events[i], count);
 	memcg_rstat_updated(memcg, count, cpu);
 	trace_count_memcg_events(memcg, idx, count);

mm/swap.c +4 -4

index 77b2d5997873..4fc322f7111a 100644
@@ -309,7 +309,7 @@ static void lru_activate(struct lruvec *lruvec, struct folio *folio)
 	trace_mm_lru_activate(folio);
 
 	__count_vm_events(PGACTIVATE, nr_pages);
-	__count_memcg_events(lruvec_memcg(lruvec), PGACTIVATE, nr_pages);
+	count_memcg_events(lruvec_memcg(lruvec), PGACTIVATE, nr_pages);
 }
 
 #ifdef CONFIG_SMP

@@ -581,7 +581,7 @@ static void lru_deactivate_file(struct lruvec *lruvec, struct folio *folio)
 
 	if (active) {
 		__count_vm_events(PGDEACTIVATE, nr_pages);
-		__count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE,
+		count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE,
 				     nr_pages);
 	}
 }

@@ -599,7 +599,7 @@ static void lru_deactivate(struct lruvec *lruvec, struct folio *folio)
 	lruvec_add_folio(lruvec, folio);
 
 	__count_vm_events(PGDEACTIVATE, nr_pages);
-	__count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_pages);
+	count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_pages);
 }
 
 static void lru_lazyfree(struct lruvec *lruvec, struct folio *folio)

@@ -625,7 +625,7 @@ static void lru_lazyfree(struct lruvec *lruvec, struct folio *folio)
 	lruvec_add_folio(lruvec, folio);
 
 	__count_vm_events(PGLAZYFREE, nr_pages);
-	__count_memcg_events(lruvec_memcg(lruvec), PGLAZYFREE, nr_pages);
+	count_memcg_events(lruvec_memcg(lruvec), PGLAZYFREE, nr_pages);
 }
 
 /*

mm/vmscan.c +7 -7

index 5efd939d8c76..f86d264558f5 100644
@@ -2028,7 +2028,7 @@ static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
 	item = PGSCAN_KSWAPD + reclaimer_offset(sc);
 	if (!cgroup_reclaim(sc))
 		__count_vm_events(item, nr_scanned);
-	__count_memcg_events(lruvec_memcg(lruvec), item, nr_scanned);
+	count_memcg_events(lruvec_memcg(lruvec), item, nr_scanned);
 	__count_vm_events(PGSCAN_ANON + file, nr_scanned);
 
 	spin_unlock_irq(&lruvec->lru_lock);

@@ -2048,7 +2048,7 @@ static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
 	item = PGSTEAL_KSWAPD + reclaimer_offset(sc);
 	if (!cgroup_reclaim(sc))
 		__count_vm_events(item, nr_reclaimed);
-	__count_memcg_events(lruvec_memcg(lruvec), item, nr_reclaimed);
+	count_memcg_events(lruvec_memcg(lruvec), item, nr_reclaimed);
 	__count_vm_events(PGSTEAL_ANON + file, nr_reclaimed);
 	spin_unlock_irq(&lruvec->lru_lock);

@@ -2138,7 +2138,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
 
 	if (!cgroup_reclaim(sc))
 		__count_vm_events(PGREFILL, nr_scanned);
-	__count_memcg_events(lruvec_memcg(lruvec), PGREFILL, nr_scanned);
+	count_memcg_events(lruvec_memcg(lruvec), PGREFILL, nr_scanned);
 
 	spin_unlock_irq(&lruvec->lru_lock);

@@ -2195,7 +2195,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
 	nr_deactivate = move_folios_to_lru(lruvec, &l_inactive);
 
 	__count_vm_events(PGDEACTIVATE, nr_deactivate);
-	__count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_deactivate);
+	count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_deactivate);
 
 	__mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, -nr_taken);
 	spin_unlock_irq(&lruvec->lru_lock);

@@ -4616,8 +4616,8 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 		__count_vm_events(item, isolated);
 		__count_vm_events(PGREFILL, sorted);
 	}
-	__count_memcg_events(memcg, item, isolated);
-	__count_memcg_events(memcg, PGREFILL, sorted);
+	count_memcg_events(memcg, item, isolated);
+	count_memcg_events(memcg, PGREFILL, sorted);
 	__count_vm_events(PGSCAN_ANON + type, isolated);
 	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH,
 				scanned, skipped, isolated,

@@ -4769,7 +4769,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	item = PGSTEAL_KSWAPD + reclaimer_offset(sc);
 	if (!cgroup_reclaim(sc))
 		__count_vm_events(item, reclaimed);
-	__count_memcg_events(memcg, item, reclaimed);
+	count_memcg_events(memcg, item, reclaimed);
 	__count_vm_events(PGSTEAL_ANON + type, reclaimed);
 
 	spin_unlock_irq(&lruvec->lru_lock);

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #13 lore raw

PATCH v2 5/7 memcg: make __mod_memcg_lruvec_state re-entrant safe against irqs

In reply to Shakeel Butt

Let's make __mod_memcg_lruvec_state re-entrant safe and name it mod_memcg_lruvec_state(). The only thing needed is to convert the usage of __this_cpu_add() to this_cpu_add(). There are two callers of mod_memcg_lruvec_state() and one of them i.e. __mod_objcg_mlstate() will be re-entrant safe as well, so, rename it mod_objcg_mlstate(). The last caller __mod_lruvec_state() still calls __mod_node_page_state() which is not re-entrant safe yet, so keep it as is.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

mm/memcontrol.c +11 ~~-11~~

index 0923072386c2..1071db0b1df8 100644
@@ -727,7 +727,7 @@ unsigned long memcg_page_state_local(struct mem_cgroup *memcg, int idx)
 }
 #endif
 
-static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
+static void mod_memcg_lruvec_state(struct lruvec *lruvec,
 				     enum node_stat_item idx,
 				     int val)
 {

@@ -745,10 +745,10 @@ static void __mod_memcg_lruvec_state(struct lruvec *lruvec,
 	cpu = get_cpu();
 
 	/* Update memcg */
-	__this_cpu_add(memcg->vmstats_percpu->state[i], val);
+	this_cpu_add(memcg->vmstats_percpu->state[i], val);
 
 	/* Update lruvec */
-	__this_cpu_add(pn->lruvec_stats_percpu->state[i], val);
+	this_cpu_add(pn->lruvec_stats_percpu->state[i], val);
 
 	val = memcg_state_val_in_pages(idx, val);
 	memcg_rstat_updated(memcg, val, cpu);

@@ -775,7 +775,7 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
 
 	/* Update memcg and lruvec */
 	if (!mem_cgroup_disabled())
-		__mod_memcg_lruvec_state(lruvec, idx, val);
+		mod_memcg_lruvec_state(lruvec, idx, val);
 }
 
 void __lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx,

@@ -2527,7 +2527,7 @@ static void commit_charge(struct folio *folio, struct mem_cgroup *memcg)
 	folio->memcg_data = (unsigned long)memcg;
 }
 
-static inline void __mod_objcg_mlstate(struct obj_cgroup *objcg,
+static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
 				       struct pglist_data *pgdat,
 				       enum node_stat_item idx, int nr)
 {

@@ -2537,7 +2537,7 @@ static inline void __mod_objcg_mlstate(struct obj_cgroup *objcg,
 	rcu_read_lock();
 	memcg = obj_cgroup_memcg(objcg);
 	lruvec = mem_cgroup_lruvec(memcg, pgdat);
-	__mod_memcg_lruvec_state(lruvec, idx, nr);
+	mod_memcg_lruvec_state(lruvec, idx, nr);
 	rcu_read_unlock();
 }

@@ -2847,12 +2847,12 @@ static void __account_obj_stock(struct obj_cgroup *objcg,
 		struct pglist_data *oldpg = stock->cached_pgdat;
 
 		if (stock->nr_slab_reclaimable_b) {
-			__mod_objcg_mlstate(objcg, oldpg, NR_SLAB_RECLAIMABLE_B,
+			mod_objcg_mlstate(objcg, oldpg, NR_SLAB_RECLAIMABLE_B,
 					  stock->nr_slab_reclaimable_b);
 			stock->nr_slab_reclaimable_b = 0;
 		}
 		if (stock->nr_slab_unreclaimable_b) {
-			__mod_objcg_mlstate(objcg, oldpg, NR_SLAB_UNRECLAIMABLE_B,
+			mod_objcg_mlstate(objcg, oldpg, NR_SLAB_UNRECLAIMABLE_B,
 					  stock->nr_slab_unreclaimable_b);
 			stock->nr_slab_unreclaimable_b = 0;
 		}

@@ -2878,7 +2878,7 @@ static void __account_obj_stock(struct obj_cgroup *objcg,
 		}
 	}
 	if (nr)
-		__mod_objcg_mlstate(objcg, pgdat, idx, nr);
+		mod_objcg_mlstate(objcg, pgdat, idx, nr);
 }
 
 static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,

@@ -2947,13 +2947,13 @@ static void drain_obj_stock(struct obj_stock_pcp *stock)
 	 */
 	if (stock->nr_slab_reclaimable_b || stock->nr_slab_unreclaimable_b) {
 		if (stock->nr_slab_reclaimable_b) {
-			__mod_objcg_mlstate(old, stock->cached_pgdat,
+			mod_objcg_mlstate(old, stock->cached_pgdat,
 					  NR_SLAB_RECLAIMABLE_B,
 					  stock->nr_slab_reclaimable_b);
 			stock->nr_slab_reclaimable_b = 0;
 		}
 		if (stock->nr_slab_unreclaimable_b) {
-			__mod_objcg_mlstate(old, stock->cached_pgdat,
+			mod_objcg_mlstate(old, stock->cached_pgdat,
 					  NR_SLAB_UNRECLAIMABLE_B,
 					  stock->nr_slab_unreclaimable_b);
 			stock->nr_slab_unreclaimable_b = 0;

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #14 lore raw

PATCH v2 6/7 memcg: no stock lock for cpu hot-unplug

In reply to Shakeel Butt

Previously on the cpu hot-unplug, the kernel would call drain_obj_stock() with objcg local lock. However local lock was not needed as the stock which was accessed belongs to a dead cpu but we kept it there to disable irqs as drain_obj_stock() may call mod_objcg_mlstate() which required irqs disabled. However there is no need to disable irqs now for mod_objcg_mlstate(), so we can remove the local lock altogether from cpu hot-unplug path.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

mm/memcontrol.c +1 ~~-10~~

index 1071db0b1df8..04d756be708b 100644
@@ -2025,17 +2025,8 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 
 static int memcg_hotplug_cpu_dead(unsigned int cpu)
 {
-	struct obj_stock_pcp *obj_st;
-	unsigned long flags;
-
-	obj_st = &per_cpu(obj_stock, cpu);
-
-	/* drain_obj_stock requires objstock.lock */
-	local_lock_irqsave(&obj_stock.lock, flags);
-	drain_obj_stock(obj_st);
-	local_unlock_irqrestore(&obj_stock.lock, flags);
-
 	/* no need for the local lock */
+	drain_obj_stock(&per_cpu(obj_stock, cpu));
 	drain_stock_fully(&per_cpu(memcg_stock, cpu));
 
 	return 0;

Signature · 1 line

2.47.1

Shakeel Butt

author maintainer

2025-05-14 18:42 UTC #15 lore raw

PATCH v2 7/7 memcg: objcg stock trylock without irq disabling

In reply to Shakeel Butt

There is no need to disable irqs to use objcg per-cpu stock, so let's just not do that but consume_obj_stock() and refill_obj_stock() will need to use trylock instead to avoid deadlock against irq. One consequence of this change is that the charge request from irq context may take slowpath more often but it should be rare.

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

---
 mm/memcontrol.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

mm/memcontrol.c +15 ~~-10~~

index 04d756be708b..e17b698f6243 100644
@@ -1882,18 +1882,17 @@ static void drain_local_memcg_stock(struct work_struct *dummy)
 static void drain_local_obj_stock(struct work_struct *dummy)
 {
 	struct obj_stock_pcp *stock;
-	unsigned long flags;
 
 	if (WARN_ONCE(!in_task(), "drain in non-task context"))
 		return;
 
-	local_lock_irqsave(&obj_stock.lock, flags);
+	local_lock(&obj_stock.lock);
 
 	stock = this_cpu_ptr(&obj_stock);
 	drain_obj_stock(stock);
 	clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
 
-	local_unlock_irqrestore(&obj_stock.lock, flags);
+	local_unlock(&obj_stock.lock);
 }
 
 static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)

@@ -2876,10 +2875,10 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,
 			      struct pglist_data *pgdat, enum node_stat_item idx)
 {
 	struct obj_stock_pcp *stock;
-	unsigned long flags;
 	bool ret = false;
 
-	local_lock_irqsave(&obj_stock.lock, flags);
+	if (!local_trylock(&obj_stock.lock))
+		return ret;
 
 	stock = this_cpu_ptr(&obj_stock);
 	if (objcg == READ_ONCE(stock->cached_objcg) && stock->nr_bytes >= nr_bytes) {

@@ -2890,7 +2889,7 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,
 			__account_obj_stock(objcg, stock, nr_bytes, pgdat, idx);
 	}
 
-	local_unlock_irqrestore(&obj_stock.lock, flags);
+	local_unlock(&obj_stock.lock);
 
 	return ret;
 }

@@ -2979,10 +2978,16 @@ static void refill_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,
 		enum node_stat_item idx)
 {
 	struct obj_stock_pcp *stock;
-	unsigned long flags;
 	unsigned int nr_pages = 0;
 
-	local_lock_irqsave(&obj_stock.lock, flags);
+	if (!local_trylock(&obj_stock.lock)) {
+		if (pgdat)
+			mod_objcg_mlstate(objcg, pgdat, idx, nr_bytes);
+		nr_pages = nr_bytes >> PAGE_SHIFT;
+		nr_bytes = nr_bytes & (PAGE_SIZE - 1);
+		atomic_add(nr_bytes, &objcg->nr_charged_bytes);
+		goto out;
+	}
 
 	stock = this_cpu_ptr(&obj_stock);
 	if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */

@@ -3004,8 +3009,8 @@ static void refill_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,
 		stock->nr_bytes &= (PAGE_SIZE - 1);
 	}
 
-	local_unlock_irqrestore(&obj_stock.lock, flags);
-
+	local_unlock(&obj_stock.lock);
+out:
 	if (nr_pages)
 		obj_cgroup_uncharge_pages(objcg, nr_pages);
 }

Signature · 1 line

2.47.1