| Message ID | 20190410203520.248158-1-rrangel@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb/hcd: Send a uevent signaling that the host controller has died | expand |
On Wed, 10 Apr 2019, Raul E Rangel wrote: > This change will send a CHANGE event to udev with the DEAD environment > variable set when the HC dies. I chose this instead of any of the other > udev events because it's representing a state change in the host > controller. The only other event that might have fit was OFFLINE, but > that seems to be used for hot-removal. > > By notifying user space the appropriate policies can be applied. > e.g., > * Collect error logs. > * Notify the user that USB is no longer functional. > * Perform a graceful reboot. > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> One or two problems... See below. > --- > > drivers/usb/core/hcd.c | 25 +++++++++++++++++++++++++ > include/linux/usb/hcd.h | 1 + > 2 files changed, 26 insertions(+) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 975d7c1288e3..b38ad9ce068b 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -2343,6 +2343,22 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg) > return status; > } > > + > +/** > + * hcd_died_work - Workqueue routine for root-hub has died. > + * @hcd: primary host controller for this root hub. > + * > + * Do not call with the shared_hcd. > + * */ > +static void hcd_died_work(struct work_struct *work) > +{ > + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); > + > + /* Notify user space that the host controller has died */ > + kobject_uevent_env(&hcd->self.root_hub->dev.kobj, KOBJ_CHANGE, > + (char *[]){ "DEAD=1", NULL }); How do you know that the root hub hasn't already been deallocated? > +} > + > /* Workqueue routine for root-hub remote wakeup */ > static void hcd_resume_work(struct work_struct *work) > { > @@ -2488,6 +2504,13 @@ void usb_hc_died (struct usb_hcd *hcd) > usb_kick_hub_wq(hcd->self.root_hub); > } > } > + > + /* Handle the case where this function gets called with a shared HCD */ > + if (usb_hcd_is_primary_hcd(hcd)) > + schedule_work(&hcd->died_work); > + else > + schedule_work(&hcd->primary_hcd->died_work); > + > spin_unlock_irqrestore (&hcd_root_hub_lock, flags); > /* Make sure that the other roothub is also deallocated. */ > } > @@ -2555,6 +2578,8 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver, > INIT_WORK(&hcd->wakeup_work, hcd_resume_work); > #endif > > + INIT_WORK(&hcd->died_work, hcd_died_work); > + > hcd->driver = driver; > hcd->speed = driver->flags & HCD_MASK; > hcd->product_desc = (driver->product_desc) ? driver->product_desc : You forgot to ensure that this work entry won't still be pending when the hcd structure is deallocated. Alan Stern > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 695931b03684..ae51d5bd1dfc 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -98,6 +98,7 @@ struct usb_hcd { > #ifdef CONFIG_PM > struct work_struct wakeup_work; /* for remote wakeup */ > #endif > + struct work_struct died_work; /* for dying */ > > /* > * hardware info/state >
On Mi, 2019-04-10 at 14:35 -0600, Raul E Rangel wrote: > This change will send a CHANGE event to udev with the DEAD environment > variable set when the HC dies. I chose this instead of any of the other > udev events because it's representing a state change in the host > controller. The only other event that might have fit was OFFLINE, but > that seems to be used for hot-removal. > > By notifying user space the appropriate policies can be applied. > e.g., > * Collect error logs. > * Notify the user that USB is no longer functional. > * Perform a graceful reboot. Could you please make sure this type of event is shared with other subsystems whose devices can "die"? It looks to me like SCSI offline should for example create the same event. This kind of thing needs to be documented. Regards Oliver
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 975d7c1288e3..b38ad9ce068b 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2343,6 +2343,22 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg) return status; } + +/** + * hcd_died_work - Workqueue routine for root-hub has died. + * @hcd: primary host controller for this root hub. + * + * Do not call with the shared_hcd. + * */ +static void hcd_died_work(struct work_struct *work) +{ + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); + + /* Notify user space that the host controller has died */ + kobject_uevent_env(&hcd->self.root_hub->dev.kobj, KOBJ_CHANGE, + (char *[]){ "DEAD=1", NULL }); +} + /* Workqueue routine for root-hub remote wakeup */ static void hcd_resume_work(struct work_struct *work) { @@ -2488,6 +2504,13 @@ void usb_hc_died (struct usb_hcd *hcd) usb_kick_hub_wq(hcd->self.root_hub); } } + + /* Handle the case where this function gets called with a shared HCD */ + if (usb_hcd_is_primary_hcd(hcd)) + schedule_work(&hcd->died_work); + else + schedule_work(&hcd->primary_hcd->died_work); + spin_unlock_irqrestore (&hcd_root_hub_lock, flags); /* Make sure that the other roothub is also deallocated. */ } @@ -2555,6 +2578,8 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver, INIT_WORK(&hcd->wakeup_work, hcd_resume_work); #endif + INIT_WORK(&hcd->died_work, hcd_died_work); + hcd->driver = driver; hcd->speed = driver->flags & HCD_MASK; hcd->product_desc = (driver->product_desc) ? driver->product_desc : diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 695931b03684..ae51d5bd1dfc 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -98,6 +98,7 @@ struct usb_hcd { #ifdef CONFIG_PM struct work_struct wakeup_work; /* for remote wakeup */ #endif + struct work_struct died_work; /* for dying */ /* * hardware info/state
This change will send a CHANGE event to udev with the DEAD environment variable set when the HC dies. I chose this instead of any of the other udev events because it's representing a state change in the host controller. The only other event that might have fit was OFFLINE, but that seems to be used for hot-removal. By notifying user space the appropriate policies can be applied. e.g., * Collect error logs. * Notify the user that USB is no longer functional. * Perform a graceful reboot. Signed-off-by: Raul E Rangel <rrangel@chromium.org> --- drivers/usb/core/hcd.c | 25 +++++++++++++++++++++++++ include/linux/usb/hcd.h | 1 + 2 files changed, 26 insertions(+)