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 2F529C4345F for ; Fri, 3 May 2024 11:28:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B87476B0088; Fri, 3 May 2024 07:28:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B38686B0089; Fri, 3 May 2024 07:28:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A26CE6B008A; Fri, 3 May 2024 07:28:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 87D416B0088 for ; Fri, 3 May 2024 07:28:52 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3F18F161094 for ; Fri, 3 May 2024 11:28:52 +0000 (UTC) X-FDA: 82076862504.20.CF44911 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf05.hostedemail.com (Postfix) with ESMTP id 2B811100010 for ; Fri, 3 May 2024 11:28:49 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=qMole+1Z; dkim=pass header.d=suse.com header.s=susede1 header.b=RaV1Bx9g; spf=pass (imf05.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.131 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=1714735730; a=rsa-sha256; cv=none; b=T5KRED0VsRUBkNYtgoN87Imbl1yON1GRfaE2swXswgoiAnb6SQto1FgGGk8N1v7/rtNp5F OPdliY75l7PCNCQaloC6OUHdfbNJGOp4r+9gImmY0qllHCfxgnhA5E20DRPQ8n7KKpvyGl Kudf9b8p9UVbI1gundphr8aGoWW8IrA= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=qMole+1Z; dkim=pass header.d=suse.com header.s=susede1 header.b=RaV1Bx9g; spf=pass (imf05.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.131 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=1714735730; 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=P9yMKpP+WtXCZbyBz2t9/3WxqhYBErFHeUW75pJnqQ8=; b=o8VVn39HKwKjy2Cy6s+Eq6h+iOgCW2xd0TwPZ2UKU1t5dABTf0dnR/xr9X3EJep3DUAGdU KHI7o9CKEtwL0fXHLjZIMPrUSRopuUQhoZR+KZkJWlOxFadLApKs8rjSbtEmCOthAIDYE9 W2csTwB9Dl0wpHT+KehF0G167rEgmd0= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [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-out2.suse.de (Postfix) with ESMTPS id 6A64F20254; Fri, 3 May 2024 11:28:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1714735728; 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=P9yMKpP+WtXCZbyBz2t9/3WxqhYBErFHeUW75pJnqQ8=; b=qMole+1Zdn0F1+ZqrG6EUVw5BjK9JmJ/rWM42iSp1qhhaRFnfLatmTJKZ1QVF47iRkZhbj lX6qTmtqvXkGpFyKOoKQI6S/5vcHacEXGRixfkQ8bCecLcnP+y/1/rJzFaiJ5Ga1o8zEPF UQ9G5e87Ncq26CecfQEajUTpHUOGkGk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1714735727; 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=P9yMKpP+WtXCZbyBz2t9/3WxqhYBErFHeUW75pJnqQ8=; b=RaV1Bx9gMOPXbKF3N6rOhGhw1fHnczyZ1plP5vjk2ZRWIo3sZd1ItSM26Xqg+gUDWJ/MZr 9UTCuRye3t7jnBKyDQfP7+5+9XB5XWKuVEl0LdoxBOenusm/k3yU8QdQfb38G9DN7TQWpM JYlOVIsMT/Wp0RETs6GHPHQV1Iprnas= 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 510AD139CB; Fri, 3 May 2024 11:28:47 +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 LDnLEG/KNGapWQAAD6G6ig (envelope-from ); Fri, 03 May 2024 11:28:47 +0000 Date: Fri, 3 May 2024 13:28:42 +0200 From: Michal Hocko To: Breno Leitao Cc: Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , paulmck@kernel.org, "open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)" , "open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)" , open list Subject: Re: [PATCH] mm: memcg: use READ_ONCE()/WRITE_ONCE() to access stock->nr_pages Message-ID: References: <20240501095420.679208-1-leitao@debian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240501095420.679208-1-leitao@debian.org> X-Stat-Signature: 5jz39bhf5qpme4cc4hkc7qdzgjuqwd74 X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 2B811100010 X-HE-Tag: 1714735729-869473 X-HE-Meta: U2FsdGVkX18XSqZQULLoxYPhAmU0Ktuhl0uOvdAC5C7coCo/8ecHfebToYZAy5zs8BIHEbSjw82VOQ8C2E7DMRujf+MN/BbFR1DLOAxYn0m3iMqu2sDU2xVzC0zwY8xgOFWCsoPyl1V6TXfeM9BTGqwaooaX7U46RVbShUXaH+Sceye4qe2EWFWrsITxAKJM8qXRsguhftcA5a/HYhko6FPb+1FB+SAsEYnOprAip3ZAxDwHXUw8Ttk2ofnVsavxgXei6Vin/b+FdUsje+y+1xB45TTGM8YbvWhsWC1gQjwncUQiHaIL6IaBlMzRpf76I/ni/39yJiYpU0qSmj0ynFGAqGJfS3vbRAK3iwUsQh8FPn2+faVcGKQLjrE12ofilcIsJAhzbYeu+Elrju5LLQ33ctEMHuzL90aQzBjJ+f+Xh/p5lfvJj3FCIOxmf3jJ29D8MnlfjrG6iDlhZ/ZfG+oknhqCZcllAelwLA1gUY4xCJ0dJP1Jyl8+JV+kajOoJPFknfkHyv/fUdOkg9N4fdqHjhbuWofo6CDP3ZHbq88M8bfzalUxQCn9perAE4Nl0hYGskn0dPE2hGSc4/srAB1dYFJpY90bCSeb9ySJj6g5LZ1RoTmvHs0QnHYBlFll3P/htGoOVbOl7COlJRLOfhF4OP0WgojEOhfn5B7ypPQz2qJeBD0rk5uluC3/m+lITo5n0K8wQCAx//1ESXn9m/p3k6P9FhZSiCi9bPly2fZ/M8sFetxN4buKLt1HFFPp/ZPG4gvfNXq2oJ7Re7uhdaOlmNhhMiwGxjepdsyLsYJxU5VqPgglP9AFRM71HdYuai5bIrgvho8Ua3UlqIQZ8fFiPx/RebAJpUg0ZxtWAahX07Ji2TLDmMQbg2wOLz+NFm6d+Ymxk4LPrYPQOmpLdRdhP/M4UeCtL6NpBhUl8RhEg7TZ17ae5CqCBHXazgnC8v+7dHhuLcWpVlAwpfG U2l3l3o4 28m6B4AtJGnr0FLmz0N0wY4I9aiiNBdcxHnbiNpPUj7OUK8VhMoyBc5vMnyNeHQ/zwo2hIAwWcZIaUgevzqHFBBtKD+Ajc3Qw/sSI4L7qwZySBnKhPkk5U0nkfdpq5ye1EaGfr4JiGEvOVHDG6SYCmrrmnxZffOkqWSHPyxyCNcUEH8P/9Xwa3G/jQmOn+mgDpKzEdAXa+BgVlikYzNSo5Fpc6rWcWRjc+LgptX5VpvZSqdmrB6rIVMlzF+p5BsT1AojoQWldc3+12UhGnelutQ1P5tOTGdJWNrTrOqwFfT0kC685dGAIgqK/NKX5+/2vQc5lSK94POiSaz7KYtc7suQW5fpb3ywGwgHfsDSHYD4F2Kw5qNsgD4+u1eAtYORp53eRRo3Xp+UURs/Y/hJgu5g3sQ== 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 01-05-24 02:54:20, Breno Leitao wrote: > A memcg pointer in the per-cpu stock can be accessed by drain_all_stock() > and consume_stock() in parallel, causing a potential race. > > KCSAN shows this data-race clearly in the splat below: > > BUG: KCSAN: data-race in drain_all_stock.part.0 / try_charge_memcg > > write to 0xffff88903f8b0788 of 4 bytes by task 35901 on cpu 2: > try_charge_memcg (mm/memcontrol.c:2323 mm/memcontrol.c:2746) > __mem_cgroup_charge (mm/memcontrol.c:7287 mm/memcontrol.c:7301) > do_anonymous_page (mm/memory.c:1054 mm/memory.c:4375 mm/memory.c:4433) > __handle_mm_fault (mm/memory.c:3878 mm/memory.c:5300 mm/memory.c:5441) > handle_mm_fault (mm/memory.c:5606) > do_user_addr_fault (arch/x86/mm/fault.c:1363) > exc_page_fault (./arch/x86/include/asm/irqflags.h:37 > ./arch/x86/include/asm/irqflags.h:72 > arch/x86/mm/fault.c:1513 > arch/x86/mm/fault.c:1563) > asm_exc_page_fault (./arch/x86/include/asm/idtentry.h:623) > > read to 0xffff88903f8b0788 of 4 bytes by task 287 on cpu 27: > drain_all_stock.part.0 (mm/memcontrol.c:2433) > mem_cgroup_css_offline (mm/memcontrol.c:5398 mm/memcontrol.c:5687) > css_killed_work_fn (kernel/cgroup/cgroup.c:5521 kernel/cgroup/cgroup.c:5794) > process_one_work (kernel/workqueue.c:3254) > worker_thread (kernel/workqueue.c:3329 kernel/workqueue.c:3416) > kthread (kernel/kthread.c:388) > ret_from_fork (arch/x86/kernel/process.c:147) > ret_from_fork_asm (arch/x86/entry/entry_64.S:257) > > value changed: 0x00000014 -> 0x00000013 > > This happens because drain_all_stock() is reading stock->nr_pages, while > consume_stock() might be updating the same address, causing a potential > data-race. > > Make the shared addresses bulletproof regarding to reads and writes, > similarly to what stock->cached_objcg and stock->cached. > Annotate all accesses to stock->nr_pages with READ_ONCE()/WRITE_ONCE(). > > Signed-off-by: Breno Leitao Acked-by: Michal Hocko It is worth mentioning that the race is harmless. > --- > mm/memcontrol.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index fabce2b50c69..d3befe3b62fa 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2310,6 +2310,7 @@ static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages) > static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > struct memcg_stock_pcp *stock; > + unsigned int stock_pages; > unsigned long flags; > bool ret = false; > > @@ -2319,8 +2320,9 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > local_lock_irqsave(&memcg_stock.stock_lock, flags); > > stock = this_cpu_ptr(&memcg_stock); > - if (memcg == READ_ONCE(stock->cached) && stock->nr_pages >= nr_pages) { > - stock->nr_pages -= nr_pages; > + stock_pages = READ_ONCE(stock->nr_pages); > + if (memcg == READ_ONCE(stock->cached) && stock_pages >= nr_pages) { > + WRITE_ONCE(stock->nr_pages, stock_pages - nr_pages); > ret = true; > } > > @@ -2334,16 +2336,18 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > */ > static void drain_stock(struct memcg_stock_pcp *stock) > { > + unsigned int stock_pages = READ_ONCE(stock->nr_pages); > struct mem_cgroup *old = READ_ONCE(stock->cached); > > if (!old) > return; > > - if (stock->nr_pages) { > - page_counter_uncharge(&old->memory, stock->nr_pages); > + if (stock_pages) { > + page_counter_uncharge(&old->memory, stock_pages); > if (do_memsw_account()) > - page_counter_uncharge(&old->memsw, stock->nr_pages); > - stock->nr_pages = 0; > + page_counter_uncharge(&old->memsw, stock_pages); > + > + WRITE_ONCE(stock->nr_pages, 0); > } > > css_put(&old->css); > @@ -2380,6 +2384,7 @@ static void drain_local_stock(struct work_struct *dummy) > static void __refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > struct memcg_stock_pcp *stock; > + unsigned int stock_pages; > > stock = this_cpu_ptr(&memcg_stock); > if (READ_ONCE(stock->cached) != memcg) { /* reset if necessary */ > @@ -2387,9 +2392,10 @@ static void __refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > css_get(&memcg->css); > WRITE_ONCE(stock->cached, memcg); > } > - stock->nr_pages += nr_pages; > + stock_pages = READ_ONCE(stock->nr_pages) + nr_pages; > + WRITE_ONCE(stock->nr_pages, stock_pages); > > - if (stock->nr_pages > MEMCG_CHARGE_BATCH) > + if (stock_pages > MEMCG_CHARGE_BATCH) > drain_stock(stock); > } > > @@ -2428,7 +2434,7 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) > > rcu_read_lock(); > memcg = READ_ONCE(stock->cached); > - if (memcg && stock->nr_pages && > + if (memcg && READ_ONCE(stock->nr_pages) && > mem_cgroup_is_descendant(memcg, root_memcg)) > flush = true; > else if (obj_stock_flush_required(stock, root_memcg)) > -- > 2.43.0 -- Michal Hocko SUSE Labs