From: Shakeel Butt <shakeel.butt@linux.dev>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Michal Hocko <mhocko@kernel.org>,
Roman Gushchin <roman.gushchin@linux.dev>,
Muchun Song <muchun.song@linux.dev>,
Vlastimil Babka <vbabka@suse.cz>,
Alexei Starovoitov <ast@kernel.org>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Harry Yoo <harry.yoo@oracle.com>,
Yosry Ahmed <yosry.ahmed@linux.dev>,
bpf@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org,
Meta kernel team <kernel-team@meta.com>
Subject: [RFC PATCH 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs
Date: Mon, 12 May 2025 20:13:10 -0700 [thread overview]
Message-ID: <20250513031316.2147548-2-shakeel.butt@linux.dev> (raw)
In-Reply-To: <20250513031316.2147548-1-shakeel.butt@linux.dev>
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 against irqs.
However it is using atomic_* ops which on x86, adds lock prefix to the
instructions. Since this is per-cpu data, the this_cpu_* ops are
preferred. However the percpu pointer is stored in struct mem_cgroup and
doing the upward traversal through struct mem_cgroup may cause two cache
misses as compared to traversing through struct memcg_vmstats_percpu
pointer.
NOTE: explore if there is atomic_* ops alternative without lock prefix.
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
mm/memcontrol.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6cfa3550f300..2c4c095bf26c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -503,7 +503,7 @@ static inline int memcg_events_index(enum vm_event_item idx)
struct memcg_vmstats_percpu {
/* Stats updates since the last flush */
- unsigned int stats_updates;
+ atomic_t stats_updates;
/* Cached pointers for fast iteration in memcg_rstat_updated() */
struct memcg_vmstats_percpu *parent;
@@ -590,12 +590,15 @@ 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 *statc;
- int cpu = smp_processor_id();
- unsigned int stats_updates;
+ int cpu;
+ 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) {
@@ -607,14 +610,16 @@ 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 = atomic_add_return(abs(val), &statc->stats_updates);
if (stats_updates < MEMCG_CHARGE_BATCH)
continue;
- atomic64_add(stats_updates, &statc->vmstats->stats_updates);
- WRITE_ONCE(statc->stats_updates, 0);
+ stats_updates = atomic_xchg(&statc->stats_updates, 0);
+ if (stats_updates)
+ atomic64_add(stats_updates,
+ &statc->vmstats->stats_updates);
}
+ put_cpu();
}
static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)
@@ -4155,7 +4160,7 @@ static void mem_cgroup_css_rstat_flush(struct cgroup_subsys_state *css, int cpu)
mem_cgroup_stat_aggregate(&ac);
}
- WRITE_ONCE(statc->stats_updates, 0);
+ atomic_set(&statc->stats_updates, 0);
/* We are in a per-cpu loop here, only do the atomic write once */
if (atomic64_read(&memcg->vmstats->stats_updates))
atomic64_set(&memcg->vmstats->stats_updates, 0);
--
2.47.1
next prev parent reply other threads:[~2025-05-13 3:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-13 3:13 [RFC PATCH 0/7] memcg: make memcg stats irq safe Shakeel Butt
2025-05-13 3:13 ` Shakeel Butt [this message]
2025-05-13 10:22 ` [RFC PATCH 1/7] memcg: memcg_rstat_updated re-entrant safe against irqs Vlastimil Babka
2025-05-13 18:09 ` Shakeel Butt
2025-05-13 3:13 ` [RFC PATCH 2/7] memcg: move preempt disable to callers of memcg_rstat_updated Shakeel Butt
2025-05-13 10:34 ` Vlastimil Babka
2025-05-13 3:13 ` [RFC PATCH 3/7] memcg: make mod_memcg_state re-entrant safe against irqs Shakeel Butt
2025-05-13 10:38 ` Vlastimil Babka
2025-05-13 3:13 ` [RFC PATCH 4/7] memcg: make count_memcg_events " Shakeel Butt
2025-05-13 10:39 ` Vlastimil Babka
2025-05-13 3:13 ` [RFC PATCH 5/7] memcg: make __mod_memcg_lruvec_state " Shakeel Butt
2025-05-13 10:40 ` Vlastimil Babka
2025-05-13 3:13 ` [RFC PATCH 6/7] memcg: objcg stock trylock without irq disabling Shakeel Butt
2025-05-13 13:05 ` Vlastimil Babka
2025-05-13 3:13 ` [RFC PATCH 7/7] memcg: no stock lock for cpu hot-unplug Shakeel Butt
2025-05-13 13:10 ` Vlastimil Babka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250513031316.2147548-2-shakeel.butt@linux.dev \
--to=shakeel.butt@linux.dev \
--cc=akpm@linux-foundation.org \
--cc=ast@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=bpf@vger.kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=harry.yoo@oracle.com \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=muchun.song@linux.dev \
--cc=roman.gushchin@linux.dev \
--cc=vbabka@suse.cz \
--cc=yosry.ahmed@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.