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 87E6CC83030 for ; Fri, 4 Jul 2025 04:45:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E621344016F; Fri, 4 Jul 2025 00:45:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E135744016D; Fri, 4 Jul 2025 00:45:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D019D44016F; Fri, 4 Jul 2025 00:45:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C00D744016D for ; Fri, 4 Jul 2025 00:45:02 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 71B121417A3 for ; Fri, 4 Jul 2025 04:45:02 +0000 (UTC) X-FDA: 83625342444.07.570ACC2 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf09.hostedemail.com (Postfix) with ESMTP id A6759140005 for ; Fri, 4 Jul 2025 04:45:00 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=QNT2sAA9; spf=pass (imf09.hostedemail.com: domain of "SRS0=LAm/=ZR=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 147.75.193.91 as permitted sender) smtp.mailfrom="SRS0=LAm/=ZR=paulmck-ThinkPad-P17-Gen-1.home=paulmck@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=1751604300; h=from:from:sender:reply-to: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=2ol/BOqOZ2xVQM/qGbItf09UjqgxLc4bYsvygH5QJsM=; b=2pzGzlswBpPKxQcOwfOH821hp8eeSJOC9uwFmdDSG5FIhg9OlqaPD3vw4KH8MJLOVENzsb woZgjqLpffEP4/lI7UXSJJXz5obedQ4Ed1+NLNQfgL2RVypoCjUkt9B+hqaSRRfgm0/Rfq 3uRDGGUxwXxE9wr8wBXY5wIXosIfhVo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751604300; a=rsa-sha256; cv=none; b=FxgtAsi9LBgYCV8WHRTCDqnQMsFRGghqrx+9lSFm0h5/DVfAgkiZpCwH+XdGh1rX/BvrbJ zIdtmPAszjuIMi1VJesV+q/f0HEO5Lh6+AbbNX9mZNf3devt++/36MAuje9bhXTXtNdNve A5dwk3G5RuoWo494R0Uf4iF6dfWIK9k= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=QNT2sAA9; spf=pass (imf09.hostedemail.com: domain of "SRS0=LAm/=ZR=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 147.75.193.91 as permitted sender) smtp.mailfrom="SRS0=LAm/=ZR=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org"; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id AE0FFA52E55; Fri, 4 Jul 2025 04:44:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 520A6C4CEE3; Fri, 4 Jul 2025 04:44:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1751604299; bh=8LL5dfM4+fApXtGjqf/V6JOtfextTXNNY3ao2nJLhgI=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=QNT2sAA9S6skXbsMPwoX70+PnJmgNecblMkkgk7iB9K5OUglNMjplv+Sjd5JGKGtZ MBnP9FcS1HflpNA2loUWJZszkknB7LUlYJ4YpPasZL1piFHrZSdm2qzwijxFeBOQTD BxqAbmx5eCaRkIn41Jw6Je3dh96dPZwV2VTQxYAAE9lQ4wFct3iB5DLmo5TkPzxOYM JokDdLbmWKiQYvklkPVK4+5RfIvl07RnDQlp5e0sIGVQi249xXB8O1n8EZGvMsNpdN z14Yx542Xq01u+jKKrbTqDVU7QVNYc4fyXc6rAdhj/nkzz+tXM0WhfipDW2YUGN33L LMMTHNVAsBfBA== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id DAFEFCE0D3B; Thu, 3 Jul 2025 21:44:58 -0700 (PDT) Date: Thu, 3 Jul 2025 21:44:58 -0700 From: "Paul E. McKenney" To: Shakeel Butt Cc: Tejun Heo , Andrew Morton , JP Kobryn , Johannes Weiner , Ying Huang , Vlastimil Babka , Alexei Starovoitov , Sebastian Andrzej Siewior , Michal =?iso-8859-1?Q?Koutn=FD?= , bpf@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team Subject: Re: [PATCH 2/2] cgroup: explain the race between updater and flusher Message-ID: Reply-To: paulmck@kernel.org References: <20250703200012.3734798-1-shakeel.butt@linux.dev> <20250703200012.3734798-2-shakeel.butt@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: A6759140005 X-Stat-Signature: gxapj9c3at18agn8d8cgh3hrpuco4za6 X-HE-Tag: 1751604300-4613 X-HE-Meta: U2FsdGVkX19CkywobwjzPOTefzuz0JwUVkPD9umUssrB8NDaEtiahY9IdVzLyWBRGyWmcOABTNz1p0AlCxVwy+RSqRThnhmHkD1fuzMgIaSsxrOQX1lloA1ti9uiLYMaAyFRzV8vwNg4HfCzLIYl/eRQuc3NXPhT3C7cZr4cO+NpW6zassLCH8TFLryoRvtD6CutO3qLpvRhCaS8dGVA6asWtRkzRas7OwycvytkJo29VncbXOIrz+kKHL+2J4XT1/IU//XhMXVBgPgiEJAmr1wfPATbZSrrgtPTQ6aAyaIl2La8LQqmi7BAL3jqUs7/3fMnBOdxQTQxP34D02bf94hjrwmsTaaUa6YECLRJtMXxGLE45LXcTVD1iz4M0bmv3QFd3INSuf+72n56oEuanAk5NlSN9djXiVSfd0QE2XXibLohn+64Kjq5Bd6aLefqPe271VbFqb2TTbKYoHniWRPzWuvYrYtPI489dKBSbWIg+5AdnbFb0cnrcxLvSoa+zkKXv1xYaDIQaYA5MNlKWX+aPr7qf9ogquV+rqyQaGdJNKcEO6SLc2gvC4yOngnCXeL6ErLFFuOKiYWiMSG0yZGrJDg1SCDhiwKy7maFi6nvPC8vDMdsTUaeqDwv0R2kJ/UHxDK/GMIXyCfJTpY2XGWeGTF2Vt843Iju/i2gZrzy94wl0iGdlpE1TZjtYxNWCnD/uKWfvQwkJbhMau9+kUcvnalG5RWJuDkq6ZY7BzcQ5DFmEfeJowgS8L1NZ6PX8nsaHSRdt2keYN0iBhShUmPP3k3MHAlBYW47929WpnWIwBmId8+s9ODhVZ2W/VMppD4VWYSB2v3qNzLf0wVLf+Z8+3fPZIPgZRm6U5KwI8bEkc0FciH4B3yg4k5PML2Sbe3C6q77Deh9NiF5LulWgCrZEvhCbrU9vgO9PA29MHVs0ms23AoeX2PxypMe/toEtT6IC4TIts+YSJWMbX8 1+9Xmq7s aeu5jIwZYS2nWwTJFCHYRNoJvto6EvS3FvmjDhmPcLt9J97h3sC9S5Fqf4lfgRBluvw2kHVBKuoS/7t9MU4lEEWuKCqubg2BwfFg8oGzLIzol2E+FzuR6vjnRkn3qWSRSYBzzVTWOu3u28DA68Ocpky9XjnSm2IpRc0oRlkod4+6BwMA+hX2pSeU0xcXnv53vYEIYB0xd7VXILMIrgy93c0xRvvJt3thJlCDwiNns+lLb1lycNttba/9Rup6j4vV4ezgXcQryGqGpXny/kRWwakbUmvo7hRNQzGKGebJXt8XIETOCyGCgW1ltgtHzKwfGW7pFRb8XZe/ZK7LECL+2Qdx2j55jbPZW5IFpbV+LzEwbhh6iusTet3mhCSmX2P7CSjeU6fcXMEo/88OYl4tQFY2O5Wz53YcKtirxGv58WTZXRcAHZv8kjRXvlB3JyQvw3aTKSWjheqdbWDINOA2CvrhCJFJ+VEzZMhhJWgs6HSj5cP1pl3y9PwFmWC/dnhfoQz+SmnkQkHa+Lwheemyb7Zr9f4uD2cwIiqChb9IPXZsQyZ4GC/ZStMfLI6qlbtPzcTRyioGs7RfTrBX6WL3LEIsHHAQShTUZTYruGMUVAXbKor8Hb7xEsL2XyTmZ080vBzjryIz52z73d/01O3LvyLmqRGmkuKOr/H3MF0uogcoydJo4/L52wOg8vNpJ2342hDLgQwep0sCyllfkEqp31ykh3R8b2ELyjNu0 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 Thu, Jul 03, 2025 at 06:54:02PM -0700, Shakeel Butt wrote: > On Thu, Jul 3, 2025 at 4:53 PM Paul E. McKenney wrote: > > > > On Thu, Jul 03, 2025 at 03:46:07PM -0700, Shakeel Butt wrote: > [...] > > > Let me answer this one first. The previous patch actually made > > > init_llist_node() do WRITE_ONCE(). > > > > > > So the actual question is why do we need > > > data_race([READ|WRITE]_ONCE()) instead of just [READ|WRITE]_ONCE()? > > > > You should *almost* always use [READ|WRITE]_ONCE() instead of data_race(). > > > > > Actually I had the similar question myself and found the following > > > comment in include/linux/compiler.h: > > > > > > /** > > > * data_race - mark an expression as containing intentional data races > > > * > > > * This data_race() macro is useful for situations in which data races > > > * should be forgiven. One example is diagnostic code that accesses > > > * shared variables but is not a part of the core synchronization design. > > > * For example, if accesses to a given variable are protected by a lock, > > > * except for diagnostic code, then the accesses under the lock should > > > * be plain C-language accesses and those in the diagnostic code should > > > * use data_race(). This way, KCSAN will complain if buggy lockless > > > * accesses to that variable are introduced, even if the buggy accesses > > > * are protected by READ_ONCE() or WRITE_ONCE(). > > > * > > > * This macro *does not* affect normal code generation, but is a hint > > > * to tooling that data races here are to be ignored. If the access must > > > * be atomic *and* KCSAN should ignore the access, use both data_race() > > > * and READ_ONCE(), for example, data_race(READ_ONCE(x)). > > > */ > > > > > > IIUC correctly, I need to protect llist_node against tearing and as well > > > as tell KCSAN to ignore the access for race then I should use both. > > > Though I think KCSAN treat [READ|WRITE]_ONCE similar to data_race(), so > > > it kind of seem redundant but I think at least I want to convey that we > > > need protection against tearing and ignore KCSAN and using both conveys > > > that. Let me know if you think otherwise. > > > > > > thanks a lot for taking a look. > > > > The thing to remember is that data_race() does not affect the > > generated code (except of course when running KCSAN), and thus does > > absolutely nothing to prevent load/store tearing. You need things like > > [READ|WRITE]_ONCE() to prevent tearing. > > > > So if it does not affect the generated code, what is the point of > > data_race()? > > > > One answer to this question is for diagnostics where you want KCSAN > > to check the main algorithm, but you don't want KCSAN to be confused > > by the diagnostic accesses. For example, you might use something like > > ASSERT_EXCLUSIVE_ACCESS() as in __list_splice_init_rcu(), and not want > > your diagnostic accesses to result in false-positive KCSAN reports > > due to interactions with ASSERT_EXCLUSIVE_ACCESS() on some particular > > memory location. And if you were to use READ_ONCE() to access that same > > memory location in your diagnostics, KCSAN would complain if they ran > > concurrently with that ASSERT_EXCLUSIVE_ACCESS(). So you would instead > > use data_race() to suppress such complaints. > > > > Does that make sense? > > Thanks a lot Paul for the awesome explanation. Do you think keeping > data_race() here would be harmful in a sense that it might cause > confusion in future? Yes, plus it might incorrectly suppress a KCSAN warning for a very real bug. So I strongly recommend removing the data_race() in this case. Thanx, Paul