linux-kernel - Re: Runtime PM workqueue killing system performance with USB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140522173951.GD14641@arm.com>
Date:	Thu, 22 May 2014 18:39:51 +0100
From:	Will Deacon <will.deacon@....com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	"sarah.a.sharp@...ux.intel.com" <sarah.a.sharp@...ux.intel.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"khilman@...aro.org" <khilman@...aro.org>
Subject: Re: Runtime PM workqueue killing system performance with USB

Hi Alan,

On Thu, May 22, 2014 at 04:02:06PM +0100, Alan Stern wrote:
> On Thu, 22 May 2014, Will Deacon wrote:
> > Consequently, I see a kworker thread on each CPU consuming a significant
> > amount of the system resources. Worse, if I enable something like kmemleak
> > (which adds more work to the failed suspend operation), I end up failing
> > to boot entirely (NFS bombs out).
> > 
> > Reverting db7c7c0aeef5 ("usb: Always return 0 or -EBUSY to the runtime
> > PM core.") fixes this for me, but the commit log suggests that will break
> > lsusb. That patch has also been in for three and a half years...
> > 
> > Any ideas on how to fix this properly? In what ways does the PM core react
> > badly to -ENOENT?
> 
> Okay, this is a bad problem.
> 
> The PM core takes any error response other than -EBUSY or -EAGAIN as an 
> indication that the device is in a runtime-PM error state.  While that 
> is appropriate for a USB device, perhaps it's not so appropriate for a 
> USB host controller.
> 
> Anyway, there are two possible ways of handling this.  One is to avoid 
> changing the error code to -EBUSY when the device in question is a root 
> hub.  Just let it go into a runtime-PM error state; it won't matter 
> since the controller doesn't support runtime PM anyway.  You can test 
> this by changing the "if (status != 0)" line in usb_runtime_suspend to
> 
> 	if (status != 0 && udev->parent)

I'd tried something like this already, but I prefer your patch below. Plus,
this hack results in a failure being logged to dmesg on the initial suspend
attempt.

> The other approach is to disable runtime PM for the root hub when the 
> host controller driver doesn't have a bus_suspend or bus_resume method.
> This seems like a cleaner approach; the patch below implements it.

Thanks for this! I can confirm that your patch below fixes the issue for me,
so:

  Reported-by: Will Deacon <will.deacon@....com>
  Tested-by: Will Deacon <will.deacon@....com>

Cheers,

Will

> Index: usb-3.15/drivers/usb/core/hub.c
> ===================================================================
> --- usb-3.15.orig/drivers/usb/core/hub.c
> +++ usb-3.15/drivers/usb/core/hub.c
> @@ -1703,8 +1703,19 @@ static int hub_probe(struct usb_interfac
>  	 */
>  	pm_runtime_set_autosuspend_delay(&hdev->dev, 0);
>  
> -	/* Hubs have proper suspend/resume support. */
> -	usb_enable_autosuspend(hdev);
> +	/*
> +	 * Hubs have proper suspend/resume support, except for root hubs
> +	 * where the controller driver doesn't have bus_suspend and
> +	 * bus_resume methods.
> +	 */
> +	if (hdev->parent) {		/* normal device */
> +		usb_enable_autosuspend(hdev);
> +	} else {			/* root hub */
> +		const struct hc_driver *drv = bus_to_hcd(hdev->bus)->driver;
> +
> +		if (drv->bus_suspend && drv->bus_resume)
> +			usb_enable_autosuspend(hdev);
> +	}
>  
>  	if (hdev->level == MAX_TOPO_LEVEL) {
>  		dev_err(&intf->dev,
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/