* [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