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 629E9C83030 for ; Thu, 3 Jul 2025 05:51:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EB1906B00EA; Thu, 3 Jul 2025 01:51:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E890C6B00EB; Thu, 3 Jul 2025 01:51:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DC6076B00ED; Thu, 3 Jul 2025 01:51:30 -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 C835D6B00EA for ; Thu, 3 Jul 2025 01:51:30 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 739B71D832B for ; Thu, 3 Jul 2025 05:51:30 +0000 (UTC) X-FDA: 83621881140.10.AEBFA21 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf30.hostedemail.com (Postfix) with ESMTP id B5ECC8000F for ; Thu, 3 Jul 2025 05:51:28 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=HiyhC5Mr; spf=pass (imf30.hostedemail.com: domain of dennis@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dennis@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1751521888; 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=fUcUgQbJvnDcuCQQNgBm0DxmPq77ovrLVQ9NxxTzTrA=; b=3vGiv4ceY7HHmw2PbkEFxoyA/A3wnFLJp5TWgdgYagMecmiFvpTGHe1vtR7RXYU9lkU/Hr CYB/b9UI/xnrFNhh2i+CjakF9KNqSFTRrpvo0GlrSutRdn+BgUgEK7tBTLLG1/86zXIk6K JQCn5AG+NecFhyYz2zXjdUHHKNvLw1k= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=HiyhC5Mr; spf=pass (imf30.hostedemail.com: domain of dennis@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dennis@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751521888; a=rsa-sha256; cv=none; b=z87Hc7F63VeVmQtPcMayp4GOjgSx8JMAvyk/RQTna2VQSDpUwBKDO8H+uPUZpnm+2C8waF XT32+jRPQhBBHXAu3RFjIl/HHbgMFiCFvu1s9EYHgwbmEU+sbe0P5XSTiVaj+lJdoSiDRC yVrL+8fbCZs6GaxaD+Zx3/aDRwjBIFk= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id C50215C48A9; Thu, 3 Jul 2025 05:51:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC59EC4CEEB; Thu, 3 Jul 2025 05:51:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1751521887; bh=1m0z6fdjZbCbVibUE1igW1GCEx8THMZqBilXOkv53uA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HiyhC5Mr3lZznq9J0P9qgJngPHFu9pvXSoCe0dBj7OjEzo7qGD5FfBL5unw2bhUqy IYgYoAMhIGd1szT/VCtbT/oPTy1PHWFK/wBzqad3Le9Mhy8boia7XuhJW3pT0AQnF/ 3Vyu98WyYmDcGU05JihVqBLyAozAMaZaVMYXCDVR5jxwCktLzP/PFCwjyhugySQCWc HjO819D56u50Lfw/UFrd8BxNBAMAwgHSsnJefO8xcZstDOIIcaKqxXev3M2qorBTgS q14YQs9/27mWXOc3ek2x8XPvPMPx7mE+YB0XBW3Aka7q9Hbv0IjjIdMDv3V6SUrX2e tG5Re3hd+ZHIQ== Date: Wed, 2 Jul 2025 22:51:25 -0700 From: Dennis Zhou To: Jeongjun Park Cc: "Christoph Lameter (Ampere)" , tj@kernel.org, akpm@linux-foundation.org, vbabka@suse.cz, rientjes@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, syzbot+e5bd32b79413e86f389e@syzkaller.appspotmail.com Subject: Re: [PATCH] mm/percpu: prevent concurrency problem for pcpu_nr_populated read with spin lock Message-ID: References: <20250702082749.141616-1-aha310510@gmail.com> <7b7d353f-f38b-3205-8fd4-1072dbf69cb6@gentwo.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: B5ECC8000F X-Stat-Signature: jia8bcp7d6gkjiq7a8ofta7bmr6ufwdx X-HE-Tag: 1751521888-850617 X-HE-Meta: U2FsdGVkX180jfIJacoJFHsVskzLisKOJpcKYqM/hbmTgt8gQTZtihyAxifZW/1++vxSeEIAOrNFfgwLTrsvdz/tLs9gwnNnZYe9Aw/OddvdNUnSmwmho9qQWPU44NuLmOgrPwgGMTAGi4fpaUgAwwiq8NAnFhw5ew45JaX0jZ2YWWyf5LhSqRInP7BJ6cGbU4K64gJmKmELPjKa74szuZPZBpiT0eaKnv/GWZpx5iJj9mK0xSW4m5fowICvQtL3nP8uwBo8l6SLUVfMNiwwzBT/BNqNcGN2gSTKZondlhL5i7HLygVeDY+VXMc0EZv03Y1c56fJhMaOp+CaUUd79WlZFsDIqRNLOU+AdVuA12vUU2bW65mItU+xktVKuVm9i2nBy3Km7RkpXA+U0aHaHvR3aZncbhj372W+QuisAtPJG1NiLjwd24X27oi2csJHaNmog3sn4Wz/OnioCEBR+A9qXv/xW0FZAw7lSRTIe3RWL9N/BuuIO/w87CTAm1zVCIFwterUA+X4+JVtM3Gf3TBRdHkjrdyje8PQ+txUsrE/2a4orOJi3rD6ZpMSEQ5nYY7fa7jEA6btBq5CQUBp9PQW0LThBy8ytIJgnp4t2/YMaXtOrB0j5PIbbEtOidpnX7WsIS1jTPqtXfUHrYg7/qG9L2iKI1EXFjHt8vwUf/2F0HePn7YI/LjFpvCWqeeI3GtzeNT6Ak+oIJsEVLJmdnSLTxfKy5M/yJ5I3WAhJAUDKOBC7EoF6B/0cThISUc7UBWZ7aeRlG5ZrvpecvaLCYZLgjQq9bylGsANvAeyqqrbpbCUgnjIQPE6IsR8SqAiFkGjD5FK0xoF362LP19DchbD1BpoasEmfUOxLoSZdWpPAwAgNv2Pj1iMeH9nwi8fFlVhXECVXTFXl1BZxlKBG1/giOnBehPpA0yy3q+cQt9oHQVAwxqJez5UJd7GUZXK7Oci2bd4OKE5JNJKhhw RfZQ7qox lDGyZI1I1qLbtZeweqyYEcPomwFCQ1v6urBrkH3enKJvVHMXTId2mjGwvLzvy5nCkslBSypVvWdRB+xUP5lozzsrj/Xf+ht/VqestnsbGtVhLQORTkAnAVG2tcKCkblzYlUIXTzAOoOsznIZ26jCjfXQExGLw1PY3IK2NNjHWPu+nSbtnVCcUIQCRwkaGSBfBjwUjUzSvUfBS3oV2QUGmFBsQlVbBJdLF01Xh/JhSpRu69M+SWUUIrX6GUIvtvSAmObXoQuzVEJF/OblSZ11pzWSTv4FdsAI8fvSjen4hU3FVHhMmw+bCeT3uw/FJBAujG4KX+ZSyyXtF616jFyuBhGAwqm/mlZnDq5EvbxJ3TwWVavsKm+XQrM427+JbhKXXO1sAD4jCcOy1YbqJSMxALcrNFvhT9cYakVDfVUSjdacRI1oj3eicACDnRa5iSWE4SuYB34Sv5u60F6mbVcKfvmFT3jo09evzoi2Cv9yY5SOYXgJGuqwn23KpHUXjfwpKc39p 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: Hello, On Thu, Jul 03, 2025 at 01:45:36PM +0900, Jeongjun Park wrote: > Christoph Lameter (Ampere) wrote: > > > > On Wed, 2 Jul 2025, Jeongjun Park wrote: > > > > > diff --git a/mm/percpu.c b/mm/percpu.c > > > index b35494c8ede2..0f98b857fb36 100644 > > > --- a/mm/percpu.c > > > +++ b/mm/percpu.c > > > @@ -3355,7 +3355,13 @@ void __init setup_per_cpu_areas(void) > > > */ > > > unsigned long pcpu_nr_pages(void) > > > { > > > - return pcpu_nr_populated * pcpu_nr_units; > > > + unsigned long flags, ret; > > > + > > > + spin_lock_irqsave(&pcpu_lock, flags); > > > + ret = pcpu_nr_populated * pcpu_nr_units; > > > + spin_unlock_irqrestore(&pcpu_lock, flags); > > > > > > Ummm.. What? You are protecting a single read with a spinlock? There needs > > to be some updating of data somewhere for this to make sense. > > > > > > Unless a different critical section protected by the lock sets the value > > intermittendly to something you are not allowed to see before a final > > store of a valid value. But that would be unusual. > > > > This is an academic exercise or did you really see a problem? > > > > What is racing? > > > > > > This patch is by no means an academic exercise. > > As written in the reported tag, This race has actually been reported > in syzbot [1]. > > [1]: https://syzkaller.appspot.com/bug?extid=e5bd32b79413e86f389e > A report by syzbot doesn't mean it is a real problem. A production problem or broken test case is much more urgent. > pcpu_nr_populated is currently being write in pcpu_chunk_populated() > and pcpu_chunk_depopulated(), and since this two functions perform > pcpu_nr_populated write under the protection of pcpu_lock, there is no > race for write/write. > > However, since pcpu_nr_pages(), which performs a read operation on > pcpu_nr_populated, is not protected by pcpu_lock, races between read/write > can easily occur. > > Therefore, I think it is appropriate to protect it through pcpu_lock > according to the comment written in the definition of pcpu_nr_populated. > You're right that this is a race condition, but this was an intention choice done because the value read here is only being used to pass information to userspace for /proc/meminfo. As Christoph mentioned, the caller of pcpu_nr_pages() will never see an invalid value nor does it really matter either. The pcpu_lock is core to the percpu allocator and isn't something we would want to blindly expose either. The appropriate solution here is what Shakeel proposed to just mark the access as a data_race(). Thanks, Dennis