All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.