From patchwork Wed May 1 09:54:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Breno Leitao X-Patchwork-Id: 13650440 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 01B52C4345F for ; Wed, 1 May 2024 09:54:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 93FEA6B00C0; Wed, 1 May 2024 05:54:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8EF306B00C1; Wed, 1 May 2024 05:54:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7B7766B00C2; Wed, 1 May 2024 05:54:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 5B4426B00C0 for ; Wed, 1 May 2024 05:54:35 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 323EB1A0815 for ; Wed, 1 May 2024 09:54:34 +0000 (UTC) X-FDA: 82069367268.15.9263120 Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by imf08.hostedemail.com (Postfix) with ESMTP id 4C52C16001E for ; Wed, 1 May 2024 09:54:32 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of breno.debian@gmail.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=breno.debian@gmail.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1714557272; 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-transfer-encoding:content-transfer-encoding: in-reply-to:references; bh=WW4YFzextgKm5BBgMPVLqw63E3+hwQm6VSxunaunImg=; b=iQgiSXqepdG12gVgTiM57SDCAS+voG2WgGUKrvRloN7Uyp8dohcrgxtKTldm3vTxYifabz S04z7j+fRDqZf34MPrDOemTfW/JLI58HtKAP5NBgS/M++oGGLf6PfkAUs2hTW+BpzqBtA2 aj8xZLPHdPFpIWAW266FtYNODZveNyg= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of breno.debian@gmail.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=breno.debian@gmail.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714557272; a=rsa-sha256; cv=none; b=EUFaDrQgZ+jr59E82y0NC9Iokl1KEKtKTfJNJxdW/uu+EqUXrE4IaXh9qo6U3f4nCCt3wR nJJxvA9K2CzASb7EvUE1JH7bvl43X+1IFsXvZtqJsh0elpapPzHA3xzjSBb7II55uRcKOn jQhagZPtiLVMTqdIhtn1BA6FRZylZIo= Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-a559b919303so849970566b.1 for ; Wed, 01 May 2024 02:54:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714557271; x=1715162071; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=WW4YFzextgKm5BBgMPVLqw63E3+hwQm6VSxunaunImg=; b=g8fH51B53bUDvodduKKc1ZYLU3Pjsjzi/u3ARh3z97U3kOfvh980Tu3ggLo6M7ncV9 lsCouyrNdEVYI4IlOmDJkybBID78kp9ok2mMY4S8aMb8MMSHJM8/2zqfONOtHZlIlFqU no4xWpRx+4P8S5lPRCrF+WolayXTJnvV3Cd23m5uXYaYrwftdNv9WgtcVsG6eiBo04bK jhEvzTa1+3dh90iVxxZOhl8kO0UV+EiO+DdTcOpFK8+IiHj2w/M1POTFReW4koBXhIhr CbGf5QNW6muOGI3bW9joGxfUU46o48YEFBgiByKsUjKlmxqPBv0eGcO5twY3XdsAk4Le NHHg== X-Forwarded-Encrypted: i=1; AJvYcCUOVFu2wMkavnV71RngELueK9UgpeF5McAO7dUL8JUyNI7Sfxrnp/+QknS3vnfYyFxYtJi7RET7Jq6MuiJ0EQWCBrY= X-Gm-Message-State: AOJu0YwOCWOAvFoIYDpkbOMz/bbJn14SHc8uFgySGlOg7ktMPudo8Gls zGnf7GkUCS5JSllzNQDMroNO3qo9+X1TcuUF/TCN7+eGWFAiPc1B4pMtkQ== X-Google-Smtp-Source: AGHT+IFslaSpA43sRj0374yg9dqVLoR6TIA5xhmPQL4CDOEGxNr4bzGxXsIGDpXERDabnIuw8Z9npQ== X-Received: by 2002:a17:906:6d55:b0:a58:c559:3c83 with SMTP id a21-20020a1709066d5500b00a58c5593c83mr1642315ejt.59.1714557270581; Wed, 01 May 2024 02:54:30 -0700 (PDT) Received: from localhost (fwdproxy-lla-118.fbsv.net. [2a03:2880:30ff:76::face:b00c]) by smtp.gmail.com with ESMTPSA id n26-20020a1709061d1a00b00a525860ecc5sm16074917ejh.180.2024.05.01.02.54.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 May 2024 02:54:30 -0700 (PDT) From: Breno Leitao To: Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton Cc: paulmck@kernel.org, cgroups@vger.kernel.org (open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)), linux-mm@kvack.org (open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)), linux-kernel@vger.kernel.org (open list) Subject: [PATCH] mm: memcg: use READ_ONCE()/WRITE_ONCE() to access stock->nr_pages Date: Wed, 1 May 2024 02:54:20 -0700 Message-ID: <20240501095420.679208-1-leitao@debian.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Rspam-User: X-Rspamd-Queue-Id: 4C52C16001E X-Rspamd-Server: rspam06 X-Stat-Signature: iyy5oj4xnd4tdq86sitmwhbnwcqd53s6 X-HE-Tag: 1714557272-802816 X-HE-Meta: U2FsdGVkX19ueVDX+ZYM2StLlxCond9712m/PX3F21Er6JwaUT1kpKtzwejnKVso0QEtbhGKyPMBu5zQQZ8zkMWBwByJdJLvAgLl08zhlGsk6bszKQAFa8vrAUc3FFBNQ4u61ul1e8Mb18FSPThZuSGuveIljCz8/rIfb19jlCq/BFt6geMxgftU6xaZyiWOYWrPCdN2XFTwDNuoneu5hhNkJfaPtevsyc8mQqfXkX0SWiIH6mpequopGG3qH139FD9nYSEDNy3+1752UzWZ6DewklyRb8kzL+IhvkBFgH8s9ElvnDUSAUnlaL1FkaaEN2qlnt7WfektX7AZ39cstuiEUITNqUZIiVeVfdfaEfOQXLfxSqFQlehxHd2AKBHMa6k9AY+eaI1wGSbbX5b69OQHDjvXlWJaoTp9a9P2pJeEAATMJn9dsv0mRr96K8NzfZDhxSikWzLFr6flvfdYsMjkXA32LZbQIqVXw9EXRUtAVyQ+yKCge3NMqx5P9Ygl3qCZX0xDb1Y66b9jqHWEaPa9s8C/2W6BotoQUmAX6/CV1t3xM7eaWAP4hsghsPzpUj3Bun2rYw0NbOwGhxLm8mk/a+KK6jQ3uleFk5Wo3wa5EkWTvwhTNxz4GCibjFFH9WHg0+tOcQ0JRSgv06DqG6OJuExxWQ7sL09xivdtpSl/LiaE17aQ8TXjmdvuyGqRdsL+qhpmq88qDhK6cMNRhUD3LzzzcIE7LrzzrrhUCivV40LQrY6DEsR7IPNYeb6wdsR33rHoIEA1/dj17kaJY7MPYJb6cWe0AbXlL5AXh8vdamyct0WnuooWBVruC2c2+/Z1H7ODprYMi1kuRkn7KVxdRu1oNT4bbv116lQH5lUUcyqe231iWRvhZvWJCPEiTNQYcFi8q+4IOcYfDVMRNhMhxoVqPwgU5JS1l4g1QUcbXpgM7H35GeOdsz+BLANPICXJhXhgkTJ8fg0uovd uux8VJL4 G62oSDZh5HGdaFOOKwfMos3VhrRS2LlxcD0GKrKl5XHnN1njEqE+pElol3qP7V6wu2YVVWkpIqBKszOlGKsoGdBcF3BlYcsRnr2pp77+SBk7tiRZhTVDvPfFy8cFXIhPzFvSgoq4/ocTYuuzIUTS478K3iLyFmMvDOEDApy2FMCYz+iwHa5JiONs/fPRnMYWF6J1hmFCKRI8XF16IQVRLj+WVyNzkmnqCCgCb5KqXftNF2iEA/k2grt+mKLfQX6b0ZxRjNgYxdQDJxtydQ9pHik5LzmtA8QAAEkns/W2soVbfWxGjyE2IoJTIqNmxMgJ2Em5s2FxVxh7xsjkF3MciYtfwneO1JNqOUPVDf5rxqpmtNaLMZ5H1L8tIHBXuVjs/D1gAWllnRkfs59T/FjJPbJ57/OxW4ZUMlG2beuVcYe11jPJD15vYC0e3qhZRdnYavBzpP2bO2GSqxGcN5+MA0IXyRYAs91s+zDLh+c2Az4UM4EycKXnnWrzDrh/I87WC0Dgz+NzOKKReeGG0mq0JgTV+bV3sxs9W2K/c7hE3d42BnpAz0+StknmeqfYZ+Lk/Za+7ALbb87eiEJAXccuegyKoTuWlABpMvBJwnSqnbo8fzPtxTHXZ70NjRfL2j1TWTB5Ka630q3gT/TlKQGy7fXkOmvZc3G2jHNyjvHCWS5wOkhTjYKGEBo/7C6nByM0hxf3TO2HphfQgZEVz6hjYYUxk/ptQpiYEXiDc X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 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: Shakeel Butt Reviewed-by: Roman Gushchin Acked-by: Michal Hocko --- 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))