Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 8, 2015 at 9:44 PM, Dmitry Torokhov wrote: > On Wed, Jul 08, 2015 at 06:00:41PM -0700, Dan Williams wrote: >> On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov >> wrote: >> > On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote: >> >> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov >> >> wrote: >> >> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: >> >> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> >> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez >> >> >> > wrote: >> >> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: >> >> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov >> >> >> > >> wrote: >> >> >> > >> > Some devices take a long time when initializing, and not all >> >> >> > >> > drivers are >> >> >> > >> > suited to initialize their devices when they are open. For >> >> >> > >> > example, >> >> >> > >> > input drivers need to interrogate their devices in order to >> >> >> > >> > publish >> >> >> > >> > device's capabilities before userspace will open them. When >> >> >> > >> > such drivers >> >> >> > >> > are compiled into kernel they may stall entire kernel >> >> >> > >> > initialization. >> >> >> > >> > >> >> >> > >> > This change allows drivers request for their probe functions to >> >> >> > >> > be >> >> >> > >> > called asynchronously during driver and device registration >> >> >> > >> > (manual >> >> >> > >> > binding is still synchronous). Because async_schedule is used >> >> >> > >> > to perform >> >> >> > >> > asynchronous calls module loading will still wait for the >> >> >> > >> > probing to >> >> >> > >> > complete. >> >> >> > >> > >> >> >> > >> > Note that the end goal is to make the probing asynchronous by >> >> >> > >> > default, >> >> >> > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a >> >> >> > >> > temporary >> >> >> > >> > measure that allows us to speed up boot process while we >> >> >> > >> > validating and >> >> >> > >> > fixing the rest of the drivers and preparing userspace. >> >> >> > >> > >> >> >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" >> >> >> > >> > >> >> >> > >> > >> >> >> > >> > Signed-off-by: Dmitry Torokhov >> >> >> > >> > --- >> >> >> > >> > drivers/base/base.h| 1 + >> >> >> > >> > drivers/base/bus.c | 31 +++--- >> >> >> > >> > drivers/base/dd.c | 149 >> >> >> > >> > ++--- >> >> >> > >> > include/linux/device.h | 28 ++ >> >> >> > >> > 4 files changed, 182 insertions(+), 27 deletions(-) >> >> >> > >> >> >> >> > >> Just noticed this patch. It caught my eye because I had a hard >> >> >> > >> time >> >> >> > >> getting an open coded implementation of asynchronous probing to >> >> >> > >> work >> >> >> > >> in the new libnvdimm subsystem. Especially the messy races of >> >> >> > >> tearing >> >> >> > >> things down while probing is still in flight. I ended up >> >> >> > >> implementing >> >> >> > >> asynchronous device registration which eliminated a lot of >> >> >> > >> complexity >> >> >> > >> and of course the bugs. In general I tend to think that async >> >> >> > >> registration is less risky than async probe since it keeps wider >> >> >> > >> portions of the traditional device model synchronous >> >> >> > > >> >> >> > > but its not see -DEFER_PROBE even before async probe. >> >> >> > >> >> >> > Except in that case you know probe has been seen by the driver at >> >> >> > least once. So I see that as less of a surprise, but point taken. >> >> >> > >> >> >> > >> and leverages the >> >> >> > >> fact that the device model is already well prepared for >> >> >> > >> asynchronous >> >> >> > >> arrival of devices due to hotplug. >> >> >> > > >> >> >> > > I think this sounds reasonable, do you have your code upstream or >> >> >> > > posted? >> >> >> > >> >> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c >> >> >> >> >> >> It should be I think rather easy for Dmitry to see if he can convert >> >> >> this input >> >> >> driver (not yet upstream) to this API and see if the same issues are >> >> >> fixed. >> >> > >> >> > No, I would rather not as it means we lose error handling on device >> >> > registration. >> >> > >> >> >> >> I think this is a red herring as I don't see how async probing is any >> >> better at handling device registration errors. The error is logged >> >> and "handled" by the fact that a device fails to appear, what other >> >> action would you take? In fact libnvdimm does detect registration >> >> failures and reports that in a parent device attribute (at least for a >> >> region device and their namespace child devices). >> > >> > What is libnvdimm behavior if you try to unload a module that tries to >> > register a device but it failed? Memory leak or crash, right? >> >> No, in the case of the "region" driver it is part of the core >> libnvdimm and it is pinned while any region device is active
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 08, 2015 at 06:00:41PM -0700, Dan Williams wrote: > On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov > wrote: > > On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote: > >> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov > >> wrote: > >> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: > >> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > >> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez > >> >> > wrote: > >> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > >> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > >> >> > >> wrote: > >> >> > >> > Some devices take a long time when initializing, and not all > >> >> > >> > drivers are > >> >> > >> > suited to initialize their devices when they are open. For > >> >> > >> > example, > >> >> > >> > input drivers need to interrogate their devices in order to > >> >> > >> > publish > >> >> > >> > device's capabilities before userspace will open them. When such > >> >> > >> > drivers > >> >> > >> > are compiled into kernel they may stall entire kernel > >> >> > >> > initialization. > >> >> > >> > > >> >> > >> > This change allows drivers request for their probe functions to > >> >> > >> > be > >> >> > >> > called asynchronously during driver and device registration > >> >> > >> > (manual > >> >> > >> > binding is still synchronous). Because async_schedule is used to > >> >> > >> > perform > >> >> > >> > asynchronous calls module loading will still wait for the > >> >> > >> > probing to > >> >> > >> > complete. > >> >> > >> > > >> >> > >> > Note that the end goal is to make the probing asynchronous by > >> >> > >> > default, > >> >> > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a > >> >> > >> > temporary > >> >> > >> > measure that allows us to speed up boot process while we > >> >> > >> > validating and > >> >> > >> > fixing the rest of the drivers and preparing userspace. > >> >> > >> > > >> >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" > >> >> > >> > > >> >> > >> > > >> >> > >> > Signed-off-by: Dmitry Torokhov > >> >> > >> > --- > >> >> > >> > drivers/base/base.h| 1 + > >> >> > >> > drivers/base/bus.c | 31 +++--- > >> >> > >> > drivers/base/dd.c | 149 > >> >> > >> > ++--- > >> >> > >> > include/linux/device.h | 28 ++ > >> >> > >> > 4 files changed, 182 insertions(+), 27 deletions(-) > >> >> > >> > >> >> > >> Just noticed this patch. It caught my eye because I had a hard > >> >> > >> time > >> >> > >> getting an open coded implementation of asynchronous probing to > >> >> > >> work > >> >> > >> in the new libnvdimm subsystem. Especially the messy races of > >> >> > >> tearing > >> >> > >> things down while probing is still in flight. I ended up > >> >> > >> implementing > >> >> > >> asynchronous device registration which eliminated a lot of > >> >> > >> complexity > >> >> > >> and of course the bugs. In general I tend to think that async > >> >> > >> registration is less risky than async probe since it keeps wider > >> >> > >> portions of the traditional device model synchronous > >> >> > > > >> >> > > but its not see -DEFER_PROBE even before async probe. > >> >> > > >> >> > Except in that case you know probe has been seen by the driver at > >> >> > least once. So I see that as less of a surprise, but point taken. > >> >> > > >> >> > >> and leverages the > >> >> > >> fact that the device model is already well prepared for > >> >> > >> asynchronous > >> >> > >> arrival of devices due to hotplug. > >> >> > > > >> >> > > I think this sounds reasonable, do you have your code upstream or > >> >> > > posted? > >> >> > > >> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c > >> >> > >> >> It should be I think rather easy for Dmitry to see if he can convert > >> >> this input > >> >> driver (not yet upstream) to this API and see if the same issues are > >> >> fixed. > >> > > >> > No, I would rather not as it means we lose error handling on device > >> > registration. > >> > > >> > >> I think this is a red herring as I don't see how async probing is any > >> better at handling device registration errors. The error is logged > >> and "handled" by the fact that a device fails to appear, what other > >> action would you take? In fact libnvdimm does detect registration > >> failures and reports that in a parent device attribute (at least for a > >> region device and their namespace child devices). > > > > What is libnvdimm behavior if you try to unload a module that tries to > > register a device but it failed? Memory leak or crash, right? > > No, in the case of the "region" driver it is part of the core > libnvdimm and it is pinned while any region device is active. No, not quite. Let's take a look for example at nd_btt_probe(). It calls __nd_btt_create() which in turn calls __nd_device_register() which returns void and
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov wrote: > On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote: >> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov >> wrote: >> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: >> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez >> >> > wrote: >> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: >> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov >> >> > >> wrote: >> >> > >> > Some devices take a long time when initializing, and not all >> >> > >> > drivers are >> >> > >> > suited to initialize their devices when they are open. For example, >> >> > >> > input drivers need to interrogate their devices in order to publish >> >> > >> > device's capabilities before userspace will open them. When such >> >> > >> > drivers >> >> > >> > are compiled into kernel they may stall entire kernel >> >> > >> > initialization. >> >> > >> > >> >> > >> > This change allows drivers request for their probe functions to be >> >> > >> > called asynchronously during driver and device registration (manual >> >> > >> > binding is still synchronous). Because async_schedule is used to >> >> > >> > perform >> >> > >> > asynchronous calls module loading will still wait for the probing >> >> > >> > to >> >> > >> > complete. >> >> > >> > >> >> > >> > Note that the end goal is to make the probing asynchronous by >> >> > >> > default, >> >> > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary >> >> > >> > measure that allows us to speed up boot process while we >> >> > >> > validating and >> >> > >> > fixing the rest of the drivers and preparing userspace. >> >> > >> > >> >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" >> >> > >> > >> >> > >> > >> >> > >> > Signed-off-by: Dmitry Torokhov >> >> > >> > --- >> >> > >> > drivers/base/base.h| 1 + >> >> > >> > drivers/base/bus.c | 31 +++--- >> >> > >> > drivers/base/dd.c | 149 >> >> > >> > ++--- >> >> > >> > include/linux/device.h | 28 ++ >> >> > >> > 4 files changed, 182 insertions(+), 27 deletions(-) >> >> > >> >> >> > >> Just noticed this patch. It caught my eye because I had a hard time >> >> > >> getting an open coded implementation of asynchronous probing to work >> >> > >> in the new libnvdimm subsystem. Especially the messy races of >> >> > >> tearing >> >> > >> things down while probing is still in flight. I ended up >> >> > >> implementing >> >> > >> asynchronous device registration which eliminated a lot of complexity >> >> > >> and of course the bugs. In general I tend to think that async >> >> > >> registration is less risky than async probe since it keeps wider >> >> > >> portions of the traditional device model synchronous >> >> > > >> >> > > but its not see -DEFER_PROBE even before async probe. >> >> > >> >> > Except in that case you know probe has been seen by the driver at >> >> > least once. So I see that as less of a surprise, but point taken. >> >> > >> >> > >> and leverages the >> >> > >> fact that the device model is already well prepared for asynchronous >> >> > >> arrival of devices due to hotplug. >> >> > > >> >> > > I think this sounds reasonable, do you have your code upstream or >> >> > > posted? >> >> > >> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c >> >> >> >> It should be I think rather easy for Dmitry to see if he can convert this >> >> input >> >> driver (not yet upstream) to this API and see if the same issues are >> >> fixed. >> > >> > No, I would rather not as it means we lose error handling on device >> > registration. >> > >> >> I think this is a red herring as I don't see how async probing is any >> better at handling device registration errors. The error is logged >> and "handled" by the fact that a device fails to appear, what other >> action would you take? In fact libnvdimm does detect registration >> failures and reports that in a parent device attribute (at least for a >> region device and their namespace child devices). > > What is libnvdimm behavior if you try to unload a module that tries to > register a device but it failed? Memory leak or crash, right? No, in the case of the "region" driver it is part of the core libnvdimm and it is pinned while any region device is active. But, it is a fair point for a generic facility it would need to consider cases where a driver registers children and then is unloaded. I'll need to think about that one more relative to async probing. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 08, 2015 at 05:54:26PM -0700, Dan Williams wrote: > On Wed, Jul 8, 2015 at 5:52 PM, Dmitry Torokhov > wrote: > > On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote: > >> On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov > >> wrote: > >> > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > >> >> The problem I ran into was needing to remove devices that still had > >> >> yet to be probed and not being able to use registration completion vs > >> >> the device_lock() to effectively synchronize the sub-system. > >> > > >> > Why do you need to "synchronize the sub-system"? The asynchronous > >> > probing should be transparent to the driver. Just unregister the device > >> > (or the driver) and driver core will ensure that probe() is not in > >> > flight. > >> > >> Async registration is indeed transparent to the driver. The primary > >> need to "flush registration" is the case of "region" devices that > >> reference a set of NVDIMM devices. A region device requires all > >> related NVDIMMs to be active before the region can be enabled. > > > > Sounds like you need to call into the subsystem to let it know that the > > device is active and activate region devices when they are ready. Could > > be either explicit call or you can try using bus notifiers for > > bind/unbind events. > > > > BTW, do you handle bind/unbind via sysfs (everyone forgets about this > > mechanism)? > > bind/unbind via systs is central to how libnvdimm operates. It's > covered by our unit tests. Ah, excellent then. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 8, 2015 at 5:52 PM, Dmitry Torokhov wrote: > On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote: >> On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov >> wrote: >> > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> >> The problem I ran into was needing to remove devices that still had >> >> yet to be probed and not being able to use registration completion vs >> >> the device_lock() to effectively synchronize the sub-system. >> > >> > Why do you need to "synchronize the sub-system"? The asynchronous >> > probing should be transparent to the driver. Just unregister the device >> > (or the driver) and driver core will ensure that probe() is not in >> > flight. >> >> Async registration is indeed transparent to the driver. The primary >> need to "flush registration" is the case of "region" devices that >> reference a set of NVDIMM devices. A region device requires all >> related NVDIMMs to be active before the region can be enabled. > > Sounds like you need to call into the subsystem to let it know that the > device is active and activate region devices when they are ready. Could > be either explicit call or you can try using bus notifiers for > bind/unbind events. > > BTW, do you handle bind/unbind via sysfs (everyone forgets about this > mechanism)? bind/unbind via systs is central to how libnvdimm operates. It's covered by our unit tests. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote: > On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov > wrote: > > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > >> The problem I ran into was needing to remove devices that still had > >> yet to be probed and not being able to use registration completion vs > >> the device_lock() to effectively synchronize the sub-system. > > > > Why do you need to "synchronize the sub-system"? The asynchronous > > probing should be transparent to the driver. Just unregister the device > > (or the driver) and driver core will ensure that probe() is not in > > flight. > > Async registration is indeed transparent to the driver. The primary > need to "flush registration" is the case of "region" devices that > reference a set of NVDIMM devices. A region device requires all > related NVDIMMs to be active before the region can be enabled. Sounds like you need to call into the subsystem to let it know that the device is active and activate region devices when they are ready. Could be either explicit call or you can try using bus notifiers for bind/unbind events. BTW, do you handle bind/unbind via sysfs (everyone forgets about this mechanism)? Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote: > On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov > wrote: > > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: > >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez > >> > wrote: > >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > >> > >> wrote: > >> > >> > Some devices take a long time when initializing, and not all > >> > >> > drivers are > >> > >> > suited to initialize their devices when they are open. For example, > >> > >> > input drivers need to interrogate their devices in order to publish > >> > >> > device's capabilities before userspace will open them. When such > >> > >> > drivers > >> > >> > are compiled into kernel they may stall entire kernel > >> > >> > initialization. > >> > >> > > >> > >> > This change allows drivers request for their probe functions to be > >> > >> > called asynchronously during driver and device registration (manual > >> > >> > binding is still synchronous). Because async_schedule is used to > >> > >> > perform > >> > >> > asynchronous calls module loading will still wait for the probing to > >> > >> > complete. > >> > >> > > >> > >> > Note that the end goal is to make the probing asynchronous by > >> > >> > default, > >> > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > >> > >> > measure that allows us to speed up boot process while we validating > >> > >> > and > >> > >> > fixing the rest of the drivers and preparing userspace. > >> > >> > > >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" > >> > >> > > >> > >> > > >> > >> > Signed-off-by: Dmitry Torokhov > >> > >> > --- > >> > >> > drivers/base/base.h| 1 + > >> > >> > drivers/base/bus.c | 31 +++--- > >> > >> > drivers/base/dd.c | 149 > >> > >> > ++--- > >> > >> > include/linux/device.h | 28 ++ > >> > >> > 4 files changed, 182 insertions(+), 27 deletions(-) > >> > >> > >> > >> Just noticed this patch. It caught my eye because I had a hard time > >> > >> getting an open coded implementation of asynchronous probing to work > >> > >> in the new libnvdimm subsystem. Especially the messy races of tearing > >> > >> things down while probing is still in flight. I ended up implementing > >> > >> asynchronous device registration which eliminated a lot of complexity > >> > >> and of course the bugs. In general I tend to think that async > >> > >> registration is less risky than async probe since it keeps wider > >> > >> portions of the traditional device model synchronous > >> > > > >> > > but its not see -DEFER_PROBE even before async probe. > >> > > >> > Except in that case you know probe has been seen by the driver at > >> > least once. So I see that as less of a surprise, but point taken. > >> > > >> > >> and leverages the > >> > >> fact that the device model is already well prepared for asynchronous > >> > >> arrival of devices due to hotplug. > >> > > > >> > > I think this sounds reasonable, do you have your code upstream or > >> > > posted? > >> > > >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c > >> > >> It should be I think rather easy for Dmitry to see if he can convert this > >> input > >> driver (not yet upstream) to this API and see if the same issues are fixed. > > > > No, I would rather not as it means we lose error handling on device > > registration. > > > > I think this is a red herring as I don't see how async probing is any > better at handling device registration errors. The error is logged > and "handled" by the fact that a device fails to appear, what other > action would you take? In fact libnvdimm does detect registration > failures and reports that in a parent device attribute (at least for a > region device and their namespace child devices). What is libnvdimm behavior if you try to unload a module that tries to register a device but it failed? Memory leak or crash, right? -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov wrote: > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> The problem I ran into was needing to remove devices that still had >> yet to be probed and not being able to use registration completion vs >> the device_lock() to effectively synchronize the sub-system. > > Why do you need to "synchronize the sub-system"? The asynchronous > probing should be transparent to the driver. Just unregister the device > (or the driver) and driver core will ensure that probe() is not in > flight. Async registration is indeed transparent to the driver. The primary need to "flush registration" is the case of "region" devices that reference a set of NVDIMM devices. A region device requires all related NVDIMMs to be active before the region can be enabled. I'll look into a more concrete example of the tradeoffs between asynchronous probing vs registration. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov wrote: > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov >> > >> wrote: >> > >> > Some devices take a long time when initializing, and not all drivers >> > >> > are >> > >> > suited to initialize their devices when they are open. For example, >> > >> > input drivers need to interrogate their devices in order to publish >> > >> > device's capabilities before userspace will open them. When such >> > >> > drivers >> > >> > are compiled into kernel they may stall entire kernel initialization. >> > >> > >> > >> > This change allows drivers request for their probe functions to be >> > >> > called asynchronously during driver and device registration (manual >> > >> > binding is still synchronous). Because async_schedule is used to >> > >> > perform >> > >> > asynchronous calls module loading will still wait for the probing to >> > >> > complete. >> > >> > >> > >> > Note that the end goal is to make the probing asynchronous by default, >> > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary >> > >> > measure that allows us to speed up boot process while we validating >> > >> > and >> > >> > fixing the rest of the drivers and preparing userspace. >> > >> > >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" >> > >> > >> > >> > >> > >> > Signed-off-by: Dmitry Torokhov >> > >> > --- >> > >> > drivers/base/base.h| 1 + >> > >> > drivers/base/bus.c | 31 +++--- >> > >> > drivers/base/dd.c | 149 >> > >> > ++--- >> > >> > include/linux/device.h | 28 ++ >> > >> > 4 files changed, 182 insertions(+), 27 deletions(-) >> > >> >> > >> Just noticed this patch. It caught my eye because I had a hard time >> > >> getting an open coded implementation of asynchronous probing to work >> > >> in the new libnvdimm subsystem. Especially the messy races of tearing >> > >> things down while probing is still in flight. I ended up implementing >> > >> asynchronous device registration which eliminated a lot of complexity >> > >> and of course the bugs. In general I tend to think that async >> > >> registration is less risky than async probe since it keeps wider >> > >> portions of the traditional device model synchronous >> > > >> > > but its not see -DEFER_PROBE even before async probe. >> > >> > Except in that case you know probe has been seen by the driver at >> > least once. So I see that as less of a surprise, but point taken. >> > >> > >> and leverages the >> > >> fact that the device model is already well prepared for asynchronous >> > >> arrival of devices due to hotplug. >> > > >> > > I think this sounds reasonable, do you have your code upstream or posted? >> > >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c >> >> It should be I think rather easy for Dmitry to see if he can convert this >> input >> driver (not yet upstream) to this API and see if the same issues are fixed. > > No, I would rather not as it means we lose error handling on device > registration. > I think this is a red herring as I don't see how async probing is any better at handling device registration errors. The error is logged and "handled" by the fact that a device fails to appear, what other action would you take? In fact libnvdimm does detect registration failures and reports that in a parent device attribute (at least for a region device and their namespace child devices). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Tue, Jul 7, 2015 at 1:23 AM, Luis R. Rodriguez wrote: > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: >> On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: >> > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: >> >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov >> >> wrote: >> >> > Some devices take a long time when initializing, and not all drivers are >> >> > suited to initialize their devices when they are open. For example, >> >> > input drivers need to interrogate their devices in order to publish >> >> > device's capabilities before userspace will open them. When such drivers >> >> > are compiled into kernel they may stall entire kernel initialization. >> >> > >> >> > This change allows drivers request for their probe functions to be >> >> > called asynchronously during driver and device registration (manual >> >> > binding is still synchronous). Because async_schedule is used to perform >> >> > asynchronous calls module loading will still wait for the probing to >> >> > complete. >> >> > >> >> > Note that the end goal is to make the probing asynchronous by default, >> >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary >> >> > measure that allows us to speed up boot process while we validating and >> >> > fixing the rest of the drivers and preparing userspace. >> >> > >> >> > This change is based on earlier patch by "Luis R. Rodriguez" >> >> > >> >> > >> >> > Signed-off-by: Dmitry Torokhov >> >> > --- >> >> > drivers/base/base.h| 1 + >> >> > drivers/base/bus.c | 31 +++--- >> >> > drivers/base/dd.c | 149 >> >> > ++--- >> >> > include/linux/device.h | 28 ++ >> >> > 4 files changed, 182 insertions(+), 27 deletions(-) >> >> >> >> Just noticed this patch. It caught my eye because I had a hard time >> >> getting an open coded implementation of asynchronous probing to work >> >> in the new libnvdimm subsystem. Especially the messy races of tearing >> >> things down while probing is still in flight. I ended up implementing >> >> asynchronous device registration which eliminated a lot of complexity >> >> and of course the bugs. In general I tend to think that async >> >> registration is less risky than async probe since it keeps wider >> >> portions of the traditional device model synchronous >> > >> > but its not see -DEFER_PROBE even before async probe. >> >> Except in that case you know probe has been seen by the driver at >> least once. So I see that as less of a surprise, but point taken. >> >> >> and leverages the >> >> fact that the device model is already well prepared for asynchronous >> >> arrival of devices due to hotplug. >> > >> > I think this sounds reasonable, do you have your code upstream or posted? >> >> Yes, see nd_device_register() in drivers/nvdimm/bus.c > > It should be I think rather easy for Dmitry to see if he can convert this > input > driver (not yet upstream) to this API and see if the same issues are fixed. > This however does not address systemd's assumption over detachment of module > load and probe. The inherent problem there was the timeout implemented and > carried in systemd over the worker that uses modlib to load modules. Upon > review the code was complex enough already and surely increasing the timeout > helps but that doesn't address all issues with a general timeout in place. > At SUSE we ended up not using a timeout for kmod built-in commands. That > leaves the original timeout purpose in place. The code for async probe was > not put in the kernel though but since its now upstream we should be able > to replace that userspace systemd work around with async probe, but systemd > folks would need to decide what they want to do. For full gory details of > this refer to: > > https://bugzilla.novell.com/show_bug.cgi?id=889297 FTR, this does not appear to be public, so I was not able to see it. >> > If not will you be at Plumbers? >> >> Yes. > > Great, Tom, Dmitry, will you be at Plumbers? Sadly, I won't make plumbers this year. Cheers, Tom -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Mon, Jun 01, 2015 at 02:04:01PM +0200, Tomeu Vizoso wrote: > On 29 May 2015 at 15:23, Tomeu Vizoso wrote: > > On 29 May 2015 at 12:48, Tomeu Vizoso wrote: > >> On 31 March 2015 at 01:20, Dmitry Torokhov > >> wrote: > >>> Some devices take a long time when initializing, and not all drivers are > >>> suited to initialize their devices when they are open. For example, > >>> input drivers need to interrogate their devices in order to publish > >>> device's capabilities before userspace will open them. When such drivers > >>> are compiled into kernel they may stall entire kernel initialization. > >>> > >>> This change allows drivers request for their probe functions to be > >>> called asynchronously during driver and device registration (manual > >>> binding is still synchronous). Because async_schedule is used to perform > >>> asynchronous calls module loading will still wait for the probing to > >>> complete. > >> > >> But what about parents? Don't we need to make sure that before probing > >> a device its parent has finished probing already? > > > > Have realized that this is an existing problem that was just made more > > probable by async probe, as before async probing the parent could have > > deferred its probe and then its children would be probed. > > > > Wonder if drivers should be modified to defer its probe if their > > parent isn't probed yet, or if we can codify that in dd.c. > > Also wonder what's the plan regarding USB interfaces requiring that > their parent's lock is taken before probing. Yes, indeed, we need to take paren's lock in async probe too. I'll make a patch. Thanks for spotting this. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote: > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: > > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > > >> wrote: > > >> > Some devices take a long time when initializing, and not all drivers > > >> > are > > >> > suited to initialize their devices when they are open. For example, > > >> > input drivers need to interrogate their devices in order to publish > > >> > device's capabilities before userspace will open them. When such > > >> > drivers > > >> > are compiled into kernel they may stall entire kernel initialization. > > >> > > > >> > This change allows drivers request for their probe functions to be > > >> > called asynchronously during driver and device registration (manual > > >> > binding is still synchronous). Because async_schedule is used to > > >> > perform > > >> > asynchronous calls module loading will still wait for the probing to > > >> > complete. > > >> > > > >> > Note that the end goal is to make the probing asynchronous by default, > > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > > >> > measure that allows us to speed up boot process while we validating and > > >> > fixing the rest of the drivers and preparing userspace. > > >> > > > >> > This change is based on earlier patch by "Luis R. Rodriguez" > > >> > > > >> > > > >> > Signed-off-by: Dmitry Torokhov > > >> > --- > > >> > drivers/base/base.h| 1 + > > >> > drivers/base/bus.c | 31 +++--- > > >> > drivers/base/dd.c | 149 > > >> > ++--- > > >> > include/linux/device.h | 28 ++ > > >> > 4 files changed, 182 insertions(+), 27 deletions(-) > > >> > > >> Just noticed this patch. It caught my eye because I had a hard time > > >> getting an open coded implementation of asynchronous probing to work > > >> in the new libnvdimm subsystem. Especially the messy races of tearing > > >> things down while probing is still in flight. I ended up implementing > > >> asynchronous device registration which eliminated a lot of complexity > > >> and of course the bugs. In general I tend to think that async > > >> registration is less risky than async probe since it keeps wider > > >> portions of the traditional device model synchronous > > > > > > but its not see -DEFER_PROBE even before async probe. > > > > Except in that case you know probe has been seen by the driver at > > least once. So I see that as less of a surprise, but point taken. > > > > >> and leverages the > > >> fact that the device model is already well prepared for asynchronous > > >> arrival of devices due to hotplug. > > > > > > I think this sounds reasonable, do you have your code upstream or posted? > > > > Yes, see nd_device_register() in drivers/nvdimm/bus.c > > It should be I think rather easy for Dmitry to see if he can convert this > input > driver (not yet upstream) to this API and see if the same issues are fixed. No, I would rather not as it means we lose error handling on device registration. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > >> wrote: > >> > Some devices take a long time when initializing, and not all drivers are > >> > suited to initialize their devices when they are open. For example, > >> > input drivers need to interrogate their devices in order to publish > >> > device's capabilities before userspace will open them. When such drivers > >> > are compiled into kernel they may stall entire kernel initialization. > >> > > >> > This change allows drivers request for their probe functions to be > >> > called asynchronously during driver and device registration (manual > >> > binding is still synchronous). Because async_schedule is used to perform > >> > asynchronous calls module loading will still wait for the probing to > >> > complete. > >> > > >> > Note that the end goal is to make the probing asynchronous by default, > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > >> > measure that allows us to speed up boot process while we validating and > >> > fixing the rest of the drivers and preparing userspace. > >> > > >> > This change is based on earlier patch by "Luis R. Rodriguez" > >> > > >> > > >> > Signed-off-by: Dmitry Torokhov > >> > --- > >> > drivers/base/base.h| 1 + > >> > drivers/base/bus.c | 31 +++--- > >> > drivers/base/dd.c | 149 > >> > ++--- > >> > include/linux/device.h | 28 ++ > >> > 4 files changed, 182 insertions(+), 27 deletions(-) > >> > >> Just noticed this patch. It caught my eye because I had a hard time > >> getting an open coded implementation of asynchronous probing to work > >> in the new libnvdimm subsystem. Especially the messy races of tearing > >> things down while probing is still in flight. I ended up implementing > >> asynchronous device registration which eliminated a lot of complexity > >> and of course the bugs. In general I tend to think that async > >> registration is less risky than async probe since it keeps wider > >> portions of the traditional device model synchronous > > > > but its not see -DEFER_PROBE even before async probe. > > Except in that case you know probe has been seen by the driver at > least once. So I see that as less of a surprise, but point taken. > > >> and leverages the > >> fact that the device model is already well prepared for asynchronous > >> arrival of devices due to hotplug. > > > > I think this sounds reasonable, do you have your code upstream or posted? > > Yes, see nd_device_register() in drivers/nvdimm/bus.c So no error handling whatsoever, as expected... > > > If not will you be at Plumbers? > > Yes. Me too. > > > Maybe we shoudl talk about this as although > > ChromeOS already likely already jumped on async probe we should address a > > way forward and path forward for other distributions and I don't think > > anyone > > is looking too much into it. async probe came to Linux for two reasons: > > > > * chromeos wanting it > > * an incorrect systemd assumption on how the driver core works > > > > So long term we still need to address the systemd approach, are they going > > to be defaulting now to async probe for all modules? How about for > > built-ins? > > > > We should talk about this and maybe at plumbers. > > > >> Splitting the "initial probe" from > >> the "manual probe" case seems like a recipe for confusion. > > > > If you can come up with pros / cons on both strategies it'd be > > valuable. > > The problem I ran into was needing to remove devices that still had > yet to be probed and not being able to use registration completion vs > the device_lock() to effectively synchronize the sub-system. Why do you need to "synchronize the sub-system"? The asynchronous probing should be transparent to the driver. Just unregister the device (or the driver) and driver core will ensure that probe() is not in flight. Confused. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > wrote: > > Some devices take a long time when initializing, and not all drivers are > > suited to initialize their devices when they are open. For example, > > input drivers need to interrogate their devices in order to publish > > device's capabilities before userspace will open them. When such drivers > > are compiled into kernel they may stall entire kernel initialization. > > > > This change allows drivers request for their probe functions to be > > called asynchronously during driver and device registration (manual > > binding is still synchronous). Because async_schedule is used to perform > > asynchronous calls module loading will still wait for the probing to > > complete. > > > > Note that the end goal is to make the probing asynchronous by default, > > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > > measure that allows us to speed up boot process while we validating and > > fixing the rest of the drivers and preparing userspace. > > > > This change is based on earlier patch by "Luis R. Rodriguez" > > > > > > Signed-off-by: Dmitry Torokhov > > --- > > drivers/base/base.h| 1 + > > drivers/base/bus.c | 31 +++--- > > drivers/base/dd.c | 149 > > ++--- > > include/linux/device.h | 28 ++ > > 4 files changed, 182 insertions(+), 27 deletions(-) > > Just noticed this patch. It caught my eye because I had a hard time > getting an open coded implementation of asynchronous probing to work > in the new libnvdimm subsystem. Especially the messy races of tearing > things down while probing is still in flight. And that is exactly the reason why asynchronous probing was moved into driver code. It knows the state of the device and knows when it is OK to remove it or start suspend transitions, etc. > I ended up implementing > asynchronous device registration which eliminated a lot of complexity > and of course the bugs. serio and gameport subsystems have been using asynchronous registration for ages (not because of probing but because of issues with serio ports - such as Synaptics pass-through - stacked on top of each other and historicaly driver code deadlocking if you try to register child device on the same bus that parent is). However I was never too happy with it because with asynchronous registration you can't really handle errors. What do you do if device registartion fails? > In general I tend to think that async > registration is less risky than async probe since it keeps wider > portions of the traditional device model synchronous and leverages the > fact that the device model is already well prepared for asynchronous > arrival of devices due to hotplug. Splitting the "initial probe" from > the "manual probe" case seems like a recipe for confusion. The split is because again serio and USB subsystems resort to somewhat manual binding due to the need of handling recursion on the bus. But otherwise we already have asynchronous probing, because we have deferred probes and also driver modules can be loaded asynchronously with device registration. If you have concrete examples where asynchronous probig as it merged causes issues I'd love to hear and fix them. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote: > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > >> wrote: > >> > Some devices take a long time when initializing, and not all drivers are > >> > suited to initialize their devices when they are open. For example, > >> > input drivers need to interrogate their devices in order to publish > >> > device's capabilities before userspace will open them. When such drivers > >> > are compiled into kernel they may stall entire kernel initialization. > >> > > >> > This change allows drivers request for their probe functions to be > >> > called asynchronously during driver and device registration (manual > >> > binding is still synchronous). Because async_schedule is used to perform > >> > asynchronous calls module loading will still wait for the probing to > >> > complete. > >> > > >> > Note that the end goal is to make the probing asynchronous by default, > >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > >> > measure that allows us to speed up boot process while we validating and > >> > fixing the rest of the drivers and preparing userspace. > >> > > >> > This change is based on earlier patch by "Luis R. Rodriguez" > >> > > >> > > >> > Signed-off-by: Dmitry Torokhov > >> > --- > >> > drivers/base/base.h| 1 + > >> > drivers/base/bus.c | 31 +++--- > >> > drivers/base/dd.c | 149 > >> > ++--- > >> > include/linux/device.h | 28 ++ > >> > 4 files changed, 182 insertions(+), 27 deletions(-) > >> > >> Just noticed this patch. It caught my eye because I had a hard time > >> getting an open coded implementation of asynchronous probing to work > >> in the new libnvdimm subsystem. Especially the messy races of tearing > >> things down while probing is still in flight. I ended up implementing > >> asynchronous device registration which eliminated a lot of complexity > >> and of course the bugs. In general I tend to think that async > >> registration is less risky than async probe since it keeps wider > >> portions of the traditional device model synchronous > > > > but its not see -DEFER_PROBE even before async probe. > > Except in that case you know probe has been seen by the driver at > least once. So I see that as less of a surprise, but point taken. > > >> and leverages the > >> fact that the device model is already well prepared for asynchronous > >> arrival of devices due to hotplug. > > > > I think this sounds reasonable, do you have your code upstream or posted? > > Yes, see nd_device_register() in drivers/nvdimm/bus.c It should be I think rather easy for Dmitry to see if he can convert this input driver (not yet upstream) to this API and see if the same issues are fixed. This however does not address systemd's assumption over detachment of module load and probe. The inherent problem there was the timeout implemented and carried in systemd over the worker that uses modlib to load modules. Upon review the code was complex enough already and surely increasing the timeout helps but that doesn't address all issues with a general timeout in place. At SUSE we ended up not using a timeout for kmod built-in commands. That leaves the original timeout purpose in place. The code for async probe was not put in the kernel though but since its now upstream we should be able to replace that userspace systemd work around with async probe, but systemd folks would need to decide what they want to do. For full gory details of this refer to: https://bugzilla.novell.com/show_bug.cgi?id=889297 > > If not will you be at Plumbers? > > Yes. Great, Tom, Dmitry, will you be at Plumbers? > > Maybe we shoudl talk about this as although > > ChromeOS already likely already jumped on async probe we should address a > > way forward and path forward for other distributions and I don't think > > anyone > > is looking too much into it. async probe came to Linux for two reasons: > > > > * chromeos wanting it > > * an incorrect systemd assumption on how the driver core works > > > > So long term we still need to address the systemd approach, are they going > > to be defaulting now to async probe for all modules? How about for > > built-ins? > > > > We should talk about this and maybe at plumbers. > > > >> Splitting the "initial probe" from > >> the "manual probe" case seems like a recipe for confusion. > > > > If you can come up with pros / cons on both strategies it'd be > > valuable. > > The problem I ran into was needing to remove devices that still had > yet to be probed and not being able to use registration completion vs > the device_lock() to effectively synchronize the sub-system. Interesting, what cases would this happen under? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a me
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez wrote: > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov >> wrote: >> > Some devices take a long time when initializing, and not all drivers are >> > suited to initialize their devices when they are open. For example, >> > input drivers need to interrogate their devices in order to publish >> > device's capabilities before userspace will open them. When such drivers >> > are compiled into kernel they may stall entire kernel initialization. >> > >> > This change allows drivers request for their probe functions to be >> > called asynchronously during driver and device registration (manual >> > binding is still synchronous). Because async_schedule is used to perform >> > asynchronous calls module loading will still wait for the probing to >> > complete. >> > >> > Note that the end goal is to make the probing asynchronous by default, >> > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary >> > measure that allows us to speed up boot process while we validating and >> > fixing the rest of the drivers and preparing userspace. >> > >> > This change is based on earlier patch by "Luis R. Rodriguez" >> > >> > >> > Signed-off-by: Dmitry Torokhov >> > --- >> > drivers/base/base.h| 1 + >> > drivers/base/bus.c | 31 +++--- >> > drivers/base/dd.c | 149 >> > ++--- >> > include/linux/device.h | 28 ++ >> > 4 files changed, 182 insertions(+), 27 deletions(-) >> >> Just noticed this patch. It caught my eye because I had a hard time >> getting an open coded implementation of asynchronous probing to work >> in the new libnvdimm subsystem. Especially the messy races of tearing >> things down while probing is still in flight. I ended up implementing >> asynchronous device registration which eliminated a lot of complexity >> and of course the bugs. In general I tend to think that async >> registration is less risky than async probe since it keeps wider >> portions of the traditional device model synchronous > > but its not see -DEFER_PROBE even before async probe. Except in that case you know probe has been seen by the driver at least once. So I see that as less of a surprise, but point taken. >> and leverages the >> fact that the device model is already well prepared for asynchronous >> arrival of devices due to hotplug. > > I think this sounds reasonable, do you have your code upstream or posted? Yes, see nd_device_register() in drivers/nvdimm/bus.c > If not will you be at Plumbers? Yes. > Maybe we shoudl talk about this as although > ChromeOS already likely already jumped on async probe we should address a > way forward and path forward for other distributions and I don't think anyone > is looking too much into it. async probe came to Linux for two reasons: > > * chromeos wanting it > * an incorrect systemd assumption on how the driver core works > > So long term we still need to address the systemd approach, are they going > to be defaulting now to async probe for all modules? How about for built-ins? > > We should talk about this and maybe at plumbers. > >> Splitting the "initial probe" from >> the "manual probe" case seems like a recipe for confusion. > > If you can come up with pros / cons on both strategies it'd be > valuable. The problem I ran into was needing to remove devices that still had yet to be probed and not being able to use registration completion vs the device_lock() to effectively synchronize the sub-system. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote: > On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov > wrote: > > Some devices take a long time when initializing, and not all drivers are > > suited to initialize their devices when they are open. For example, > > input drivers need to interrogate their devices in order to publish > > device's capabilities before userspace will open them. When such drivers > > are compiled into kernel they may stall entire kernel initialization. > > > > This change allows drivers request for their probe functions to be > > called asynchronously during driver and device registration (manual > > binding is still synchronous). Because async_schedule is used to perform > > asynchronous calls module loading will still wait for the probing to > > complete. > > > > Note that the end goal is to make the probing asynchronous by default, > > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > > measure that allows us to speed up boot process while we validating and > > fixing the rest of the drivers and preparing userspace. > > > > This change is based on earlier patch by "Luis R. Rodriguez" > > > > > > Signed-off-by: Dmitry Torokhov > > --- > > drivers/base/base.h| 1 + > > drivers/base/bus.c | 31 +++--- > > drivers/base/dd.c | 149 > > ++--- > > include/linux/device.h | 28 ++ > > 4 files changed, 182 insertions(+), 27 deletions(-) > > Just noticed this patch. It caught my eye because I had a hard time > getting an open coded implementation of asynchronous probing to work > in the new libnvdimm subsystem. Especially the messy races of tearing > things down while probing is still in flight. I ended up implementing > asynchronous device registration which eliminated a lot of complexity > and of course the bugs. In general I tend to think that async > registration is less risky than async probe since it keeps wider > portions of the traditional device model synchronous but its not see -DEFER_PROBE even before async probe. > and leverages the > fact that the device model is already well prepared for asynchronous > arrival of devices due to hotplug. I think this sounds reasonable, do you have your code upstream or posted? If not will you be at Plumbers? Maybe we shoudl talk about this as although ChromeOS already likely already jumped on async probe we should address a way forward and path forward for other distributions and I don't think anyone is looking too much into it. async probe came to Linux for two reasons: * chromeos wanting it * an incorrect systemd assumption on how the driver core works So long term we still need to address the systemd approach, are they going to be defaulting now to async probe for all modules? How about for built-ins? We should talk about this and maybe at plumbers. > Splitting the "initial probe" from > the "manual probe" case seems like a recipe for confusion. If you can come up with pros / cons on both strategies it'd be valuable. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov wrote: > Some devices take a long time when initializing, and not all drivers are > suited to initialize their devices when they are open. For example, > input drivers need to interrogate their devices in order to publish > device's capabilities before userspace will open them. When such drivers > are compiled into kernel they may stall entire kernel initialization. > > This change allows drivers request for their probe functions to be > called asynchronously during driver and device registration (manual > binding is still synchronous). Because async_schedule is used to perform > asynchronous calls module loading will still wait for the probing to > complete. > > Note that the end goal is to make the probing asynchronous by default, > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > measure that allows us to speed up boot process while we validating and > fixing the rest of the drivers and preparing userspace. > > This change is based on earlier patch by "Luis R. Rodriguez" > > > Signed-off-by: Dmitry Torokhov > --- > drivers/base/base.h| 1 + > drivers/base/bus.c | 31 +++--- > drivers/base/dd.c | 149 > ++--- > include/linux/device.h | 28 ++ > 4 files changed, 182 insertions(+), 27 deletions(-) Just noticed this patch. It caught my eye because I had a hard time getting an open coded implementation of asynchronous probing to work in the new libnvdimm subsystem. Especially the messy races of tearing things down while probing is still in flight. I ended up implementing asynchronous device registration which eliminated a lot of complexity and of course the bugs. In general I tend to think that async registration is less risky than async probe since it keeps wider portions of the traditional device model synchronous and leverages the fact that the device model is already well prepared for asynchronous arrival of devices due to hotplug. Splitting the "initial probe" from the "manual probe" case seems like a recipe for confusion. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On 29 May 2015 at 15:23, Tomeu Vizoso wrote:
> On 29 May 2015 at 12:48, Tomeu Vizoso wrote:
>> On 31 March 2015 at 01:20, Dmitry Torokhov wrote:
>>> Some devices take a long time when initializing, and not all drivers are
>>> suited to initialize their devices when they are open. For example,
>>> input drivers need to interrogate their devices in order to publish
>>> device's capabilities before userspace will open them. When such drivers
>>> are compiled into kernel they may stall entire kernel initialization.
>>>
>>> This change allows drivers request for their probe functions to be
>>> called asynchronously during driver and device registration (manual
>>> binding is still synchronous). Because async_schedule is used to perform
>>> asynchronous calls module loading will still wait for the probing to
>>> complete.
>>
>> But what about parents? Don't we need to make sure that before probing
>> a device its parent has finished probing already?
>
> Have realized that this is an existing problem that was just made more
> probable by async probe, as before async probing the parent could have
> deferred its probe and then its children would be probed.
>
> Wonder if drivers should be modified to defer its probe if their
> parent isn't probed yet, or if we can codify that in dd.c.
Also wonder what's the plan regarding USB interfaces requiring that
their parent's lock is taken before probing.
Thanks,
Tomeu
> Regards,
>
> Tomeu
>
>> This backtrace
>> illustrates the problem:
>>
>> [] (__dabt_svc) from [] (host1x_syncpt_alloc+0x14/0x134)
>> [] (host1x_syncpt_alloc) from []
>> (host1x_syncpt_request+0x28/0x2c)
>> [] (host1x_syncpt_request) from []
>> (tegra_dc_probe+0x198/0x35c)
>> [] (tegra_dc_probe) from []
>> (platform_drv_probe+0x54/0xbc)
>> [] (platform_drv_probe) from []
>> (driver_probe_device+0x184/0x2c4)
>> [] (driver_probe_device) from []
>> (__driver_attach+0x9c/0xa0)
>> [] (__driver_attach) from [] (bus_for_each_dev+0x78/0xac)
>> [] (bus_for_each_dev) from [] (driver_attach+0x2c/0x30)
>> [] (driver_attach) from []
>> (driver_attach_async+0x18/0x1c)
>> [] (driver_attach_async) from []
>> (async_run_entry_fn+0x58/0x128)
>> [] (async_run_entry_fn) from []
>> (process_one_work+0x140/0x50c)
>> [] (process_one_work) from [] (worker_thread+0x54/0x52c)
>> [] (worker_thread) from [] (kthread+0xec/0x104)
>> [] (kthread) from [] (ret_from_fork+0x14/0x2c)
>>
>> host1x_syncpt_request() assumes that the parent of the DC (host1x) has
>> been probed already and its drvdata is available.
>>
>> Thanks,
>>
>> Tomeu
>>
>>> Note that the end goal is to make the probing asynchronous by default,
>>> so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary
>>> measure that allows us to speed up boot process while we validating and
>>> fixing the rest of the drivers and preparing userspace.
>>>
>>> This change is based on earlier patch by "Luis R. Rodriguez"
>>>
>>>
>>> Signed-off-by: Dmitry Torokhov
>>> ---
>>> drivers/base/base.h| 1 +
>>> drivers/base/bus.c | 31 +++---
>>> drivers/base/dd.c | 149
>>> ++---
>>> include/linux/device.h | 28 ++
>>> 4 files changed, 182 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/base/base.h b/drivers/base/base.h
>>> index 251c5d3..fd3347d 100644
>>> --- a/drivers/base/base.h
>>> +++ b/drivers/base/base.h
>>> @@ -116,6 +116,7 @@ static inline int driver_match_device(struct
>>> device_driver *drv,
>>> {
>>> return drv->bus->match ? drv->bus->match(dev, drv) : 1;
>>> }
>>> +extern bool driver_allows_async_probing(struct device_driver *drv);
>>>
>>> extern int driver_add_groups(struct device_driver *drv,
>>> const struct attribute_group **groups);
>>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>>> index 79bc203..5005924 100644
>>> --- a/drivers/base/bus.c
>>> +++ b/drivers/base/bus.c
>>> @@ -10,6 +10,7 @@
>>> *
>>> */
>>>
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -549,15 +550,12 @@ void bus_probe_device(struct device *dev)
>>> {
>>> struct bus_type *bus = dev->bus;
>>> struct subsys_interface *sif;
>>> - int ret;
>>>
>>> if (!bus)
>>> return;
>>>
>>> - if (bus->p->drivers_autoprobe) {
>>> - ret = device_attach(dev);
>>> - WARN_ON(ret < 0);
>>> - }
>>> + if (bus->p->drivers_autoprobe)
>>> + device_initial_probe(dev);
>>>
>>> mutex_lock(&bus->p->mutex);
>>> list_for_each_entry(sif, &bus->p->interfaces, node)
>>> @@ -659,6 +657,17 @@ static ssize_t uevent_store(struct device_driver *drv,
>>> const char *buf,
>>> }
>>> static DRIVER_ATTR_WO(uevent);
>>>
>>> +static void driver_attach_async(void *_drv, async_cookie_t cookie)
>>> +{
>>> + struct device_driver *drv = _drv;
>>> + int ret;
>>> +
>>> + ret = driver_attach(drv);
>>> +
>>> + pr_debug("bus: '%s': driver %s
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On 29 May 2015 at 12:48, Tomeu Vizoso wrote:
> On 31 March 2015 at 01:20, Dmitry Torokhov wrote:
>> Some devices take a long time when initializing, and not all drivers are
>> suited to initialize their devices when they are open. For example,
>> input drivers need to interrogate their devices in order to publish
>> device's capabilities before userspace will open them. When such drivers
>> are compiled into kernel they may stall entire kernel initialization.
>>
>> This change allows drivers request for their probe functions to be
>> called asynchronously during driver and device registration (manual
>> binding is still synchronous). Because async_schedule is used to perform
>> asynchronous calls module loading will still wait for the probing to
>> complete.
>
> But what about parents? Don't we need to make sure that before probing
> a device its parent has finished probing already?
Have realized that this is an existing problem that was just made more
probable by async probe, as before async probing the parent could have
deferred its probe and then its children would be probed.
Wonder if drivers should be modified to defer its probe if their
parent isn't probed yet, or if we can codify that in dd.c.
Regards,
Tomeu
> This backtrace
> illustrates the problem:
>
> [] (__dabt_svc) from [] (host1x_syncpt_alloc+0x14/0x134)
> [] (host1x_syncpt_alloc) from []
> (host1x_syncpt_request+0x28/0x2c)
> [] (host1x_syncpt_request) from []
> (tegra_dc_probe+0x198/0x35c)
> [] (tegra_dc_probe) from [] (platform_drv_probe+0x54/0xbc)
> [] (platform_drv_probe) from []
> (driver_probe_device+0x184/0x2c4)
> [] (driver_probe_device) from []
> (__driver_attach+0x9c/0xa0)
> [] (__driver_attach) from [] (bus_for_each_dev+0x78/0xac)
> [] (bus_for_each_dev) from [] (driver_attach+0x2c/0x30)
> [] (driver_attach) from [] (driver_attach_async+0x18/0x1c)
> [] (driver_attach_async) from []
> (async_run_entry_fn+0x58/0x128)
> [] (async_run_entry_fn) from []
> (process_one_work+0x140/0x50c)
> [] (process_one_work) from [] (worker_thread+0x54/0x52c)
> [] (worker_thread) from [] (kthread+0xec/0x104)
> [] (kthread) from [] (ret_from_fork+0x14/0x2c)
>
> host1x_syncpt_request() assumes that the parent of the DC (host1x) has
> been probed already and its drvdata is available.
>
> Thanks,
>
> Tomeu
>
>> Note that the end goal is to make the probing asynchronous by default,
>> so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary
>> measure that allows us to speed up boot process while we validating and
>> fixing the rest of the drivers and preparing userspace.
>>
>> This change is based on earlier patch by "Luis R. Rodriguez"
>>
>>
>> Signed-off-by: Dmitry Torokhov
>> ---
>> drivers/base/base.h| 1 +
>> drivers/base/bus.c | 31 +++---
>> drivers/base/dd.c | 149
>> ++---
>> include/linux/device.h | 28 ++
>> 4 files changed, 182 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/base/base.h b/drivers/base/base.h
>> index 251c5d3..fd3347d 100644
>> --- a/drivers/base/base.h
>> +++ b/drivers/base/base.h
>> @@ -116,6 +116,7 @@ static inline int driver_match_device(struct
>> device_driver *drv,
>> {
>> return drv->bus->match ? drv->bus->match(dev, drv) : 1;
>> }
>> +extern bool driver_allows_async_probing(struct device_driver *drv);
>>
>> extern int driver_add_groups(struct device_driver *drv,
>> const struct attribute_group **groups);
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index 79bc203..5005924 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -10,6 +10,7 @@
>> *
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -549,15 +550,12 @@ void bus_probe_device(struct device *dev)
>> {
>> struct bus_type *bus = dev->bus;
>> struct subsys_interface *sif;
>> - int ret;
>>
>> if (!bus)
>> return;
>>
>> - if (bus->p->drivers_autoprobe) {
>> - ret = device_attach(dev);
>> - WARN_ON(ret < 0);
>> - }
>> + if (bus->p->drivers_autoprobe)
>> + device_initial_probe(dev);
>>
>> mutex_lock(&bus->p->mutex);
>> list_for_each_entry(sif, &bus->p->interfaces, node)
>> @@ -659,6 +657,17 @@ static ssize_t uevent_store(struct device_driver *drv,
>> const char *buf,
>> }
>> static DRIVER_ATTR_WO(uevent);
>>
>> +static void driver_attach_async(void *_drv, async_cookie_t cookie)
>> +{
>> + struct device_driver *drv = _drv;
>> + int ret;
>> +
>> + ret = driver_attach(drv);
>> +
>> + pr_debug("bus: '%s': driver %s async attach completed: %d\n",
>> +drv->bus->name, drv->name, ret);
>> +}
>> +
>> /**
>> * bus_add_driver - Add a driver to the bus.
>> * @drv: driver.
>> @@ -691,9 +700,15 @@ int bus_add_driver(struct device_driver *drv)
>>
>> klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
>>
Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers
On 31 March 2015 at 01:20, Dmitry Torokhov wrote:
> Some devices take a long time when initializing, and not all drivers are
> suited to initialize their devices when they are open. For example,
> input drivers need to interrogate their devices in order to publish
> device's capabilities before userspace will open them. When such drivers
> are compiled into kernel they may stall entire kernel initialization.
>
> This change allows drivers request for their probe functions to be
> called asynchronously during driver and device registration (manual
> binding is still synchronous). Because async_schedule is used to perform
> asynchronous calls module loading will still wait for the probing to
> complete.
But what about parents? Don't we need to make sure that before probing
a device its parent has finished probing already? This backtrace
illustrates the problem:
[] (__dabt_svc) from [] (host1x_syncpt_alloc+0x14/0x134)
[] (host1x_syncpt_alloc) from []
(host1x_syncpt_request+0x28/0x2c)
[] (host1x_syncpt_request) from []
(tegra_dc_probe+0x198/0x35c)
[] (tegra_dc_probe) from [] (platform_drv_probe+0x54/0xbc)
[] (platform_drv_probe) from []
(driver_probe_device+0x184/0x2c4)
[] (driver_probe_device) from [] (__driver_attach+0x9c/0xa0)
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xac)
[] (bus_for_each_dev) from [] (driver_attach+0x2c/0x30)
[] (driver_attach) from [] (driver_attach_async+0x18/0x1c)
[] (driver_attach_async) from []
(async_run_entry_fn+0x58/0x128)
[] (async_run_entry_fn) from []
(process_one_work+0x140/0x50c)
[] (process_one_work) from [] (worker_thread+0x54/0x52c)
[] (worker_thread) from [] (kthread+0xec/0x104)
[] (kthread) from [] (ret_from_fork+0x14/0x2c)
host1x_syncpt_request() assumes that the parent of the DC (host1x) has
been probed already and its drvdata is available.
Thanks,
Tomeu
> Note that the end goal is to make the probing asynchronous by default,
> so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary
> measure that allows us to speed up boot process while we validating and
> fixing the rest of the drivers and preparing userspace.
>
> This change is based on earlier patch by "Luis R. Rodriguez"
>
>
> Signed-off-by: Dmitry Torokhov
> ---
> drivers/base/base.h| 1 +
> drivers/base/bus.c | 31 +++---
> drivers/base/dd.c | 149
> ++---
> include/linux/device.h | 28 ++
> 4 files changed, 182 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 251c5d3..fd3347d 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -116,6 +116,7 @@ static inline int driver_match_device(struct
> device_driver *drv,
> {
> return drv->bus->match ? drv->bus->match(dev, drv) : 1;
> }
> +extern bool driver_allows_async_probing(struct device_driver *drv);
>
> extern int driver_add_groups(struct device_driver *drv,
> const struct attribute_group **groups);
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 79bc203..5005924 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -10,6 +10,7 @@
> *
> */
>
> +#include
> #include
> #include
> #include
> @@ -549,15 +550,12 @@ void bus_probe_device(struct device *dev)
> {
> struct bus_type *bus = dev->bus;
> struct subsys_interface *sif;
> - int ret;
>
> if (!bus)
> return;
>
> - if (bus->p->drivers_autoprobe) {
> - ret = device_attach(dev);
> - WARN_ON(ret < 0);
> - }
> + if (bus->p->drivers_autoprobe)
> + device_initial_probe(dev);
>
> mutex_lock(&bus->p->mutex);
> list_for_each_entry(sif, &bus->p->interfaces, node)
> @@ -659,6 +657,17 @@ static ssize_t uevent_store(struct device_driver *drv,
> const char *buf,
> }
> static DRIVER_ATTR_WO(uevent);
>
> +static void driver_attach_async(void *_drv, async_cookie_t cookie)
> +{
> + struct device_driver *drv = _drv;
> + int ret;
> +
> + ret = driver_attach(drv);
> +
> + pr_debug("bus: '%s': driver %s async attach completed: %d\n",
> +drv->bus->name, drv->name, ret);
> +}
> +
> /**
> * bus_add_driver - Add a driver to the bus.
> * @drv: driver.
> @@ -691,9 +700,15 @@ int bus_add_driver(struct device_driver *drv)
>
> klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
> if (drv->bus->p->drivers_autoprobe) {
> - error = driver_attach(drv);
> - if (error)
> - goto out_unregister;
> + if (driver_allows_async_probing(drv)) {
> + pr_debug("bus: '%s': probing driver %s
> asynchronously\n",
> + drv->bus->name, drv->name);
> + async_schedule(driver_attach_async, drv);
> + } else {
> + error = driver_attach(drv);
> + if
