Re: [RFC][PATCH v2 20/31] timers: usb: Use del_timer_shutdown() before freeing timer

From: Alan Stern
Date: Thu Oct 27 2022 - 16:38:26 EST


On Thu, Oct 27, 2022 at 11:05:45AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Before a timer is freed, del_timer_shutdown() must be called.

Is this supposed to be true for all timers? Because the USB subsystem
contains an awful lot more timers than just the two you touched in this
patch.

Alan Stern

> Link: https://lore.kernel.org/all/20220407161745.7d6754b3@xxxxxxxxxxxxxxxxxx/
>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxxxxxx>
> Cc: Johan Hovold <johan@xxxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Cc: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> Cc: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> Cc: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> Cc: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> Cc: Bhuvanesh Surachari <Bhuvanesh_Surachari@xxxxxxxxxx>
> Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 3 +++
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/serial/garmin_gps.c | 2 +-
> drivers/usb/serial/mos7840.c | 2 +-
> 4 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bbab424b0d55..397f263ab7da 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1261,6 +1261,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
>
> /* Don't do a long sleep inside a workqueue routine */
> if (type == HUB_INIT2) {
> + /* Timers must be shutdown before they are re-initialized */
> + if (hub->init_work.work.func)
> + del_timer_shutdown(&hub->init_work.timer);
> INIT_DELAYED_WORK(&hub->init_work, hub_init_func3);
> queue_delayed_work(system_power_efficient_wq,
> &hub->init_work,
> diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
> index 931e6362a13d..a6e2f8358adf 100644
> --- a/drivers/usb/gadget/udc/m66592-udc.c
> +++ b/drivers/usb/gadget/udc/m66592-udc.c
> @@ -1519,7 +1519,7 @@ static int m66592_remove(struct platform_device *pdev)
>
> usb_del_gadget_udc(&m66592->gadget);
>
> - del_timer_sync(&m66592->timer);
> + del_timer_shutdown(&m66592->timer);
> iounmap(m66592->reg);
> free_irq(platform_get_irq(pdev, 0), m66592);
> m66592_free_request(&m66592->ep[0].ep, m66592->ep0_req);