Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Parav Pandit <parav@nvidia.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>
CC:     Will Deacon <will@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        "bagasdotme@gmail.com" <bagasdotme@gmail.com>,
        Alan Stern <stern@rowland.harvard.edu>,
        "parri.andrea@gmail.com" <parri.andrea@gmail.com>,
        Peter Zijlstra <peterz@infradead.org>,
        "boqun.feng@gmail.com" <boqun.feng@gmail.com>,
        Nicholas Piggin <npiggin@gmail.com>,
        "dhowells@redhat.com" <dhowells@redhat.com>,
        "j.alglave@ucl.ac.uk" <j.alglave@ucl.ac.uk>,
        "luc.maranget@inria.fr" <luc.maranget@inria.fr>,
        Akira Yokosawa <akiyks@gmail.com>,
        Dan Lustig <dlustig@nvidia.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Jonathan Corbet <corbet@lwn.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Linux-Arch <linux-arch@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: RE: [PATCH v4] locking/memory-barriers.txt: Improve documentation for
 writel() example
Thread-Topic: [PATCH v4] locking/memory-barriers.txt: Improve documentation
 for writel() example
Thread-Index: AQHY3JEEUGpScoYh8UGbpcw6oN0VqK4THE8AgADc+4CAAIFqgIAAJRcwgA4HJICAAAKPMA==
Date:   Thu, 27 Oct 2022 18:24:35 +0000
Message-ID: <PH0PR12MB5481D4417D82F130804ABA4FDC339@PH0PR12MB5481.namprd12.prod.outlook.com>
References: <20221010101331.29942-1-parav@nvidia.com>
 <d5faaf6f-7de5-49b0-92d6-9989ffbdbf2e@app.fastmail.com>
 <20221018100554.GA3112@willie-the-truck>
 <20221018174907.GT5600@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR12MB54810CA260159E805D448375DC289@PH0PR12MB5481.namprd12.prod.outlook.com>
 <20221027181503.GE5600@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20221027181503.GE5600@paulmck-ThinkPad-P17-Gen-1>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?3RCWQJ5pcwTbjlmEBeGrAYXgl9M6ThvXkFGOfj9CR9xWgORnplL/xC9ctaYw?=
 =?us-ascii?Q?VTwdzMaqfnASV5kc76mZaLIF2OOLkyAd/6HVW0jU9xXK5wGgUQRizAuA4MFR?=
 =?us-ascii?Q?Od6Oon3PoVZkfEGMi3VeLldnIdvZZaFqhGadRju7te8IKarhrBB0YzOGUEv7?=
 =?us-ascii?Q?iYRHhI9sW+NTCLLn4vHVlYcp7c+uDG/8HaEN4rrjM5SmyDCmp6JncNesg5FC?=
 =?us-ascii?Q?YX2sryhuIKiCb/DZtCzSXK8SLL1cweledOWHQYvLIM7NuUkk/xEnlzgBHa3e?=
 =?us-ascii?Q?WCJpOlmKIHLjGc2CjKOoUOcrvWM8gY4rAGEyMSJdqTfTVtRGXWZSocDWBZ8X?=
 =?us-ascii?Q?X61TOusnCVpo4hkaYBYjD7C03FLfSoxbNlKv196jo+1Sg+f7wrWGVjVqylMn?=
 =?us-ascii?Q?2evLBuW+W13vCglpI3Glh2FgAnkYhycGXZ6owPu9v8pgJC16XFpVxvZC3omi?=
 =?us-ascii?Q?/7sWgULBy6zepv+jHflIl4D9p5SPe/wk50F474QaQpP6TEx8DIadM0Nkmv6e?=
 =?us-ascii?Q?LDGySRG1uLUgFI4HVLVlmh+jiGI+JS/3MDcxFkdENxHICvAvjx5FZj03NWG7?=
 =?us-ascii?Q?xHIbx4yKdy3HHQHdLb0iU5FFTaSvQ+kB+dfoXj0ShfzdHqP1wS5l02I3JePM?=
 =?us-ascii?Q?dUR1pTsbXw9BiBeyTX2QXrbfkrWR7GcCrVjX5qmYsmoMII+MPXgsrp+95O+L?=
 =?us-ascii?Q?QZYS56dpsCvhquKSWMVMyx6YuwRAGRcxzEkxW4/bYLkq0jRAnjK9EcJOOJO7?=
 =?us-ascii?Q?N+3d83ANJYoK+rYQWDoIJ3RimWqbEQB5ARTHcnSXEgbZX/+UFqwPUn/hsegw?=
 =?us-ascii?Q?x/64R0MzSzP7LJGgTmJsqTngxAMStr1K03LBAnRWU92gsVq36oKQaEpkdqTH?=
 =?us-ascii?Q?N98/+wtwQXKfJJ9xPFG2PKuwdit3TrU2bv25+190h2aVNjzUyP2emsYVBi8g?=
 =?us-ascii?Q?wR7qi5btIyHWSO4Ke7H0cGuZ2TdLpWRK9IsM9t4Hn7VEskTPVhEpODAKeEOE?=
 =?us-ascii?Q?WQ5k/L1ViDWTAxgOTon/8ekTwIiOoN+WvKE8UvYNucWI3S59bjBCBJ83iU2+?=
 =?us-ascii?Q?bd88gE9BCFdEZlxMDna1ps2cU39MmNlwZKhkvf6x/xuQHL9AoguxIaxa9d92?=
 =?us-ascii?Q?ULBOZkMHUVPDyvqDOwMDo2P+KuM60gc7H8tYeUjCtmw93UBhbExlYQb6Brn3?=
 =?us-ascii?Q?+VKwVOMT9ocd82CszMf4T8jjcNDkprx/VySlCPnKpAYVzR4ReKpYqCEakYUR?=
 =?us-ascii?Q?HFp2Cp65zXTB3ygbvus/buFM2jCu7lgdKIUEqnjqPeRtMMhHmjUklrhqzikg?=
 =?us-ascii?Q?9m3uQmOECnw8ZpHsglmIEqs6b/ABlXOWSebNrdXAC32XoWgCvBoj8uGxGH6O?=
 =?us-ascii?Q?krCRrc9qaiDRJqm29DzweDZrmk0J6dvqgUj4ilW471JIVLdr0d65Jitl0keP?=
 =?us-ascii?Q?sJMAcfXBYpPfANVVk/7k3B707oqebBbswPntBxAlhYSYjz2yYDl0LiaYjc8l?=
 =?us-ascii?Q?a+YAI4VaqDBQ80lqAEkPnTjiHd7lFTh1dR9hBB6MNDiM1NdQSI9hvr26A1Io?=
 =?us-ascii?Q?VoKEEQzPRStNxSNxGih7GSkOBo+UmD2qMzY176SaarsH8gYm0+G/uhlXHiRt?=
 =?us-ascii?Q?0/Oi54S2OY4Olbv4k/eiCfU=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR12MB5481.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 68ba79f9-edf4-45c2-741f-08dab8488006
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Oct 2022 18:24:35.0784
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: FcMzYyyqVZzTl7gLkZA+aHx9SsY7hE/WmeOiX8wLlt7CKTLeWDKXVShyb6qcNA9T/XP1N49GemSYdp46R7B/iQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR12MB5894
X-Spam-Status: No, score=-1.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> From: Paul E. McKenney <paulmck@kernel.org>
> Sent: Thursday, October 27, 2022 2:15 PM
>=20
> On Tue, Oct 18, 2022 at 08:33:08PM +0000, Parav Pandit wrote:
> > Hi Paul, Will,
> >
> > > From: Paul E. McKenney <paulmck@kernel.org>
> > > Sent: Tuesday, October 18, 2022 1:49 PM
> > >
> > > On Tue, Oct 18, 2022 at 11:05:55AM +0100, Will Deacon wrote:
> > > > On Mon, Oct 17, 2022 at 10:55:00PM +0200, Arnd Bergmann wrote:
> > > > > On Mon, Oct 10, 2022, at 12:13 PM, Parav Pandit wrote:
> > > > > > The cited commit describes that when using writel(), explcit
> > > > > > wmb() is not needed. wmb() is an expensive barrier. writel()
> > > > > > uses the needed platform specific barrier instead of expensive
> wmb().
> > > > > >
> > > > > > Hence update the example to be more accurate that matches the
> > > > > > current implementation.
> > > > > >
> > > > > > commit 5846581e3563 ("locking/memory-barriers.txt: Fix broken
> > > > > > DMA
> > > vs.
> > > > > > MMIO ordering example")
> > > > > >
> > > > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > >
> > > > > I have no objections, though I still don't see a real need to
> > > > > change the wording here.
> > > >
> > > > FWIW, I also don't think this change is necessary. If anything,
> > > > I'd say we'd be better off _removing_ the text about writel from
> > > > this section and extending the reference to the "KERNEL I/O
> > > > BARRIER EFFECTS" section, as you could make similar comments about
> e.g.
> > > > readb() and subsequent barriers.
> > > >
> > > > For example, something like the diff below.
> > >
> > > I do like this change, but we might be dealing with two different
> > > groups of readers.  Will and Arnd implemented significant parts of
> > > the current MMIO/DMA ordering infrastructure.  It is thus quite
> > > possible that wording which suffices to remind them of how things
> > > work might or might not help someone new to Linux who is trying to
> > > figure out what is required to make their driver work.
> > >
> > > The traditional resolution of this sort of thing is to provide the
> > > documentation to a newbie and take any resulting confusion seriously.
> > >
> > > Parav, thoughts?
> >
> > I am ok with the change from Will that removes the writel() description=
.
> > However, it removes useful short description from the example of "why"
> writel() is used.
> > This is useful for newbie and experienced developers both.
> >
> > So how about below additional change on top of Will's change?
> > This also aligns to rest of the short C comments in this example pseudo
> code.
> >
> > If ok, I will take Will's and mine below change to v5.
> >
> > index 4d24d39f5e42..5939c5e09570 100644
> > --- a/Documentation/memory-barriers.txt
> > +++ b/Documentation/memory-barriers.txt
> > @@ -1919,7 +1919,9 @@ There are some more advanced barrier functions:
> >                 /* assign ownership */
> >                 desc->status =3D DEVICE_OWN;
> >
> > -               /* notify device of new descriptors */
> > +               /* Make descriptor status visible to the device followe=
d by
> > +                * notify device of new descriptors
> > +                */
> >                 writel(DESC_NOTIFY, doorbell);
>=20
> Hearing no objections, please proceed.
>=20
> 							Thanx, Paul
Thanks Paul, I will respin v5 shortly.