On Tue, 11 May 2021 at 22:37:55 -0400, Ashton Fagg wrote: > Inspiration for this diff was drawn from Joshua Stein [1], seeminly he > had a use-case that was somewhat similar to mine at one point but it > doesn't look like this was ever committed. Nor can I find any discussion > on it.
I'm glad I could inspire you to repost the work I already did years ago. On Tue, 11 May 2021 at 20:53:17 -0600, Theo de Raadt wrote: > Unfortunately there is no instrumentation in drivers or subsystems to > capture "ready to do work", and create the events at that point instead. > hotplug is hooked into subr_autoconf.c, this code is for handling the > device heirarchy, but it is not involved in operation, and thus unaware > of "readiness". Fixing this would require a subsystem by subsystem study, > figuring out where the glue should exist, and creating the events at > the CORRECT time. When this same objection was raised in 2018 when I proposed this, I said that I looked through the bugs archives and could only find references to hotplug-related issues 8 years prior, and they were considered solved: https://marc.info/?l=openbsd-bugs&m=127990199813627&w=2 I also proposed this diff, which individual drivers could use to defer hotplug notification from their attach routine if they needed to do extra work before becoming "ready". No one spoke up of any drivers needing such functionality, and all of this work was ignored since you and Mark wanted the Xorg input device notifications to come through wscons instead of hotplug. But either way, if a driver is causing a panic because it responds before it is ready, the same thing would happen without hotplug if it was communicated with at the wrong time (like some script running in a loop). Those drivers should just not respond to ioctls or whatever they are doing before they are ready rather than just hoping that no one sees their attachment too early. diff --git sys/dev/hotplug.c sys/dev/hotplug.c index e1e7bad95c9..434c43f4135 100644 --- sys/dev/hotplug.c +++ sys/dev/hotplug.c @@ -73,6 +73,12 @@ hotplug_device_attach(enum devclass class, char *name) hotplug_put_event(&he); } +void +hotplug_defer_attach(struct device *dev) +{ + dev->dv_flags |= DVF_HOTPLUG_DEFER; +} + void hotplug_device_detach(enum devclass class, char *name) { diff --git sys/kern/subr_autoconf.c sys/kern/subr_autoconf.c index 13fafc6994e..e09f11486c0 100644 --- sys/kern/subr_autoconf.c +++ sys/kern/subr_autoconf.c @@ -403,7 +403,7 @@ config_attach(struct device *parent, void *match, void *aux, cfprint_t print) (*ca->ca_attach)(parent, dev, aux); config_process_deferred_children(dev); #if NHOTPLUG > 0 - if (!cold) + if (!cold && !(dev->dv_flags & DVF_HOTPLUG_DEFER)) hotplug_device_attach(cd->cd_class, dev->dv_xname); #endif diff --git sys/sys/device.h sys/sys/device.h index 90827f53503..b73b248d040 100644 --- sys/sys/device.h +++ sys/sys/device.h @@ -81,7 +81,8 @@ struct device { }; /* dv_flags */ -#define DVF_ACTIVE 0x0001 /* device is activated */ +#define DVF_ACTIVE 0x0001 /* device is activated */ +#define DVF_HOTPLUG_DEFER 0x0002 /* defer hotplug announce */ TAILQ_HEAD(devicelist, device); diff --git sys/sys/hotplug.h sys/sys/hotplug.h index f2da3f6cfd8..7e68482a428 100644 --- sys/sys/hotplug.h +++ sys/sys/hotplug.h @@ -35,6 +35,7 @@ struct hotplug_event { #ifdef _KERNEL void hotplug_device_attach(enum devclass, char *); void hotplug_device_detach(enum devclass, char *); +void hotplug_defer_attach(struct device *); #endif #endif /* _SYS_HOTPLUG_H_ */
