Linux driver-core infrastructure
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
@ 2026-08-11  0:47 Daniel Golle
  2026-08-11  2:33 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Golle @ 2026-08-11  0:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	driver-core, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Andrew Lunn, Vladimir Oltean
  Cc: netdev, linux-kernel, Miri Korenblit, linux-wireless,
	Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth,
	Hans de Goede

Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
deferred re-probe of their own device from a work item in module
text. The hand-rolled copies share two bug classes: the work function
ends with module_put(THIS_MODULE), racing a concurrent rmmod freeing
the module text (the race module_put_and_kthread_exit() exists to
close for kthreads), and nothing synchronizes the deferred detach
against device_shutdown() or an administrative unbind.

Patch 1 moves the deferred work into the driver core.
device_schedule_reprobe() runs builtin code, so no module reference
is needed; it checks under a single __device_driver_lock() hold that
the device is still bound to the driver that scheduled the re-probe,
and skips the detach once device_shutdown() has reached the device
(a new one-bit shutdown_done flag in struct device_private). Patches
2 and 3 are mechanical conversions. Patch 4 (btintel_pcie) also
removes that driver's remove()-from-own-work contract; it changes
more and can be dropped without affecting patches 1-3.

This grew out of review of the mxl862xx DSA series [1][2], which
copied the iwlwifi pattern. There ->shutdown() clears drvdata so that
->remove() becomes a no-op, a stale re-probe escalates to
use-after-free, and its v10 cover letter walks through why a driver
cannot fully close the race itself: device_shutdown() holds
device_lock() across ->shutdown() and device_reprobe() takes the same
lock, so any driver-side check remains a TOCTOU. The helper closes it
in the core; mxl862xx will convert once it lands.

Testing: checkpatch --strict and kernel-doc clean. Patches 1-3 were
runtime-tested (backported to 7.0.11) on Intel AX101 hardware with
PROVE_LOCKING and DEBUG_OBJECTS_WORK, driving iwlwifi's crash
escalation into the re-probe path: normal detach+rebind, an unbind
racing a pending re-probe (the unbind is not undone), rmmod with a
re-probe pending (now succeeds instead of EBUSY), and reboot with a
re-probe pending; no lockdep or debugobjects reports. hci_h5 and
btintel_pcie are compile-tested only.

[1] https://lore.kernel.org/all/cover.1786294649.git.daniel@makrotopia.org/
[2] https://sashiko.dev/#/patchset/cover.1786294649.git.daniel%40makrotopia.org

Daniel Golle (4):
  driver core: add device_schedule_reprobe()
  wifi: iwlwifi: use device_schedule_reprobe()
  Bluetooth: hci_h5: use device_schedule_reprobe()
  Bluetooth: btintel_pcie: use device_schedule_reprobe() after reset

 drivers/base/base.h                           |  5 ++
 drivers/base/core.c                           |  3 +
 drivers/base/dd.c                             | 82 +++++++++++++++++++
 drivers/bluetooth/btintel_pcie.c              | 48 ++++++-----
 drivers/bluetooth/hci_h5.c                    | 41 ++--------
 .../net/wireless/intel/iwlwifi/iwl-trans.c    | 40 +--------
 include/linux/device.h                        |  2 +
 7 files changed, 122 insertions(+), 99 deletions(-)

