Re: [PATCH V1 1/1] usb: hub: introduce delay after usb hub mutex release

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Dec 01, 2021 at 06:28:08AM +0530, Sanjeev Chugh wrote:
> Rogue usb sticks can cause endless port connect change events
> due to unstable electric connection between usb chip and
> the port. Port connect change will cause device enumeration for
> the new device connection and new device processing is done with
> the usb hub mutex acquired. This can cause very short time gap
> between unlock/lock of hub device mutex. So if some thread is
> running at low priority than hub_thread, it can face hub device
> mutex starvation.
> 
> This commit introduces a sleep of 25ms after the hub device mutex is
> unlocked in hub_events so that if hub_thread is stuck in a
> endless loop, all other threads will get fair amount of chance
> to acquire the usb hub mutex.
> 
> Signed-off-by: Sanjeev Chugh <sanjeev_chugh@mentor.com>
> ---
>  drivers/usb/core/hub.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 00070a8..0be2acc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5763,6 +5763,17 @@ static void hub_event(struct work_struct *work)
>  out_hdev_lock:
>  	usb_unlock_device(hdev);
>  
> +	/*
> +	 * Rogue usb sticks can cause endless device connection
> +	 * events due to unstable electric connection.

What is a "usb stick"?  Any usb device could cause this, right?

> This
> +	 * can cause very short time gap between unlock/lock
> +	 * of hub device mutex thus causing mutex starvation
> +	 * for some other lower priority thread. Thus sleep
> +	 * would give fair chance to all other threads to
> +	 * acquire the usb hub mutex.
> +	 */
> +	msleep(25);

What normal code path did you just slow down here?  Why not slow down
the disconnect path instead of the connect path?

thanks,

greg k-h