From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71579C4345F for ; Fri, 3 May 2024 11:27:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C70D76B007B; Fri, 3 May 2024 07:27:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C21096B0085; Fri, 3 May 2024 07:27:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AE8936B0088; Fri, 3 May 2024 07:27:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 926A06B007B for ; Fri, 3 May 2024 07:27:49 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B425BC10C3 for ; Fri, 3 May 2024 11:27:48 +0000 (UTC) X-FDA: 82076859816.23.01B8DBE Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf29.hostedemail.com (Postfix) with ESMTP id 31C32120018 for ; Fri, 3 May 2024 11:27:44 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Rx8vmp8Z; dkim=pass header.d=suse.com header.s=susede1 header.b=Rx8vmp8Z; spf=pass (imf29.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714735665; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=UXpjzV5DWrRABNO+vO47pjlZXBhvnc1hWU3ZO34EKqg=; b=VWY/s7NiMaIFCz25I0bzj0pXIq0lV/oWnCRLoxpTv/EBADq1dP0Jyw33DVhncDuEdfvc4U pqynGz8nxA+Sk6Vw23wyEUgPTnexmhFPgHPfxsNNHIg2stmsrQOoVbLQWEllTDhDFjL3h3 UoeJ9+Ypcc5Ew9S0k4+KtKFlEgj8oiQ= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Rx8vmp8Z; dkim=pass header.d=suse.com header.s=susede1 header.b=Rx8vmp8Z; spf=pass (imf29.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714735665; a=rsa-sha256; cv=none; b=IDk1+0ulN8ajdIgWb+n/bDYmnUtrV4SHsK/8WwPBQzK86sNXtIgdXsV7j/B+Gvf71y8dQl T2gNNVYdGaTcF4EjxWXz4hT5h9Iu4KaMFZySQqq8XFTBDqAEvlx+E0hxOLWbcESf6Tvc0q +6GZUYxMrmOJM+yt+IgJ1j1SnHaZSBI= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B99F23387A; Fri, 3 May 2024 11:27:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1714735662; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UXpjzV5DWrRABNO+vO47pjlZXBhvnc1hWU3ZO34EKqg=; b=Rx8vmp8Zigf3yHzGm1pm9Kmc0A6srYlVYDeWNYIw+gz2cyITTJE9exIDiHErn8Hc6h3ZVf TBVOYBBkck1vthjzNSpb170SUsJQUYxiZRJVelC8VbtQtYBiLottbMnvOM2tj5CB5cNnXn BJYpuaOQUzp7OBFNN2QiCivdXHFBgRE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1714735662; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UXpjzV5DWrRABNO+vO47pjlZXBhvnc1hWU3ZO34EKqg=; b=Rx8vmp8Zigf3yHzGm1pm9Kmc0A6srYlVYDeWNYIw+gz2cyITTJE9exIDiHErn8Hc6h3ZVf TBVOYBBkck1vthjzNSpb170SUsJQUYxiZRJVelC8VbtQtYBiLottbMnvOM2tj5CB5cNnXn BJYpuaOQUzp7OBFNN2QiCivdXHFBgRE= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 9F2AF139CB; Fri, 3 May 2024 11:27:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id G6nZIy7KNGZgWQAAD6G6ig (envelope-from ); Fri, 03 May 2024 11:27:42 +0000 Date: Fri, 3 May 2024 13:27:41 +0200 From: Michal Hocko To: Breno Leitao Cc: Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , leit@meta.com, "open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)" , "open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)" , open list Subject: Re: [PATCH] memcg: Fix data-race KCSAN bug in rstats Message-ID: References: <20240424125940.2410718-1-leitao@debian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240424125940.2410718-1-leitao@debian.org> X-Rspamd-Action: no action X-Stat-Signature: f6sr9jkiab5tabhpiqd5jnrah3eb5sxt X-Rspamd-Queue-Id: 31C32120018 X-Rspamd-Server: rspam10 X-Rspam-User: X-HE-Tag: 1714735664-630058 X-HE-Meta: U2FsdGVkX18ipAqTIokDfm2wly2z5hyDan/XNWMNAwox8bJhiXmloeBJ5SHakj4HLadwjePeRItUCj+7Edbcd36v7WrR8tdht9d7Rv/j7bbOlY4XYFqel0irzNvzMSYAhMkO2APQkjxPBGpZsVfui80fl9Ie9c4zn7LA1kWAuTDXyjLntg9RLIEWBgiiSYrTbDS92is27AZZdNFHxLQbEuigqyxWZGeQhO+PzHRYnal1yS1kYBlZzTOOwrNM8/Nxym5EiYeBpH9OB5oTIqWi2LoL2uAktmgrKWmXp4PgHKl0DCjIZmE68ex0fp3H0F1zyozR9IlNgCkZpqHED47LA9Xu3qc+xbv2mG6FCTvOgBYBxKQD79TX0r0GRlKbfObAbCQt3uEo8oWdRyqFfDiY0RfWQBLnQAdqSPmVvXWuI53s1uE5Djuo6SiQBgvqqBX/Oa5Zc7PaJGnYfVjCJjUrLEtbFbmau0Xv/4F+Z9pA+6Aco5EBnZcyowzfb8UkTZfozH5a3C/TJadfC6eZi9t2BBokKt7jk7OK+ViqYlmdAY0mbHi3oV+PycD5ORTDOBBNa1nRahV3QDDCUrHHUX9kuiFRCANC5TnCgKVQjteivGbH0cW0l5dkdpmdufVXEfF488mRbtv5/21NPLP8DLwMWdcG89EAjGKeA1+EMIcq4MKZZiuylJzcDfzjmERvYSLOHM+9qjaKnnuc4otwQEfiB58SmD8iYhBoDVMum7xA2sNcGXAPqMJTzJUfgF4vh5i21hYJ1zAvNhrSVEJIbJRfuCylVCT3LLPpmK8sDOewHALsiVLMupxe9INOaHKlnBj6l0JQh4cL1DSADuRsOaQtTO+kY0ksPZhisiJB6R6/bD3DBO0EdEcJYxY4rnnj3dGb2LmAZ6s57sy1UufB8AiWlXavNeRnQ5nE/ky2l1aFq0d/OkkNzHJ9J6Ij6XgXQc+iY35BP1ClhQIeWHfQZai Ofar4cUq K+lJaympUiRbdCrh1vVu4ZjdT+l+YeWgd4dUNP8xWgbJihbCN81nVwxcwYLAosI//UezlRfGetG21W7Rj5nOndF9oopSBfp4t1atXuscDL18GFbeP2EwwtQi2lm0hkWQAWJgbl/SmeO7wWMI5osyuuu9toiuJAgeRjiLvRviFgCtVT6J+/3+QejgLa2bWav1BhZmLeLmAE9wAPhlfp7O6zQtwxXY4BGLg4b3Tgjm3yCPImf4dn+eumlYZlmLEl0KmWTKGJitgNyl5iisy0LjrKzSSiXwrSCMMsKX0kMEAhFZQP8JzWV3Qxvu5GZrQsxb97dSeh616hRshgvXkBzd/mZK2SQYRkFK5GeqOkqjdR66Xmok= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed 24-04-24 05:59:39, Breno Leitao wrote: > 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) > > > 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) > > > 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. > > Suggested-by: Shakeel Butt > Signed-off-by: Breno Leitao Acked-by: Michal Hocko It is worth mentioning that the race is harmless. Thanks! > --- > mm/memcontrol.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index fabce2b50c69..3c99457b36a1 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -715,6 +715,7 @@ 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; > > if (!val) > return; > @@ -722,8 +723,9 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val) > 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(struct mem_cgroup *memcg, int val) > * 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); > } > } > > @@ -5845,7 +5847,7 @@ static void mem_cgroup_css_rstat_flush(struct cgroup_subsys_state *css, int cpu) > } > } > } > - 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); > -- > 2.43.0 > -- Michal Hocko SUSE Labs