[RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Douglas Anderson]

The moment we link a "struct device" into the list of devices for the
bus, it's possible probe can happen. This is because another thread
can load the driver at any time and that can cause the device to
probe. This has been seen in practice with a stack crawl that looks
like this [1]:

  really_probe()
  __driver_probe_device()
  driver_probe_device()
  __driver_attach()
  bus_for_each_dev()
  driver_attach()
  bus_add_driver()
  driver_register()
  __platform_driver_register()
  init_module() [some module]
  do_one_initcall()
  do_init_module()
  load_module()
  __arm64_sys_finit_module()
  invoke_syscall()

Practically speaking, in the case this happened
device_links_driver_bound() got called for the device before
"dev->fwnode->dev" was assigned. This prevented
__fw_devlink_pickup_dangling_consumers() from being called which meant
that other devices waiting on our driver's sub-nodes were stuck
deferring forever.

Fix the problem by adjusting where we link the device. Notably:
* Make sure we assign the dev->fwnode->dev before we link the device,
  since that needs to happen before a device probes.
* Make sure we link the device _before_ sending the UEVENT, as
  described in commit 2023c610dc54 ("Driver core: add new device to
  bus's list before probing").

[1] Captured on a machine running a downstream 6.6 kernel

Cc: stable@vger.kernel.org
Fixes: 2023c610dc54 ("Driver core: add new device to bus's list before probing")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
This is a super tricky area of code, and I don't have a lot of
confidence that I got this exactly right. If you think I should do
something different, please yell!

Notably:
* I don't know 100% for sure what happens if probe runs at the same
  time as (or before) the call to `bus_notify(dev,
  BUS_NOTIFY_ADD_DEVICE)`. Presumably we're at least no worse off than
  we were before this patch.
* I haven't dug into whether there could be other types of race
  conditions with a driver trying to probe the device at the same time
  we call bus_probe_device().

I can try to dig into those things, but I also figured that people on
the mailing lists would know this code a whole lot better than I do.

Feel free to consider this RFC patch as a bug report and post a
totally different solution. I won't be offended! :-)

I have only done very minimal testing with this patch so far (the
system doesn't seem to blow up with this patch). I'm kicking off tests
for the weekend, but I figured I'd post this up in parallel.

 drivers/base/base.h |  1 +
 drivers/base/bus.c  | 16 ++++++++++++++--
 drivers/base/core.c | 24 ++++++++++++++++++------
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 1af95ac68b77..d4933ba7b651 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -166,6 +166,7 @@ static inline void auxiliary_bus_init(void) { }
 struct kobject *virtual_device_parent(void);
 
 int bus_add_device(struct device *dev);
+void bus_link_device(struct device *dev);
 void bus_probe_device(struct device *dev);
 void bus_remove_device(struct device *dev);
 void bus_notify(struct device *dev, enum bus_notifier_event value);
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index bb61d8adbab1..573c22ebb5b7 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -510,7 +510,6 @@ EXPORT_SYMBOL_GPL(bus_for_each_drv);
  *
  * - Add device's bus attributes.
  * - Create links to device's bus.
- * - Add the device to its bus's list of devices.
  */
 int bus_add_device(struct device *dev)
 {
@@ -545,7 +544,6 @@ int bus_add_device(struct device *dev)
 	if (error)
 		goto out_subsys;
 
-	klist_add_tail(&dev->p->knode_bus, &sp->klist_devices);
 	return 0;
 
 out_subsys:
@@ -557,6 +555,20 @@ int bus_add_device(struct device *dev)
 	return error;
 }
 
+/**
+ * bus_link_device - Make a device findable in the list of devices
+ * @dev: device being linked
+ *
+ * - Add the device to its bus's list of devices.
+ */
+void bus_link_device(struct device *dev)
+{
+	struct subsys_private *sp = bus_to_subsys(dev->bus);
+
+	if (sp)
+		klist_add_tail(&dev->p->knode_bus, &sp->klist_devices);
+}
+
 /**
  * bus_probe_device - probe drivers for a new device
  * @dev: device to probe
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 791f9e444df8..6ff4725d677a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3663,12 +3663,6 @@ int device_add(struct device *dev)
 		devtmpfs_create_node(dev);
 	}
 
-	/* Notify clients of device addition.  This call must come
-	 * after dpm_sysfs_add() and before kobject_uevent().
-	 */
-	bus_notify(dev, BUS_NOTIFY_ADD_DEVICE);
-	kobject_uevent(&dev->kobj, KOBJ_ADD);
-
 	/*
 	 * Check if any of the other devices (consumers) have been waiting for
 	 * this device (supplier) to be added so that they can create a device
@@ -3680,12 +3674,30 @@ int device_add(struct device *dev)
 	 * But this also needs to happen before bus_probe_device() to make sure
 	 * waiting consumers can link to it before the driver is bound to the
 	 * device and the driver sync_state callback is called for this device.
+	 *
+	 * Because a bus may be probed the moment bus_link_device() is called,
+	 * this must happen before bus_link_device().
 	 */
 	if (dev->fwnode && !dev->fwnode->dev) {
 		dev->fwnode->dev = dev;
 		fw_devlink_link_device(dev);
 	}
 
+	/*
+	 * The moment we link the bus in, it's possible for another thread
+	 * (one registering a new driver) to notice it and start probing.
+	 * At the same time, we need to link the bus before the uevent is
+	 * sent announcing the device or user programs might try to access
+	 * the device before it has been added to the bus.
+	 */
+	bus_link_device(dev);
+
+	/* Notify clients of device addition.  This call must come
+	 * after dpm_sysfs_add() and before kobject_uevent().
+	 */
+	bus_notify(dev, BUS_NOTIFY_ADD_DEVICE);
+	kobject_uevent(&dev->kobj, KOBJ_ADD);
+
 	bus_probe_device(dev);
 
 	/*
-- 
2.53.0.959.g497ff81fa9-goog

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Greg Kroah-Hartman]

On Fri, Mar 20, 2026 at 08:06:58PM -0700, Douglas Anderson wrote:
> The moment we link a "struct device" into the list of devices for the
> bus, it's possible probe can happen. This is because another thread
> can load the driver at any time and that can cause the device to
> probe. This has been seen in practice with a stack crawl that looks
> like this [1]:
> 
>   really_probe()
>   __driver_probe_device()
>   driver_probe_device()
>   __driver_attach()
>   bus_for_each_dev()
>   driver_attach()
>   bus_add_driver()
>   driver_register()
>   __platform_driver_register()
>   init_module() [some module]
>   do_one_initcall()
>   do_init_module()
>   load_module()
>   __arm64_sys_finit_module()
>   invoke_syscall()

Are you sure this isn't just a platform bus issue?  A bus should NOT be
allowing a driver to be added at the same time a device is being added
for that bus, ideally there should be a bus-specific lock somewhere for
this.

When a device is added to the bus, yes, a probe can happen, and is
expected to happen, for that device, so this feels odd.

that being said, your patch does seem sane, and I don't see anything
obviously wrong with it.  But it feels odd that this is just now showing
up for something that has been this way for a few decades...

thanks,

greg k-h

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Doug Anderson]

Hi,

On Fri, Mar 20, 2026 at 10:41 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Mar 20, 2026 at 08:06:58PM -0700, Douglas Anderson wrote:
> > The moment we link a "struct device" into the list of devices for the
> > bus, it's possible probe can happen. This is because another thread
> > can load the driver at any time and that can cause the device to
> > probe. This has been seen in practice with a stack crawl that looks
> > like this [1]:
> >
> >   really_probe()
> >   __driver_probe_device()
> >   driver_probe_device()
> >   __driver_attach()
> >   bus_for_each_dev()
> >   driver_attach()
> >   bus_add_driver()
> >   driver_register()
> >   __platform_driver_register()
> >   init_module() [some module]
> >   do_one_initcall()
> >   do_init_module()
> >   load_module()
> >   __arm64_sys_finit_module()
> >   invoke_syscall()
>
> Are you sure this isn't just a platform bus issue?  A bus should NOT be
> allowing a driver to be added at the same time a device is being added
> for that bus, ideally there should be a bus-specific lock somewhere for
> this.

Sure, if the right fix for this is somewhere in the platform bus code
then I'd be happy with a patch there to fix it. ...but from my quick
glance (admittedly, it's Friday night and I'm tired), it seems like
the problem is just with driver_register() being called at the same
time as device_add().

Certainly adding some sort of locking could be a solution (happy for
someone to tell me where to place them), but we'd have to make sure we
aren't regressing performance for the normal case...


> When a device is added to the bus, yes, a probe can happen, and is
> expected to happen, for that device, so this feels odd.
>
> that being said, your patch does seem sane, and I don't see anything
> obviously wrong with it.  But it feels odd that this is just now showing
> up for something that has been this way for a few decades...

I suspect it's a latent bug that was triggered by a new Android
feature. It's showing up on phones that have
"ro.boot.load_modules_parallel" set. I think you can get to the
relevant source code at:

https://cs.android.com/android/platform/superproject/main/+/main:system/core/libmodprobe/libmodprobe.cpp?q=LoadModulesParallel

I suspect the bug is never triggered with more normal module loading
schemes. Indeed, one phone that has nearly the same set of drivers but
has parallel module loading turned off has no reports of this
problem...

I'd also note that the only actual symptom we're seeing is with
fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
enough). fw_devlink is a "new" (ish) feature, is officially optional,
and isn't used on all hardware.

...so I think there are enough reasons to explain why we're seeing
this now even though it seems like it's been wrong for a few decades.

-Doug

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Greg Kroah-Hartman]

On Sat, Mar 21, 2026 at 12:35:32AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Fri, Mar 20, 2026 at 10:41 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Mar 20, 2026 at 08:06:58PM -0700, Douglas Anderson wrote:
> > > The moment we link a "struct device" into the list of devices for the
> > > bus, it's possible probe can happen. This is because another thread
> > > can load the driver at any time and that can cause the device to
> > > probe. This has been seen in practice with a stack crawl that looks
> > > like this [1]:
> > >
> > >   really_probe()
> > >   __driver_probe_device()
> > >   driver_probe_device()
> > >   __driver_attach()
> > >   bus_for_each_dev()
> > >   driver_attach()
> > >   bus_add_driver()
> > >   driver_register()
> > >   __platform_driver_register()
> > >   init_module() [some module]
> > >   do_one_initcall()
> > >   do_init_module()
> > >   load_module()
> > >   __arm64_sys_finit_module()
> > >   invoke_syscall()
> >
> > Are you sure this isn't just a platform bus issue?  A bus should NOT be
> > allowing a driver to be added at the same time a device is being added
> > for that bus, ideally there should be a bus-specific lock somewhere for
> > this.
> 
> Sure, if the right fix for this is somewhere in the platform bus code
> then I'd be happy with a patch there to fix it. ...but from my quick
> glance (admittedly, it's Friday night and I'm tired), it seems like
> the problem is just with driver_register() being called at the same
> time as device_add().
> 
> Certainly adding some sort of locking could be a solution (happy for
> someone to tell me where to place them), but we'd have to make sure we
> aren't regressing performance for the normal case...
> 
> 
> > When a device is added to the bus, yes, a probe can happen, and is
> > expected to happen, for that device, so this feels odd.
> >
> > that being said, your patch does seem sane, and I don't see anything
> > obviously wrong with it.  But it feels odd that this is just now showing
> > up for something that has been this way for a few decades...
> 
> I suspect it's a latent bug that was triggered by a new Android
> feature. It's showing up on phones that have
> "ro.boot.load_modules_parallel" set. I think you can get to the
> relevant source code at:
> 
> https://cs.android.com/android/platform/superproject/main/+/main:system/core/libmodprobe/libmodprobe.cpp?q=LoadModulesParallel
> 
> I suspect the bug is never triggered with more normal module loading
> schemes. Indeed, one phone that has nearly the same set of drivers but
> has parallel module loading turned off has no reports of this
> problem...

Ah, I think we always assumed that modules can NOT be loaded in
parallel, isn't there an internal module lock that prevents this from
happening?

So yes, that might be the root problem here.

> I'd also note that the only actual symptom we're seeing is with
> fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> enough). fw_devlink is a "new" (ish) feature, is officially optional,
> and isn't used on all hardware.

That's true too, can we set that earlier?

thanks,

greg k-h

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Doug Anderson]

Hi,

On Sat, Mar 21, 2026 at 12:42 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Mar 21, 2026 at 12:35:32AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Mar 20, 2026 at 10:41 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Mar 20, 2026 at 08:06:58PM -0700, Douglas Anderson wrote:
> > > > The moment we link a "struct device" into the list of devices for the
> > > > bus, it's possible probe can happen. This is because another thread
> > > > can load the driver at any time and that can cause the device to
> > > > probe. This has been seen in practice with a stack crawl that looks
> > > > like this [1]:
> > > >
> > > >   really_probe()
> > > >   __driver_probe_device()
> > > >   driver_probe_device()
> > > >   __driver_attach()
> > > >   bus_for_each_dev()
> > > >   driver_attach()
> > > >   bus_add_driver()
> > > >   driver_register()
> > > >   __platform_driver_register()
> > > >   init_module() [some module]
> > > >   do_one_initcall()
> > > >   do_init_module()
> > > >   load_module()
> > > >   __arm64_sys_finit_module()
> > > >   invoke_syscall()
> > >
> > > Are you sure this isn't just a platform bus issue?  A bus should NOT be
> > > allowing a driver to be added at the same time a device is being added
> > > for that bus, ideally there should be a bus-specific lock somewhere for
> > > this.
> >
> > Sure, if the right fix for this is somewhere in the platform bus code
> > then I'd be happy with a patch there to fix it. ...but from my quick
> > glance (admittedly, it's Friday night and I'm tired), it seems like
> > the problem is just with driver_register() being called at the same
> > time as device_add().
> >
> > Certainly adding some sort of locking could be a solution (happy for
> > someone to tell me where to place them), but we'd have to make sure we
> > aren't regressing performance for the normal case...
> >
> >
> > > When a device is added to the bus, yes, a probe can happen, and is
> > > expected to happen, for that device, so this feels odd.
> > >
> > > that being said, your patch does seem sane, and I don't see anything
> > > obviously wrong with it.  But it feels odd that this is just now showing
> > > up for something that has been this way for a few decades...
> >
> > I suspect it's a latent bug that was triggered by a new Android
> > feature. It's showing up on phones that have
> > "ro.boot.load_modules_parallel" set. I think you can get to the
> > relevant source code at:
> >
> > https://cs.android.com/android/platform/superproject/main/+/main:system/core/libmodprobe/libmodprobe.cpp?q=LoadModulesParallel
> >
> > I suspect the bug is never triggered with more normal module loading
> > schemes. Indeed, one phone that has nearly the same set of drivers but
> > has parallel module loading turned off has no reports of this
> > problem...
>
> Ah, I think we always assumed that modules can NOT be loaded in
> parallel, isn't there an internal module lock that prevents this from
> happening?
>
> So yes, that might be the root problem here.

It's late Friday night for me (technically Saturday morning), so I'm
not going to dig now. ...but I'm fairly certain that Android isn't
using any downstream kernel patches to accomplish its "parallel module
loading". It's just userspace jamming modules in as fast as it can.
Userspace loading modules quickly shouldn't cause the kernel to behave
badly.

If the right solution is to add more locking to the kernel to slow
userspace down, that is also something I could try. It will likely end
up impacting boot speed, but of course correctness comes first. Let me
know if this is a direction I should dig (or someone is free to post a
patch and I can test it).


> > I'd also note that the only actual symptom we're seeing is with
> > fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> > enough). fw_devlink is a "new" (ish) feature, is officially optional,
> > and isn't used on all hardware.
>
> That's true too, can we set that earlier?

Yes, I can post a patch that _just_ moves the set of dev->fwnode->dev
earlier, and that will probably fix my symptoms (I'll need to test).
This patch already moves it a bit earlier, but if we don't break the
linking out as a separate step it would need to move even higher up in
the function.

Originally, I was going to just propose that, but then I realized that
some of the other code in device_add() probably also ought to run
before we let the driver probe, and hence I ended up with this patch.

-Doug

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Alan Stern]

On Sat, Mar 21, 2026 at 01:05:48AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Sat, Mar 21, 2026 at 12:42 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Mar 21, 2026 at 12:35:32AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Fri, Mar 20, 2026 at 10:41 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Mar 20, 2026 at 08:06:58PM -0700, Douglas Anderson wrote:
> > > > > The moment we link a "struct device" into the list of devices for the
> > > > > bus, it's possible probe can happen. This is because another thread
> > > > > can load the driver at any time and that can cause the device to
> > > > > probe. This has been seen in practice with a stack crawl that looks
> > > > > like this [1]:
> > > > >
> > > > >   really_probe()
> > > > >   __driver_probe_device()
> > > > >   driver_probe_device()
> > > > >   __driver_attach()
> > > > >   bus_for_each_dev()
> > > > >   driver_attach()
> > > > >   bus_add_driver()
> > > > >   driver_register()
> > > > >   __platform_driver_register()
> > > > >   init_module() [some module]
> > > > >   do_one_initcall()
> > > > >   do_init_module()
> > > > >   load_module()
> > > > >   __arm64_sys_finit_module()
> > > > >   invoke_syscall()
> > > >
> > > > Are you sure this isn't just a platform bus issue?  A bus should NOT be
> > > > allowing a driver to be added at the same time a device is being added
> > > > for that bus, ideally there should be a bus-specific lock somewhere for
> > > > this.
> > >
> > > Sure, if the right fix for this is somewhere in the platform bus code
> > > then I'd be happy with a patch there to fix it. ...but from my quick
> > > glance (admittedly, it's Friday night and I'm tired), it seems like
> > > the problem is just with driver_register() being called at the same
> > > time as device_add().
> > >
> > > Certainly adding some sort of locking could be a solution (happy for
> > > someone to tell me where to place them), but we'd have to make sure we
> > > aren't regressing performance for the normal case...
> > >
> > >
> > > > When a device is added to the bus, yes, a probe can happen, and is
> > > > expected to happen, for that device, so this feels odd.
> > > >
> > > > that being said, your patch does seem sane, and I don't see anything
> > > > obviously wrong with it.  But it feels odd that this is just now showing
> > > > up for something that has been this way for a few decades...
> > >
> > > I suspect it's a latent bug that was triggered by a new Android
> > > feature. It's showing up on phones that have
> > > "ro.boot.load_modules_parallel" set. I think you can get to the
> > > relevant source code at:
> > >
> > > https://cs.android.com/android/platform/superproject/main/+/main:system/core/libmodprobe/libmodprobe.cpp?q=LoadModulesParallel
> > >
> > > I suspect the bug is never triggered with more normal module loading
> > > schemes. Indeed, one phone that has nearly the same set of drivers but
> > > has parallel module loading turned off has no reports of this
> > > problem...
> >
> > Ah, I think we always assumed that modules can NOT be loaded in
> > parallel, isn't there an internal module lock that prevents this from
> > happening?
> >
> > So yes, that might be the root problem here.
> 
> It's late Friday night for me (technically Saturday morning), so I'm
> not going to dig now. ...but I'm fairly certain that Android isn't
> using any downstream kernel patches to accomplish its "parallel module
> loading". It's just userspace jamming modules in as fast as it can.
> Userspace loading modules quickly shouldn't cause the kernel to behave
> badly.
> 
> If the right solution is to add more locking to the kernel to slow
> userspace down, that is also something I could try. It will likely end
> up impacting boot speed, but of course correctness comes first. Let me
> know if this is a direction I should dig (or someone is free to post a
> patch and I can test it).

As far as I know, there's no particular reason why modules shouldn't be 
loaded in parallel, or at least, in very quick succession.  Locking 
shouldn't matter either -- that is, the existing locks ought to be 
adequate.

> > > I'd also note that the only actual symptom we're seeing is with
> > > fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> > > enough). fw_devlink is a "new" (ish) feature, is officially optional,
> > > and isn't used on all hardware.
> >
> > That's true too, can we set that earlier?
> 
> Yes, I can post a patch that _just_ moves the set of dev->fwnode->dev
> earlier, and that will probably fix my symptoms (I'll need to test).
> This patch already moves it a bit earlier, but if we don't break the
> linking out as a separate step it would need to move even higher up in
> the function.
> 
> Originally, I was going to just propose that, but then I realized that
> some of the other code in device_add() probably also ought to run
> before we let the driver probe, and hence I ended up with this patch.

This sounds like a more generic problem.  A bunch of things happen after 
bus_add_device() that should be completed before probing can start; the 
firmware node stuff is just one of them.

Splitting bus_add_device() in two sounds reasonable, although I would 
rename the old routine to bus_link_device, since all it does it add some 
groups and symlinks.  The new routine can be called bus_add_device().

The real question is whether any of the other stuff that happens before 
bus_probe_device() needs to come after the device is added to the bus's 
list.  The bus_notify() and kobject_uevent() calls are good examples; I 
don't know what their requirements are.  Should they be moved down, 
between the new bus_add_device() and bus_probe_device()?

Alan Stern

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Doug Anderson]

Hi,

On Sat, Mar 21, 2026 at 8:54 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> > > > I'd also note that the only actual symptom we're seeing is with
> > > > fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> > > > enough). fw_devlink is a "new" (ish) feature, is officially optional,
> > > > and isn't used on all hardware.
> > >
> > > That's true too, can we set that earlier?
> >
> > Yes, I can post a patch that _just_ moves the set of dev->fwnode->dev
> > earlier, and that will probably fix my symptoms (I'll need to test).
> > This patch already moves it a bit earlier, but if we don't break the
> > linking out as a separate step it would need to move even higher up in
> > the function.
> >
> > Originally, I was going to just propose that, but then I realized that
> > some of the other code in device_add() probably also ought to run
> > before we let the driver probe, and hence I ended up with this patch.
>
> This sounds like a more generic problem.  A bunch of things happen after
> bus_add_device() that should be completed before probing can start; the
> firmware node stuff is just one of them.
>
> Splitting bus_add_device() in two sounds reasonable, although I would
> rename the old routine to bus_link_device, since all it does it add some
> groups and symlinks.  The new routine can be called bus_add_device().

LOL, so basically you want them named exactly opposite I did? That's
OK with me, though maybe:

bus_prep_device()
bus_add_device()


> The real question is whether any of the other stuff that happens before
> bus_probe_device() needs to come after the device is added to the bus's
> list.  The bus_notify() and kobject_uevent() calls are good examples; I
> don't know what their requirements are.  Should they be moved down,
> between the new bus_add_device() and bus_probe_device()?

Yes, this is my question too. The question is, what's worse:

1. Potentially the device getting probed before the calls to
"bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
"kobject_uevent(&dev->kobj, KOBJ_ADD)"

2. Calling "bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
"kobject_uevent(&dev->kobj, KOBJ_ADD)" without first adding to the
subsystem's list of devices.

As it is, my patch says #2 is worse and thus allows for #1. ...but I
don't actually know the answer. The main reason I chose to allow for
#1 is that it makes the behavior less different than it was before my
patch, but that doesn't mean it's correct.

Reading  through a handful of "BUS_NOTIFY_ADD_DEVICE" calls, my guess
is that they expect to be called before probe... That would imply that
my patch made the wrong choice...


I think another option here might be to just add a new bitfield flag
to "struct device", like "ready_to_probe". Right before the call to
bus_probe_device(dev), we could do something like:

device_lock(dev);
dev->ready_to_probe = true;
device_unlcok(dev);

Then in __driver_attach() I can have:

device_lock(dev);
ready_to_probe = dev->ready_to_probe;
device_unlock(dev);
if (!ready_to_probe)
  return 0;

If I do that, I don't think I'll even need to re-order anything, and I
think it's all safe. Basically, it just the device hidden from the
__driver_attach() logic until probe is ready.

What do you think?

-Doug

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Alan Stern]

On Mon, Mar 23, 2026 at 10:07:01AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Sat, Mar 21, 2026 at 8:54 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > > > > I'd also note that the only actual symptom we're seeing is with
> > > > > fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> > > > > enough). fw_devlink is a "new" (ish) feature, is officially optional,
> > > > > and isn't used on all hardware.
> > > >
> > > > That's true too, can we set that earlier?
> > >
> > > Yes, I can post a patch that _just_ moves the set of dev->fwnode->dev
> > > earlier, and that will probably fix my symptoms (I'll need to test).
> > > This patch already moves it a bit earlier, but if we don't break the
> > > linking out as a separate step it would need to move even higher up in
> > > the function.
> > >
> > > Originally, I was going to just propose that, but then I realized that
> > > some of the other code in device_add() probably also ought to run
> > > before we let the driver probe, and hence I ended up with this patch.
> >
> > This sounds like a more generic problem.  A bunch of things happen after
> > bus_add_device() that should be completed before probing can start; the
> > firmware node stuff is just one of them.

You know, I wrote this but I'm not so sure that it's accurate.  We've 
gone many years with no big changes to this code; most likely it doesn't 
need alterations now.

> > Splitting bus_add_device() in two sounds reasonable, although I would
> > rename the old routine to bus_link_device, since all it does it add some
> > groups and symlinks.  The new routine can be called bus_add_device().
> 
> LOL, so basically you want them named exactly opposite I did? That's
> OK with me, though maybe:
> 
> bus_prep_device()
> bus_add_device()

Nothing wrong with those names.  But instead of making these fairly 
intrusive changes it might be better just to move the firmware stuff to 
a different place in the code (you mentioned this possibility in your 
first email).  It would be a smaller change, that's for sure.

> > The real question is whether any of the other stuff that happens before
> > bus_probe_device() needs to come after the device is added to the bus's
> > list.  The bus_notify() and kobject_uevent() calls are good examples; I
> > don't know what their requirements are.  Should they be moved down,
> > between the new bus_add_device() and bus_probe_device()?
> 
> Yes, this is my question too. The question is, what's worse:
> 
> 1. Potentially the device getting probed before the calls to
> "bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
> "kobject_uevent(&dev->kobj, KOBJ_ADD)"
> 
> 2. Calling "bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
> "kobject_uevent(&dev->kobj, KOBJ_ADD)" without first adding to the
> subsystem's list of devices.
> 
> As it is, my patch says #2 is worse and thus allows for #1. ...but I
> don't actually know the answer. The main reason I chose to allow for
> #1 is that it makes the behavior less different than it was before my
> patch, but that doesn't mean it's correct.
> 
> Reading  through a handful of "BUS_NOTIFY_ADD_DEVICE" calls, my guess
> is that they expect to be called before probe... That would imply that
> my patch made the wrong choice...

I don't know about other subsystems, only USB.  As far as I remember, 
USB doesn't really care about the order of probing vs. notifications.  
In general, since probing can be asynchronous with registration, the 
kernel always has to work with probing after bus_notify().  It's 
best to preserve that order unless there's a very good reason not to.

uevents may be different, since they go to userspace.  It could be 
weird for a user program to be told about a new device and then not be 
able to find it on the bus.

> I think another option here might be to just add a new bitfield flag
> to "struct device", like "ready_to_probe". Right before the call to
> bus_probe_device(dev), we could do something like:
> 
> device_lock(dev);
> dev->ready_to_probe = true;
> device_unlcok(dev);
> 
> Then in __driver_attach() I can have:
> 
> device_lock(dev);
> ready_to_probe = dev->ready_to_probe;
> device_unlock(dev);
> if (!ready_to_probe)
>   return 0;
> 
> If I do that, I don't think I'll even need to re-order anything, and I
> think it's all safe. Basically, it just the device hidden from the
> __driver_attach() logic until probe is ready.
> 
> What do you think?

There should not be any difference between probing caused by the device 
being added to the bus, vs. caused by a new driver being registered, vs. 
caused by anything else (such as sysfs).  None of these should be 
allowed until all of them can be handled properly.

And linking the device into the bus's list of devices should be the 
event that makes probing possible.

Alan Stern

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Doug Anderson]

Hi,

On Mon, Mar 23, 2026 at 10:47 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Mon, Mar 23, 2026 at 10:07:01AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Sat, Mar 21, 2026 at 8:54 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > > > > I'd also note that the only actual symptom we're seeing is with
> > > > > > fw_devlink misbehaving (because dev->fwnode->dev wasn't set early
> > > > > > enough). fw_devlink is a "new" (ish) feature, is officially optional,
> > > > > > and isn't used on all hardware.
> > > > >
> > > > > That's true too, can we set that earlier?
> > > >
> > > > Yes, I can post a patch that _just_ moves the set of dev->fwnode->dev
> > > > earlier, and that will probably fix my symptoms (I'll need to test).
> > > > This patch already moves it a bit earlier, but if we don't break the
> > > > linking out as a separate step it would need to move even higher up in
> > > > the function.
> > > >
> > > > Originally, I was going to just propose that, but then I realized that
> > > > some of the other code in device_add() probably also ought to run
> > > > before we let the driver probe, and hence I ended up with this patch.
> > >
> > > This sounds like a more generic problem.  A bunch of things happen after
> > > bus_add_device() that should be completed before probing can start; the
> > > firmware node stuff is just one of them.
>
> You know, I wrote this but I'm not so sure that it's accurate.  We've
> gone many years with no big changes to this code; most likely it doesn't
> need alterations now.

That's fair, but I'm really just worried that the Android parallel
module loading code, which is only ~1 year old (and needs to be opted
in for each device) is stressing things in a way that nobody else is.
In most distros, I think modules are loaded "on-demand". A device gets
added first, and then we figure out which module has the driver that's
needed and load the module. Nice and sequential. Android seems to have
a different approach. As far as I understand it just has a list of
modules to load and slams through loading all of them even as devices
are still being added.

I don't think that what Android is doing is technically "wrong", but
it's certainly odd compared to all other distros.

Unless we say that what Android is doing is wrong (and decide what to
do about it), it seems like we need to make sure it's robust.


> > > Splitting bus_add_device() in two sounds reasonable, although I would
> > > rename the old routine to bus_link_device, since all it does it add some
> > > groups and symlinks.  The new routine can be called bus_add_device().
> >
> > LOL, so basically you want them named exactly opposite I did? That's
> > OK with me, though maybe:
> >
> > bus_prep_device()
> > bus_add_device()
>
> Nothing wrong with those names.  But instead of making these fairly
> intrusive changes it might be better just to move the firmware stuff to
> a different place in the code (you mentioned this possibility in your
> first email).  It would be a smaller change, that's for sure.

I'll do that if that's what everyone wants, but the more I think about
it the more worried I am that we'll end up with a hidden / harder to
debug problem where some driver gets unhappy when its probe is called
before dpm_sysfs_add(), device_pm_add(), device_create_file(),
device_create_sys_dev_entry(), BUS_NOTIFY_ADD_DEVICE, ...


> > > The real question is whether any of the other stuff that happens before
> > > bus_probe_device() needs to come after the device is added to the bus's
> > > list.  The bus_notify() and kobject_uevent() calls are good examples; I
> > > don't know what their requirements are.  Should they be moved down,
> > > between the new bus_add_device() and bus_probe_device()?
> >
> > Yes, this is my question too. The question is, what's worse:
> >
> > 1. Potentially the device getting probed before the calls to
> > "bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
> > "kobject_uevent(&dev->kobj, KOBJ_ADD)"
> >
> > 2. Calling "bus_notify(dev, BUS_NOTIFY_ADD_DEVICE)" and
> > "kobject_uevent(&dev->kobj, KOBJ_ADD)" without first adding to the
> > subsystem's list of devices.
> >
> > As it is, my patch says #2 is worse and thus allows for #1. ...but I
> > don't actually know the answer. The main reason I chose to allow for
> > #1 is that it makes the behavior less different than it was before my
> > patch, but that doesn't mean it's correct.
> >
> > Reading  through a handful of "BUS_NOTIFY_ADD_DEVICE" calls, my guess
> > is that they expect to be called before probe... That would imply that
> > my patch made the wrong choice...
>
> I don't know about other subsystems, only USB.  As far as I remember,
> USB doesn't really care about the order of probing vs. notifications.
> In general, since probing can be asynchronous with registration, the
> kernel always has to work with probing after bus_notify().  It's
> best to preserve that order unless there's a very good reason not to.
>
> uevents may be different, since they go to userspace.  It could be
> weird for a user program to be told about a new device and then not be
> able to find it on the bus.
>
> > I think another option here might be to just add a new bitfield flag
> > to "struct device", like "ready_to_probe". Right before the call to
> > bus_probe_device(dev), we could do something like:
> >
> > device_lock(dev);
> > dev->ready_to_probe = true;
> > device_unlcok(dev);
> >
> > Then in __driver_attach() I can have:
> >
> > device_lock(dev);
> > ready_to_probe = dev->ready_to_probe;
> > device_unlock(dev);
> > if (!ready_to_probe)
> >   return 0;
> >
> > If I do that, I don't think I'll even need to re-order anything, and I
> > think it's all safe. Basically, it just the device hidden from the
> > __driver_attach() logic until probe is ready.
> >
> > What do you think?
>
> There should not be any difference between probing caused by the device
> being added to the bus, vs. caused by a new driver being registered, vs.
> caused by anything else (such as sysfs).  None of these should be
> allowed until all of them can be handled properly.

Right. ...and I think that's what my proposed "ready_to_probe" does.
It really does seem like quite a safe change. It _just_ prevents the
driver load path from initiating a probe too early.


> And linking the device into the bus's list of devices should be the
> event that makes probing possible.

Sure, but moving the linking into the bus's list of devices all the
way to the end is definitely a bigger change. If nothing else,
"bus_for_each_dev()" starts to be able to find the device once it's
linked into the list. If any of the ~50 drivers who register for
BUS_NOTIFY_ADD_DEVICE are relying on the device to show up in
"bus_for_each_dev()", it would be bad...

-Doug

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Alan Stern]

On Mon, Mar 23, 2026 at 03:24:52PM -0700, Doug Anderson wrote:
> > You know, I wrote this but I'm not so sure that it's accurate.  We've
> > gone many years with no big changes to this code; most likely it doesn't
> > need alterations now.
> 
> That's fair, but I'm really just worried that the Android parallel
> module loading code, which is only ~1 year old (and needs to be opted
> in for each device) is stressing things in a way that nobody else is.
> In most distros, I think modules are loaded "on-demand". A device gets
> added first, and then we figure out which module has the driver that's
> needed and load the module. Nice and sequential. Android seems to have
> a different approach. As far as I understand it just has a list of
> modules to load and slams through loading all of them even as devices
> are still being added.
> 
> I don't think that what Android is doing is technically "wrong", but
> it's certainly odd compared to all other distros.
> 
> Unless we say that what Android is doing is wrong (and decide what to
> do about it), it seems like we need to make sure it's robust.

Loading a bunch of modules as fast as possible should not be wrong.  The 
kernel certainly ought to be able to handle it.

> > Nothing wrong with those names.  But instead of making these fairly
> > intrusive changes it might be better just to move the firmware stuff to
> > a different place in the code (you mentioned this possibility in your
> > first email).  It would be a smaller change, that's for sure.
> 
> I'll do that if that's what everyone wants, but the more I think about
> it the more worried I am that we'll end up with a hidden / harder to
> debug problem where some driver gets unhappy when its probe is called
> before dpm_sysfs_add(), device_pm_add(), device_create_file(),
> device_create_sys_dev_entry(), BUS_NOTIFY_ADD_DEVICE, ...

It's hard to know for all of them.  However, it seems pretty clear that 
device_pm_add() should come before probing, since a probe routine will 
generally want to affect the device's runtime PM state.

> > There should not be any difference between probing caused by the device
> > being added to the bus, vs. caused by a new driver being registered, vs.
> > caused by anything else (such as sysfs).  None of these should be
> > allowed until all of them can be handled properly.
> 
> Right. ...and I think that's what my proposed "ready_to_probe" does.
> It really does seem like quite a safe change. It _just_ prevents the
> driver load path from initiating a probe too early.

Any such consideration should apply to all the probe paths, not just 
driver loading.  (Also, if it's too early to probe the device, perhaps 
the return code should be -EAGAIN instead of 0.)

I'm not at all sure whether the constraints we've got will need to force 
some events to happen after adding the device to the bus list and before 
allowing probing to start.

> > And linking the device into the bus's list of devices should be the
> > event that makes probing possible.
> 
> Sure, but moving the linking into the bus's list of devices all the
> way to the end is definitely a bigger change. If nothing else,
> "bus_for_each_dev()" starts to be able to find the device once it's
> linked into the list. If any of the ~50 drivers who register for
> BUS_NOTIFY_ADD_DEVICE are relying on the device to show up in
> "bus_for_each_dev()", it would be bad...

I don't know the answer to this.  That is, I don't know if there are any 
notification handlers depending on the device showing up in the bus's 
list.  The safest thing to do is issue the notification after adding the 
device to the list -- which may mean after probing has potentially 
started.  Is there any reason why that would be a problem?  I'm not 
aware of any.

The order constraints should be commented explicitly in device_add(), 
not just implicitly implied by the code.  Otherwise people won't know 
what changes are allowed and what changes are forbidden.

Alan Stern

Re: [RFC PATCH] driver core: Don't link the device to the bus until we're ready to probe

[Doug Anderson]

Hi,

On Tue, Mar 24, 2026 at 8:21 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> > I'll do that if that's what everyone wants, but the more I think about
> > it the more worried I am that we'll end up with a hidden / harder to
> > debug problem where some driver gets unhappy when its probe is called
> > before dpm_sysfs_add(), device_pm_add(), device_create_file(),
> > device_create_sys_dev_entry(), BUS_NOTIFY_ADD_DEVICE, ...
>
> It's hard to know for all of them.  However, it seems pretty clear that
> device_pm_add() should come before probing, since a probe routine will
> generally want to affect the device's runtime PM state.

Yup, that seems right to me, too. It's why I was trying to avoid just
moving fixing the fwdevlink assignment. I didn't want to run into more
hard-to-debug issues later.


> > > There should not be any difference between probing caused by the device
> > > being added to the bus, vs. caused by a new driver being registered, vs.
> > > caused by anything else (such as sysfs).  None of these should be
> > > allowed until all of them can be handled properly.
> >
> > Right. ...and I think that's what my proposed "ready_to_probe" does.
> > It really does seem like quite a safe change. It _just_ prevents the
> > driver load path from initiating a probe too early.
>
> Any such consideration should apply to all the probe paths, not just
> driver loading.  (Also, if it's too early to probe the device, perhaps
> the return code should be -EAGAIN instead of 0.)

In my proposed solution, I was returning 0 from __driver_attach(). The
only place that's called from is driver_attach(), which calls it with
bus_for_each_dev(). I don't think returning -EAGAIN is a good idea
there since it stops bus_for_each_dev(). In general __driver_attach()
always returns 0.

In general, the goal of my new proposed patch is to add the device to
the subsystem's "klist_devices" exactly where we do it today for
maximum compatibility. This means that if any code was relying on
being able to find the device, they can still find it. The _only_
exception is that I don't want to be able to find the device in
driver_attach(). So my proposed solution just hides the device in that
one case.

I believe this should be fine. Specifically, driver_attach() could
have been called (in another thread) immediately before
bus_add_device() and everything would have been fine. driver_attach()
wouldn't have found the device (because it wasn't linked in) but the
probe would still happen.


> I'm not at all sure whether the constraints we've got will need to force
> some events to happen after adding the device to the bus list and before
> allowing probing to start.
>
> > > And linking the device into the bus's list of devices should be the
> > > event that makes probing possible.
> >
> > Sure, but moving the linking into the bus's list of devices all the
> > way to the end is definitely a bigger change. If nothing else,
> > "bus_for_each_dev()" starts to be able to find the device once it's
> > linked into the list. If any of the ~50 drivers who register for
> > BUS_NOTIFY_ADD_DEVICE are relying on the device to show up in
> > "bus_for_each_dev()", it would be bad...
>
> I don't know the answer to this.  That is, I don't know if there are any
> notification handlers depending on the device showing up in the bus's
> list.  The safest thing to do is issue the notification after adding the
> device to the list -- which may mean after probing has potentially
> started.  Is there any reason why that would be a problem?  I'm not
> aware of any.

I'm not completely sure I follow what you're suggesting here...


> The order constraints should be commented explicitly in device_add(),
> not just implicitly implied by the code.  Otherwise people won't know
> what changes are allowed and what changes are forbidden.

Yup! I added comments about ordering constraints in this RFC patch,
and will continue to do so as it evolves.

I still believe adding a flag that just hides the device from
driver_attach() is a safe and correct approach. In general I don't
want to fragment the discussoin, but I think it might be useful to
send a v2 that shows what that looks like. Any objections?

-Doug