| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <22f69e6e-7908-4e92-96ca-5c70d535c439@lucifer.local>
Date: Thu, 15 May 2025 13:47:11 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>, Michal Hocko <mhocko@...nel.org>,
Roman Gushchin <roman.gushchin@...ux.dev>,
Muchun Song <muchun.song@...ux.dev>, Vlastimil Babka <vbabka@...e.cz>,
Alexei Starovoitov <ast@...nel.org>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Harry Yoo <harry.yoo@...cle.com>, Yosry Ahmed <yosry.ahmed@...ux.dev>,
bpf@...r.kernel.org, linux-mm@...ck.org, cgroups@...r.kernel.org,
linux-kernel@...r.kernel.org, Meta kernel team <kernel-team@...a.com>
Subject: 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 <lorenzo.stoakes@...cle.com>
Date: Thu, 15 May 2025 13:43:46 +0100
Subject: [PATCH] fix
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
---
mm/memcontrol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2464a58fbf17..40fcc2259e5f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -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!
On Wed, May 14, 2025 at 11:41:52AM -0700, Shakeel Butt wrote:
> 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 <shakeel.butt@...ux.dev>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
> ---
> mm/memcontrol.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 89476a71a18d..2464a58fbf17 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -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;
> }
>
> --
> 2.47.1
>
>
----8<----
>From 28275e5d054506746d310cf5ebd1fafdb0881dba Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Date: Thu, 15 May 2025 13:43:46 +0100
Subject: [PATCH] fix
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
---
mm/memcontrol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2464a58fbf17..40fcc2259e5f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -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
Powered by blists - more mailing lists