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 B32E5CF6497 for ; Sun, 29 Sep 2024 17:07:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 414A68D0010; Sun, 29 Sep 2024 13:07:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3C56C8D0002; Sun, 29 Sep 2024 13:07:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 264D58D0010; Sun, 29 Sep 2024 13:07:40 -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 073718D0002 for ; Sun, 29 Sep 2024 13:07:39 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id AA3CA1C7343 for ; Sun, 29 Sep 2024 17:07:39 +0000 (UTC) X-FDA: 82618407438.08.F1EDA70 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf21.hostedemail.com (Postfix) with ESMTP id 0911F1C0015 for ; Sun, 29 Sep 2024 17:07:36 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="t2LXR/Do"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of "SRS0=+mCV=Q3=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 147.75.193.91 as permitted sender) smtp.mailfrom="SRS0=+mCV=Q3=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727629555; a=rsa-sha256; cv=none; b=47an3sretR7+q1dxTuzH0CM1rR/dsxn4Hs4wh+cLu02Hv7pMhq1N6LedEPWwKqfMX4JLvl CsoHzT3Ojm7Yn3FdOuq6z0zNqWJtVbYd2bogKaqqefBxdu2MMErOVeUauPF0YfK5PTxnfK Yqy+o3XB8u5rcXnl54ec/LTBs54Bn8U= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="t2LXR/Do"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of "SRS0=+mCV=Q3=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 147.75.193.91 as permitted sender) smtp.mailfrom="SRS0=+mCV=Q3=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727629555; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Ig2A8FyPUNuy4geHYLJWyQ7A4wlx47iTow3Yypxy22I=; b=JgX7JnD0U+Q4iTi4+3rI1J6WvMC3KrRLqZwbgU7y/9T6YKLo11gsUr1E2HQ3gA+kf5H8XU e9eyJ869FMsotO39BNTMIRTfM/T+7Jjniz5n4rL2gpy2/aXhu2fZlY3b43/koCJqYKZq0F CZOoyhFA7pRunO9BhjZTgg5zSaL2QN4= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id DAD87A40DFC; Sun, 29 Sep 2024 17:07:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C6DBBC4CEC5; Sun, 29 Sep 2024 17:07:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727629655; bh=O7w8DeVwq7CwRT5PMbyZVQpfUrTlDNjY5NwQLo7OfxA=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=t2LXR/DoOMh4sF/JbbWKkcGIj78RzUgUI8rsWOJITBhe6S9skX8osKV86lRHspwJ1 tpYMKTXppM93TfsEcEru0yQz8rxkPjYPl4118Xa1M5HI2aNeEk8rT+kcdN7sVerTyr kswROxQkl3RFeliMR9ybHJXeo0nBBiUCG8izn5VsM/7D1zw8pNkpZIopW7RMUcDoSh brhwrd6Y+uWwi2vxh8/4YGF/vfJBmQ9NqpQ1uZyHOLc1UAxX7FJeaq0hutwHTzJK6a eCuUCucuyXbHZ6AF6OH/7t0czyr5NEN5GyAHjun6Ksb3yzjwHYT+zXBhgOIEVyvsuO bNImMGb9NCvtw== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id 6E859CE04D4; Sun, 29 Sep 2024 10:07:35 -0700 (PDT) Date: Sun, 29 Sep 2024 10:07:35 -0700 From: "Paul E. McKenney" To: Mathieu Desnoyers Cc: Linus Torvalds , linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Sebastian Andrzej Siewior , Will Deacon , Peter Zijlstra , Boqun Feng , Alan Stern , John Stultz , Neeraj Upadhyay , Frederic Weisbecker , Joel Fernandes , Josh Triplett , Uladzislau Rezki , Steven Rostedt , Lai Jiangshan , Zqiang , Ingo Molnar , Waiman Long , Mark Rutland , Thomas Gleixner , Vlastimil Babka , maged.michael@gmail.com, Mateusz Guzik , Gary Guo , Jonas Oberhauser , rcu@vger.kernel.org, linux-mm@kvack.org, lkmm@lists.linux.dev, Nikita Popov , llvm@lists.linux.dev Subject: Re: [PATCH v1 2/2] Documentation: RCU: Refer to ptr_eq() Message-ID: Reply-To: paulmck@kernel.org References: <20240929111608.1016757-1-mathieu.desnoyers@efficios.com> <20240929111608.1016757-3-mathieu.desnoyers@efficios.com> <8153f0f1-fd18-4ae1-9dc5-f9b725429cad@paulmck-laptop> <3a298b64-dd7c-4a0d-950e-8e5b98b39fee@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3a298b64-dd7c-4a0d-950e-8e5b98b39fee@efficios.com> X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 0911F1C0015 X-Stat-Signature: wdtwaqswn3fdit7znu3qjzgnhawg9fbi X-Rspam-User: X-HE-Tag: 1727629656-722476 X-HE-Meta: U2FsdGVkX1+D33Sl3zlf7Ai44d32o0sIYP2nBHvDsCw6eXNCk1Th5pnNdFLogwuZV/cMTmWRtQbgcr2JnoF8H9Myn7jB+tlrTXTb/sLkn4L+lQHEP30rZvlKr5uJuhuDskVP2+OwTlY4ACFZjplzQ0tt2cfIVzty75hmrW0/zfaBhGqIoTsYhIqrHURwwZ324sLh7IFyXCNE7NlwXikAKj3g++SqMBVwBD8WZXdWjDI8RtRStsaGVQ3S8f7cZrQILwgdMpXxIQbIdDhhxoMvFfzSs58ON5uOkSPpf5F5l0DawAFrJZOnHD4UjSVK4w8I2rMpyVx9JotYRqjERynDajwQUcFNeaa6ySJQHWQiSnk9Im6NQbApSyac3GYqJgE2SKm7PBarY7fB02a9QNL49QbPTFU8GSeNuJLZ+SPoyQqQzLAZc6frVPQ/MB3InSTt1uJsI0zTZ7I5L8hl53e3NrWU1VWFWx0SZY1Pyv3l/zOzPWYYInpBiE/va+1I0lt7hFbqZotZNxck+Q7O4ehwEby3C9KZAon8YKuts+qM0QBOZplm3kuEsUs5oRbGJP+PXL9wRR47E15c5FARVXkTxgDKw84PSwapKtZGy5zcn4GlRimK6lcz0Bf7H99bS8rc4v7TGxWEWo7jVFy9DKyimyEJsLGPJsSTKuY/PisK4If5JeMtonQ61hqvghFwKn3NXjykZviBjqzupoklqtcfL68p26+LvtNI+GVfrfULPYv1GaJkFR9v3njXDi5WlcLZzWsiCjgVkMRMuhbCf6OsI4lISlnS2yPqtplddX8GKR2Kw4Xdr4u3idNDpDYwaP8YCBHb4VXmUsG4RsrMf7kOElVHrd7qRqIVahxj4+vr60MBnl/bVKafzfI6ZgrCXA14nGr7lpjNozjpNTOAqaghEYq91kFRqYO6pOE6IZg8oA5uirmnqG4OKvzf0XK2arF8Ti2cKc64sAXKce0hrI6 qeRyvWBF 3J373E42UFOiDu+H8PJRF7Glbp+gHbVLU1Nt15kMxjb24WY9dBgO4m8XTjQgfiwxQ7bUbvUOuzQy75ATd3W5Q33XUqCglmbLhlPmboFvb1UBLW3h7axkLlpXIZH2utfzzydw3VORQg3xufzQqOaCZTF5CHqu/PdLdMaJt7lEe5jja4t/A2WMSVw5iHf+sQY7Al0vBn2AujT+LsTebSAC82XXZtfywKMDukM+5nlC2UA2funVLJP/4+ksqpdryh87lHd+I8R4ypMmX1YXeTVcBV+6/cn4ow9eX0INLDeYMs3WRdHVvyhz4dUkWe0Ypv3MhdbzmEeNTGGaTIhN4lNMSuWDnF72WESKguxeTkC/KSV4mATCdDijfb+pd3LQ8N8lw3b9VaggcN1nG5zEVIywHIQda2+RMTcY1vJpwelhQAWkKmuKhtXm1D0NUWPphX88goEDz1w5VQanbra7CdUVmLxnfeTSbs4wYXkhfLiVAquUS3rr7zcZ7nQRb9XhBJENDLmOsc7oA1xYvM182N814TCL0mDwKhxUEp4Hz7ycLb+yNVqS0MMiM4V4yblztW67bneITXb7NSPmyC9joT9o8AXJv6d277fuLa7wCJs9ZjPcY95CmEBN4FvVslh6w0QDCdVWJPXXay5+t5CErIvaoQgcR4lIw5CRNooqzMTj/2+ZIUhO/CD+ppt/abvVaoFWiE4zK5c4wYsE7ZJdWSRdfSvmbtD9dQVPwpFhhbf8LPKKpUQLAnzkLC6BAjGvAre1BHxkCEddbGfombZHVNN8xGcLxFm0iL3pGwEK5jiY5NEn4mxhTbYXd6lBtCez5161meBLUV6XWpqnZvLSlOAhslke0lBVOcGPb/itGvcNmhtCEqlot00lQWL07Lm+QFd+Z2ortI9q4mYVyWdiLkj1GTSAhTKueweLWDUJtYzqBLfeuwdpP2jM3w2Gh+/oDN2kIqkFp1rL38DYShxeO4IVZ6Bsq8W54 +dRQDcPK Z6a7QYTCU/4kNzenqB8vGO0lFF+sOabwr00iSgerE8uK1jMSVWbZiDBgefBXvpw7U30Hk4+yvMgukBRlPshFnZ7sQPed6h89xKh6e/OJoDB1Xz5JsqwKpEpug4YUSkVknTKk6C1ZOaTIkHlKw8jOhnc0swF9s0S+w9qeDsKoOm7n+mrCQCagEnKQhwGxq8KkLCDOG8A8TvKQVKeILlc72FjrWnwjL25Tnjb60paNWT6xsTGJdr/1O9rBKFvQZs3h5aehnRGTEqsh8yT4yaWC/dYWYHqLNhnETVk9Zr+B7QHdLsF7+gGI2gRL/juaPBaEnRhcs1yJS0F5+CldSYtEfHrzy6sxyn4jMNp9/vURwawGJupt4YxfoWe7u30xQcdwjEzJ3xr/nGPH3bPQs5M5DyziZvDaKZZOJqmebbqNGp4niJ4YrBZhANC6v+YszbAhrsiS0UZNNOqLX6vpSyZtAjp+85i1p6y81Gt1eCb1SGs8cm6t4yPbSiEEuEc67AHc 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 Sun, Sep 29, 2024 at 12:09:54PM -0400, Mathieu Desnoyers wrote: > On 2024-09-29 17:51, Paul E. McKenney wrote: > > On Sun, Sep 29, 2024 at 07:16:08AM -0400, Mathieu Desnoyers wrote: > > > Refer to ptr_eq() in the rcu_dereference() documentation. > > > > > > ptr_eq() is a mechanism that preserves address dependencies when > > > comparing pointers, and should be favored when comparing a pointer > > > obtained from rcu_dereference() against another pointer. > > > > > > Signed-off-by: Mathieu Desnoyers > > > Cc: Greg Kroah-Hartman > > > Cc: Sebastian Andrzej Siewior > > > Cc: "Paul E. McKenney" > > > Cc: Will Deacon > > > Cc: Peter Zijlstra > > > Cc: Boqun Feng > > > Cc: Alan Stern > > > Cc: John Stultz > > > Cc: Neeraj Upadhyay > > > Cc: Linus Torvalds > > > Cc: Boqun Feng > > > Cc: Frederic Weisbecker > > > Cc: Joel Fernandes > > > Cc: Josh Triplett > > > Cc: Uladzislau Rezki > > > Cc: Steven Rostedt > > > Cc: Lai Jiangshan > > > Cc: Zqiang > > > Cc: Ingo Molnar > > > Cc: Waiman Long > > > Cc: Mark Rutland > > > Cc: Thomas Gleixner > > > Cc: Vlastimil Babka > > > Cc: maged.michael@gmail.com > > > Cc: Mateusz Guzik > > > Cc: Gary Guo > > > Cc: Jonas Oberhauser > > > Cc: rcu@vger.kernel.org > > > Cc: linux-mm@kvack.org > > > Cc: lkmm@lists.linux.dev > > > Cc: Nikita Popov > > > Cc: llvm@lists.linux.dev > > > --- > > > Changes since v0: > > > - Include feedback from Alan Stern. > > > --- > > > Documentation/RCU/rcu_dereference.rst | 32 ++++++++++++++++++++++----- > > > 1 file changed, 27 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/RCU/rcu_dereference.rst b/Documentation/RCU/rcu_dereference.rst > > > index 2524dcdadde2..9ef97b7ca74d 100644 > > > --- a/Documentation/RCU/rcu_dereference.rst > > > +++ b/Documentation/RCU/rcu_dereference.rst > > > @@ -104,11 +104,12 @@ readers working properly: > > > after such branches, but can speculate loads, which can again > > > result in misordering bugs. > > > -- Be very careful about comparing pointers obtained from > > > - rcu_dereference() against non-NULL values. As Linus Torvalds > > > - explained, if the two pointers are equal, the compiler could > > > - substitute the pointer you are comparing against for the pointer > > > - obtained from rcu_dereference(). For example:: > > > +- Use operations that preserve address dependencies (such as > > > + "ptr_eq()") to compare pointers obtained from rcu_dereference() > > > + against non-NULL pointers. As Linus Torvalds explained, if the > > > + two pointers are equal, the compiler could substitute the > > > + pointer you are comparing against for the pointer obtained from > > > + rcu_dereference(). For example:: > > > p = rcu_dereference(gp); > > > if (p == &default_struct) > > > @@ -125,6 +126,23 @@ readers working properly: > > > On ARM and Power hardware, the load from "default_struct.a" > > > can now be speculated, such that it might happen before the > > > rcu_dereference(). This could result in bugs due to misordering. > > > + Performing the comparison with "ptr_eq()" ensures the compiler > > > + does not perform such transformation. > > > + > > > + If the comparison is against another pointer, the compiler is > > > + allowed to use either pointer for the following accesses, which > > > + loses the address dependency and allows weakly-ordered > > > + architectures such as ARM and PowerPC to speculate the > > > + address-dependent load before rcu_dereference(). For example:: > > > + > > > + p1 = READ_ONCE(gp); > > > + p2 = rcu_dereference(gp); > > > + if (p1 == p2) > > > + do_default(p2->a); > > > + > > > + The compiler can use p1->a rather than p2->a, destroying the > > > + address dependency. Performing the comparison with "ptr_eq()" > > > + ensures the compiler preserves the address dependencies. > > > > Bitter experience leads me to suggest a "// BUGGY" comment on the "if" > > statement in the above example, and a corrected code snippet right here. :-/ > > Changing for the following: > > + p1 = READ_ONCE(gp); > + p2 = rcu_dereference(gp); > + if (p1 == p2) /* BUGGY!!! */ > + do_default(p2->a); > + > + The compiler can use p1->a rather than p2->a, destroying the > + address dependency. Performing the comparison with "ptr_eq()" > + ensures the compiler preserves the address dependencies. > + Corrected code:: > + > + p1 = READ_ONCE(gp); > + p2 = rcu_dereference(gp); > + if (ptr_eq(p1, p2)) > + do_default(p2->a); > > > > > Other than that, loks good! > > Let me know if I should add an acked-by from you on this > documentation patch as well. Much better! Acked-by: Paul E. McKenney > Thanks, > > Mathieu > > > > > Thanx, Paul > > > > > However, comparisons are OK in the following cases: > > > @@ -204,6 +222,10 @@ readers working properly: > > > comparison will provide exactly the information that the > > > compiler needs to deduce the value of the pointer. > > > + When in doubt, use operations that preserve address dependencies > > > + (such as "ptr_eq()") to compare pointers obtained from > > > + rcu_dereference() against non-NULL pointers. > > > + > > > - Disable any value-speculation optimizations that your compiler > > > might provide, especially if you are making use of feedback-based > > > optimizations that take data collected from prior runs. Such > > > -- > > > 2.39.2 > > > > > -- > Mathieu Desnoyers > EfficiOS Inc. > https://www.efficios.com >