Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 27 Oct 2022 16:38:19 -0400
From:   Alan Stern <stern@rowland.harvard.edu>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Stephen Boyd <sboyd@kernel.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Felipe Balbi <balbi@kernel.org>,
        Johan Hovold <johan@kernel.org>,
        Mathias Nyman <mathias.nyman@linux.intel.com>,
        Kai-Heng Feng <kai.heng.feng@canonical.com>,
        Matthias Kaehlcke <mka@chromium.org>,
        Michael Grzeschik <m.grzeschik@pengutronix.de>,
        Bhuvanesh Surachari <Bhuvanesh_Surachari@mentor.com>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        linux-usb@vger.kernel.org
Subject: Re: [RFC][PATCH v2 20/31] timers: usb: Use del_timer_shutdown()
 before freeing timer
Message-ID: <Y1rsO2GeGReS0BvU@rowland.harvard.edu>
References: <20221027150525.753064657@goodmis.org>
 <20221027150928.983388020@goodmis.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221027150928.983388020@goodmis.org>
Precedence: bulk

On Thu, Oct 27, 2022 at 11:05:45AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> 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@gandalf.local.home/
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Cc: Bhuvanesh Surachari <Bhuvanesh_Surachari@mentor.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  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);