From: Andrew Morton <akpm@linux-foundation.org>
To: mm-commits@vger.kernel.org,yosryahmed@google.com,songmuchun@bytedance.com,shakeel.butt@linux.dev,roman.gushchin@linux.dev,mhocko@suse.com,hannes@cmpxchg.org,leitao@debian.org,akpm@linux-foundation.org
Subject: [merged mm-stable] memcg-fix-data-race-kcsan-bug-in-rstats.patch removed from -mm tree
Date: Sun, 05 May 2024 17:58:27 -0700 [thread overview]
Message-ID: <20240506005827.87A79C113CC@smtp.kernel.org> (raw)
The quilt patch titled
Subject: memcg: fix data-race KCSAN bug in rstats
has been removed from the -mm tree. Its filename was
memcg-fix-data-race-kcsan-bug-in-rstats.patch
This patch was dropped because it was merged into the mm-stable branch
of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
------------------------------------------------------
From: Breno Leitao <leitao@debian.org>
Subject: memcg: fix data-race KCSAN bug in rstats
Date: Wed, 24 Apr 2024 05:59:39 -0700
A data-race issue in memcg rstat occurs when two distinct code paths
access the same 4-byte region concurrently. KCSAN detection triggers the
following BUG as a result.
BUG: KCSAN: data-race in __count_memcg_events / mem_cgroup_css_rstat_flush
write to 0xffffe8ffff98e300 of 4 bytes by task 5274 on cpu 17:
mem_cgroup_css_rstat_flush (mm/memcontrol.c:5850)
cgroup_rstat_flush_locked (kernel/cgroup/rstat.c:243 (discriminator 7))
cgroup_rstat_flush (./include/linux/spinlock.h:401 kernel/cgroup/rstat.c:278)
mem_cgroup_flush_stats.part.0 (mm/memcontrol.c:767)
memory_numa_stat_show (mm/memcontrol.c:6911)
<snip>
read to 0xffffe8ffff98e300 of 4 bytes by task 410848 on cpu 27:
__count_memcg_events (mm/memcontrol.c:725 mm/memcontrol.c:962)
count_memcg_event_mm.part.0 (./include/linux/memcontrol.h:1097 ./include/linux/memcontrol.h:1120)
handle_mm_fault (mm/memory.c:5483 mm/memory.c:5622)
<snip>
value changed: 0x00000029 -> 0x00000000
The race occurs because two code paths access the same "stats_updates"
location. Although "stats_updates" is a per-CPU variable, it is remotely
accessed by another CPU at
cgroup_rstat_flush_locked()->mem_cgroup_css_rstat_flush(), leading to the
data race mentioned.
Considering that memcg_rstat_updated() is in the hot code path, adding a
lock to protect it may not be desirable, especially since this variable
pertains solely to statistics.
Therefore, annotating accesses to stats_updates with READ/WRITE_ONCE() can
prevent KCSAN splats and potential partial reads/writes.
Link: https://lkml.kernel.org/r/20240424125940.2410718-1-leitao@debian.org
Fixes: 9cee7e8ef3e3 ("mm: memcg: optimize parent iteration in memcg_rstat_updated()")
Signed-off-by: Breno Leitao <leitao@debian.org>
Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Muchun Song <songmuchun@bytedance.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
mm/memcontrol.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
--- a/mm/memcontrol.c~memcg-fix-data-race-kcsan-bug-in-rstats
+++ a/mm/memcontrol.c
@@ -715,6 +715,7 @@ static inline void memcg_rstat_updated(s
{
struct memcg_vmstats_percpu *statc;
int cpu = smp_processor_id();
+ unsigned int stats_updates;
if (!val)
return;
@@ -722,8 +723,9 @@ static inline void memcg_rstat_updated(s
cgroup_rstat_updated(memcg->css.cgroup, cpu);
statc = this_cpu_ptr(memcg->vmstats_percpu);
for (; statc; statc = statc->parent) {
- statc->stats_updates += abs(val);
- if (statc->stats_updates < MEMCG_CHARGE_BATCH)
+ stats_updates = READ_ONCE(statc->stats_updates) + abs(val);
+ WRITE_ONCE(statc->stats_updates, stats_updates);
+ if (stats_updates < MEMCG_CHARGE_BATCH)
continue;
/*
@@ -731,9 +733,9 @@ static inline void memcg_rstat_updated(s
* redundant. Avoid the overhead of the atomic update.
*/
if (!memcg_vmstats_needs_flush(statc->vmstats))
- atomic64_add(statc->stats_updates,
+ atomic64_add(stats_updates,
&statc->vmstats->stats_updates);
- statc->stats_updates = 0;
+ WRITE_ONCE(statc->stats_updates, 0);
}
}
@@ -5887,7 +5889,7 @@ static void mem_cgroup_css_rstat_flush(s
}
}
}
- statc->stats_updates = 0;
+ WRITE_ONCE(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);
_
Patches currently in -mm which might be from leitao@debian.org are
reply other threads:[~2024-05-06 0:58 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20240506005827.87A79C113CC@smtp.kernel.org \
--to=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=leitao@debian.org \
--cc=mhocko@suse.com \
--cc=mm-commits@vger.kernel.org \
--cc=roman.gushchin@linux.dev \
--cc=shakeel.butt@linux.dev \
--cc=songmuchun@bytedance.com \
--cc=yosryahmed@google.com \
/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.