From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 346EF27E056; Mon, 6 Apr 2026 16:35:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775493309; cv=none; b=f49qaSEWqtBCEOq1GUngFOAzORHwN10k38Tb/ihzMnd2Hqh8DCQBteD4TD52FBZxvHrFQ/lYYXrhYLkOawndrDWSLJ5hMQjX+UQHAh5EVLGMnkeL2zF7k1zKwRLFv5zCCnHi6AE5ML6joJaEGcnkj1Bzrxn1YvgyTa9mOWU5szQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775493309; c=relaxed/simple; bh=gQNVM0vS0NwKn8hC5VdGPw22C0ngOezWXdkapljDSYs=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=JDgnP1aQfgTQ+FKhlOHT1R7OeeeRGRAOI2OpgOvlTlScKt+qXHKEcBNE4qivIcqPY0vpkzROlDWjLOsTDm2NsrMXKcYh5fCxosdcRSss0C49NRGc8Hp6qxoStVh6O6Tlt9N0GYO563tjDi9XUvEbZ5ia9pOhmBXYqd/43EM+rTk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B3Wd8hWQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B3Wd8hWQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 01DFEC4CEF7; Mon, 6 Apr 2026 16:35:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775493309; bh=gQNVM0vS0NwKn8hC5VdGPw22C0ngOezWXdkapljDSYs=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=B3Wd8hWQ/VWakkggFk3qhyJFCP03r2YWIu3UTabixMOm91yQF+Kmh+sy9Wx8OX/oX R10zR6FrTXBMQUS/E6h3Ar6K1KhvRyAHKY3jYLTMrD5M1tYYqGTu70n/Z0nksxVDMW rhmwqAMz3oAx+GPXI8UK7cMKqzrXAQ6Zw9DMnP8l4Ji9cCs3skNbzX2N8hRL50rChU 4QxBfuATJLcNUrXYCUAC0tUx0u3DF8Ca2sbG9+sQxCSB6oysshJp7yAaDTj1jquixn Iz7LBWhVAG/PffTA62Kt9rx0vl8dpERmC5bk8qkFkt8qvxTCCuddlrJWUQZWwot4ZM SMvl4c9CI26xA== Received: from cu01147a.smtpx.saremail.com ([195.16.150.122] helo=lobster-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1w9muo-00000009GIj-2BDo; Mon, 06 Apr 2026 16:35:06 +0000 Date: Mon, 06 Apr 2026 17:34:59 +0100 Message-ID: <871pgscaj0.wl-maz@kernel.org> From: Marc Zyngier To: Doug Anderson Cc: Greg Kroah-Hartman , "Rafael J . Wysocki" , Danilo Krummrich , Alan Stern , Saravana Kannan , Christoph Hellwig , Eric Dumazet , Johan Hovold , Leon Romanovsky , Alexander Lobakin , Alexey Kardashevskiy , Robin Murphy , stable@vger.kernel.org, driver-core@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/9] driver core: Don't let a device probe until it's ready In-Reply-To: References: <20260404000644.522677-1-dianders@chromium.org> <20260403170432.v4.1.Id750b0fbcc94f23ed04b7aecabcead688d0d8c17@changeid> <873418d2fz.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 195.16.150.122 X-SA-Exim-Rcpt-To: dianders@chromium.org, gregkh@linuxfoundation.org, rafael@kernel.org, dakr@kernel.org, stern@rowland.harvard.edu, saravanak@kernel.org, hch@lst.de, edumazet@google.com, johan@kernel.org, leon@kernel.org, aleksander.lobakin@intel.com, aik@ozlabs.ru, robin.murphy@arm.com, stable@vger.kernel.org, driver-core@lists.linux.dev, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Mon, 06 Apr 2026 15:41:08 +0100, Doug Anderson wrote: >=20 > Hi, >=20 > On Sun, Apr 5, 2026 at 11:32=E2=80=AFPM Marc Zyngier wro= te: > > > > > + * blocked those attempts. Now that all of the above initializa= tion has > > > + * happened, unblock probe. If probe happens through another th= read > > > + * after this point but before bus_probe_device() runs then it'= s fine. > > > + * bus_probe_device() -> device_initial_probe() -> __device_att= ach() > > > + * will notice (under device_lock) that the device is already b= ound. > > > + */ > > > + dev_set_ready_to_probe(dev); > > > > I think this lacks some ordering properties that we should be allowed > > to rely on. In this case, the 'ready_to_probe' flag being set should > > that all of the data structures are observable by another CPU. > > > > Unfortunately, this doesn't seem to be the case, see below. >=20 > I agree. I think Danilo was proposing fixing this by just doing: >=20 > device_lock(dev); > dev_set_ready_to_probe(dev); > device_unlock(dev); >=20 > While that's a bit of an overkill, it also works I think. Do folks > have a preference for what they'd like to see in v5? It would work, but I find the construct rather obscure, and it implies that there is a similar lock taken on the read path. Looking at the code for a couple of minutes doesn't lead to an immediate clue that such lock is indeed taken on all read paths. > >=20 > > > @@ -675,8 +691,34 @@ struct device { > > > #ifdef CONFIG_IOMMU_DMA > > > bool dma_iommu:1; > > > #endif > > > + > > > + DECLARE_BITMAP(flags, DEV_FLAG_COUNT); > > > }; > > > > > > +#define __create_dev_flag_accessors(accessor_name, flag_name) \ > > > +static inline bool dev_##accessor_name(const struct device *dev) \ > > > +{ \ > > > + return test_bit(flag_name, dev->flags); \ > > > +} \ > > > +static inline void dev_set_##accessor_name(struct device *dev) \ > > > +{ \ > > > + set_bit(flag_name, dev->flags); \ > > > > Atomic operations that are not RMW or that do not return a value are > > unordered (see Documentation/atomic_bitops.txt). This implies that > > observing the flag being set from another CPU does not guarantee that > > the previous stores in program order are observed. > > > > For that guarantee to hold, you'd need to have an > > smp_mb__before_atomic() just before set_bit(), giving it release > > semantics. This is equally valid for the test, clear and assign > > variants. > > > > I doubt this issue is visible on a busy system (which would be the > > case at boot time), but I thought I'd mention it anyway. >=20 > Are you suggesting I add smp memory barriers directly in all the > accessors? ...or just that clients of these functions should use > memory barriers as appropriate? >=20 > In other words, would I do: >=20 > smp_mb__before_atomic(); > dev_set_ready_to_probe(dev); >=20 > ...or add the barrier into all of the accessor? >=20 > My thought was to not add the barrier into the accessors since at > least one of the accessors talks about being run from a hot path > (dma_reset_need_sync()). ...but I just want to make sure. I don't think this needs to be inflicted on all flags, specially the ones you are simply moving into the bitmap and that didn't have any particular ordering requirements. 'ready_to_probe' is a bit different, as it is new and tries to offer ordering semantics. So an open-coded barrier on both sides would do the trick, unless you go for the lock and can convince yourself that it is indeed always acquired on all the read paths. Thanks, M. --=20 Jazz isn't dead. It just smells funny.