-- 
2.55.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  0:47 [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions Daniel Golle
@ 2026-08-11  2:33 ` Greg Kroah-Hartman
  2026-08-11  7:32   ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-11  2:33 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Rafael J. Wysocki, Danilo Krummrich, driver-core, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth, Hans de Goede

On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
> deferred re-probe of their own device from a work item in module
> text.

That's a mess, why?  Why not fix that up to not do that?  Thousands of
other kernel drivers do not do that, what makes these so special?

> The hand-rolled copies share two bug classes: the work function
> ends with module_put(THIS_MODULE),

That's broken as-is.  a module should NEVER be calling
module_get(THIS_MODULE) either.

> racing a concurrent rmmod freeing
> the module text (the race module_put_and_kthread_exit() exists to
> close for kthreads), and nothing synchronizes the deferred detach
> against device_shutdown() or an administrative unbind.

yeah, that's a mess, don't do that.

Fix up the original drivers please, let's not encourage others to copy
this broken scheme.

Also, your patches were not threaded properly :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  2:33 ` Greg Kroah-Hartman
@ 2026-08-11  7:32   ` Hans de Goede
  2026-08-11  8:07     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-08-11  7:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Daniel Golle
  Cc: Rafael J. Wysocki, Danilo Krummrich, driver-core, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth

Hi Greg,

On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
> On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
>> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
>> deferred re-probe of their own device from a work item in module
>> text.
> 
> That's a mess, why?  Why not fix that up to not do that?  Thousands of
> other kernel drivers do not do that, what makes these so special?

I can only speak for the hci_h5 driver where I added the reprobe code-path.

The problem is some of the Bluetooth HCI devices using hci_h5 loose all
state during system-suspend. This means that the HCI and the Bluetooth core
end up being out of sync.

So we basically need to tear down and re-build everything including
e.g. the firmware upload which happens at probe(). Doing a full reprobe
is by far the easiest way to do this.

I suspect the other 4 users + the pending driver which triggered
this are similar.

Sure we can do the whole tear-down + setup from some worker
scheduled at resume, while keep the driver attached but if we need
to duplicate that over 4 drivers + the pending driver which triggers
this then IMHO those 5 users are a pattern which deserves having
some helper to do this through the existing probe() + remove(),
rather then requiring those 5 drivers to open code this themselves.

Note that we already have device_reprobe(), which has 15 existing
users. This series just adds a helper to do a device_reprobe() from
a worker in a safe way.

Regards,

Hans




> 
>> The hand-rolled copies share two bug classes: the work function
>> ends with module_put(THIS_MODULE),
> 
> That's broken as-is.  a module should NEVER be calling
> module_get(THIS_MODULE) either.
> 
>> racing a concurrent rmmod freeing
>> the module text (the race module_put_and_kthread_exit() exists to
>> close for kthreads), and nothing synchronizes the deferred detach
>> against device_shutdown() or an administrative unbind.
> 
> yeah, that's a mess, don't do that.
> 
> Fix up the original drivers please, let's not encourage others to copy
> this broken scheme.
> 
> Also, your patches were not threaded properly :(
> 
> thanks,
> 
> greg k-h


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  7:32   ` Hans de Goede
@ 2026-08-11  8:07     ` Greg Kroah-Hartman
  2026-08-11  8:51       ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-11  8:07 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Daniel Golle, Rafael J. Wysocki, Danilo Krummrich, driver-core,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth

On Tue, Aug 11, 2026 at 09:32:45AM +0200, Hans de Goede wrote:
> Hi Greg,
> 
> On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
> > On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
> >> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
> >> deferred re-probe of their own device from a work item in module
> >> text.
> > 
> > That's a mess, why?  Why not fix that up to not do that?  Thousands of
> > other kernel drivers do not do that, what makes these so special?
> 
> I can only speak for the hci_h5 driver where I added the reprobe code-path.
> 
> The problem is some of the Bluetooth HCI devices using hci_h5 loose all
> state during system-suspend. This means that the HCI and the Bluetooth core
> end up being out of sync.
> 
> So we basically need to tear down and re-build everything including
> e.g. the firmware upload which happens at probe(). Doing a full reprobe
> is by far the easiest way to do this.
> 
> I suspect the other 4 users + the pending driver which triggered
> this are similar.

Full reprobe feels different than the THIS_MODULE stuff, which is what I
objected to here.

> Sure we can do the whole tear-down + setup from some worker
> scheduled at resume, while keep the driver attached but if we need
> to duplicate that over 4 drivers + the pending driver which triggers
> this then IMHO those 5 users are a pattern which deserves having
> some helper to do this through the existing probe() + remove(),
> rather then requiring those 5 drivers to open code this themselves.
> 
> Note that we already have device_reprobe(), which has 15 existing
> users. This series just adds a helper to do a device_reprobe() from
> a worker in a safe way.

That feels a bit better, as long as this code really is "safe" :)

So, how can this be tested and fixed up so it isn't a RFC anymore?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  8:07     ` Greg Kroah-Hartman
@ 2026-08-11  8:51       ` Hans de Goede
  2026-08-11 15:01         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-08-11  8:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Daniel Golle, Rafael J. Wysocki, Danilo Krummrich, driver-core,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth

Hi,

On 11-Aug-26 10:07, Greg Kroah-Hartman wrote:
> On Tue, Aug 11, 2026 at 09:32:45AM +0200, Hans de Goede wrote:
>> Hi Greg,
>>
>> On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
>>> On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
>>>> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
>>>> deferred re-probe of their own device from a work item in module
>>>> text.
>>>
>>> That's a mess, why?  Why not fix that up to not do that?  Thousands of
>>> other kernel drivers do not do that, what makes these so special?
>>
>> I can only speak for the hci_h5 driver where I added the reprobe code-path.
>>
>> The problem is some of the Bluetooth HCI devices using hci_h5 loose all
>> state during system-suspend. This means that the HCI and the Bluetooth core
>> end up being out of sync.
>>
>> So we basically need to tear down and re-build everything including
>> e.g. the firmware upload which happens at probe(). Doing a full reprobe
>> is by far the easiest way to do this.
>>
>> I suspect the other 4 users + the pending driver which triggered
>> this are similar.
> 
> Full reprobe feels different than the THIS_MODULE stuff, which is what I
> objected to here.

Right, the helper from 1/4 to safely do reprobe from a worker actually
removes the need for THIS_MODULE stuff :)

The THIS_MODULE stuff in the open-coded implementations is there to
avoid someone doing a rmmod while the reprobe is running. The new
helper replaces this with some checks in the workqueue function
checking the driver has not been rmmod-ed in the mean time.

>> Sure we can do the whole tear-down + setup from some worker
>> scheduled at resume, while keep the driver attached but if we need
>> to duplicate that over 4 drivers + the pending driver which triggers
>> this then IMHO those 5 users are a pattern which deserves having
>> some helper to do this through the existing probe() + remove(),
>> rather then requiring those 5 drivers to open code this themselves.
>>
>> Note that we already have device_reprobe(), which has 15 existing
>> users. This series just adds a helper to do a device_reprobe() from
>> a worker in a safe way.
> 
> That feels a bit better, as long as this code really is "safe" :)
> 
> So, how can this be tested and fixed up so it isn't a RFC anymore?

To me it looks like the main things are:

1. Agree that a helper to safely do a device_reprobe() from a worker
is helpful (I think this is done now?)

2. Get patch 1/4 reviewed. I can do an initial review but I'm not very
familiar with the driver/device core internals.

3. Test this. I can test this on a hci_h5 BT HCI that will hit this
code path.

I'll try to get 2. and 3. done soon-ish.

Regards,

Hans



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  8:51       ` Hans de Goede
@ 2026-08-11 15:01         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2026-08-11 15:01 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Greg Kroah-Hartman, Daniel Golle, Rafael J. Wysocki,
	Danilo Krummrich, driver-core, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Andrew Lunn,
	Vladimir Oltean, netdev, linux-kernel, Miri Korenblit,
	linux-wireless, Marcel Holtmann, linux-bluetooth, Johannes Berg,
	Srivatsa, Ravishankar, Kiran K, ChandraShekar

Hi Hans,

On Tue, Aug 11, 2026 at 4:51 AM Hans de Goede
<johannes.goede@oss.qualcomm.com> wrote:
>
> Hi,
>
> On 11-Aug-26 10:07, Greg Kroah-Hartman wrote:
> > On Tue, Aug 11, 2026 at 09:32:45AM +0200, Hans de Goede wrote:
> >> Hi Greg,
> >>
> >> On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
> >>> On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
> >>>> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
> >>>> deferred re-probe of their own device from a work item in module
> >>>> text.
> >>>
> >>> That's a mess, why?  Why not fix that up to not do that?  Thousands of
> >>> other kernel drivers do not do that, what makes these so special?
> >>
> >> I can only speak for the hci_h5 driver where I added the reprobe code-path.
> >>
> >> The problem is some of the Bluetooth HCI devices using hci_h5 loose all
> >> state during system-suspend. This means that the HCI and the Bluetooth core
> >> end up being out of sync.
> >>
> >> So we basically need to tear down and re-build everything including
> >> e.g. the firmware upload which happens at probe(). Doing a full reprobe
> >> is by far the easiest way to do this.
> >>
> >> I suspect the other 4 users + the pending driver which triggered
> >> this are similar.
> >
> > Full reprobe feels different than the THIS_MODULE stuff, which is what I
> > objected to here.
>
> Right, the helper from 1/4 to safely do reprobe from a worker actually
> removes the need for THIS_MODULE stuff :)
>
> The THIS_MODULE stuff in the open-coded implementations is there to
> avoid someone doing a rmmod while the reprobe is running. The new
> helper replaces this with some checks in the workqueue function
> checking the driver has not been rmmod-ed in the mean time.
>
> >> Sure we can do the whole tear-down + setup from some worker
> >> scheduled at resume, while keep the driver attached but if we need
> >> to duplicate that over 4 drivers + the pending driver which triggers
> >> this then IMHO those 5 users are a pattern which deserves having
> >> some helper to do this through the existing probe() + remove(),
> >> rather then requiring those 5 drivers to open code this themselves.
> >>
> >> Note that we already have device_reprobe(), which has 15 existing
> >> users. This series just adds a helper to do a device_reprobe() from
> >> a worker in a safe way.
> >
> > That feels a bit better, as long as this code really is "safe" :)
> >
> > So, how can this be tested and fixed up so it isn't a RFC anymore?
>
> To me it looks like the main things are:
>
> 1. Agree that a helper to safely do a device_reprobe() from a worker
> is helpful (I think this is done now?)
>
> 2. Get patch 1/4 reviewed. I can do an initial review but I'm not very
> familiar with the driver/device core internals.
>
> 3. Test this. I can test this on a hci_h5 BT HCI that will hit this
> code path.
>
> I'll try to get 2. and 3. done soon-ish.

The 2 Intel drivers are related; they are part of a combo Wi-Fi + BT
solution. This is why they attempt to reprobe each other to recover if
the entire system (not just wifi or bt) has run into a fault and needs
to be recovered as a whole.

As for introducing device_schedule_reprobe that seem to be a great
initiative, thanks for leading it.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-11 15:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11  0:47 [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions Daniel Golle
2026-08-11  2:33 ` Greg Kroah-Hartman
2026-08-11  7:32   ` Hans de Goede
2026-08-11  8:07     ` Greg Kroah-Hartman
2026-08-11  8:51       ` Hans de Goede
2026-08-11 15:01         ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox