Hi
On 29.04.2015 11:18, Chris Bainbridge wrote:
> Regression in commit 638139eb95d2d241781330a321e88c8dafe46078
> Author: Petr Mladek <pmladek@xxxxxxx>
> Date: Fri Sep 19 17:32:24 2014 +0200
>
> usb: hub: allow to process more usb hub events in parallel
>
> The regression resulted in intermittent failure to initialise a 10-port
> hub (with three internal VL812 4-port hub controllers) on boot, with a
> failure rate of around 8%, due to multiple race conditions when
> accessing addr_dev and slot_id in struct xhci_hcd.
>
> This regression also exposed a problem with xhci_setup_device, which
> "should be protected by the usb_address0_mutex" but no longer is due to
> commit 6fecd4f:
> Author: Todd E Brandt <todd.e.brandt@xxxxxxxxxxxxxxx>
> Date: Mon May 19 10:55:32 2014 -0700
>
> USB: separate usb_address0 mutexes for each bus
>
> With separate buses (and locks) it is no longer the case that a single
> lock will protect xhci_setup_device from accesses by two parallel
> threads processing events on the two buses.
>
> Fix this by adding a mutex to protect addr_dev and slot_id in struct
> xhci_hcd, and by making the assignment of slot_id atomic.
>
> Fixes multiple boot errors:
>
> [ 0.583008] xhci_hcd 0000:00:14.0: Bad Slot ID 2
> [ 0.583009] xhci_hcd 0000:00:14.0: Could not allocate xHCI USB device data structures
> [ 0.583012] usb usb1-port3: couldn't allocate usb_device
>
> And:
>
> [ 0.637409] xhci_hcd 0000:00:14.0: Error while assigning device slot ID
> [ 0.637417] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 32.
> [ 0.637421] usb usb1-port1: couldn't allocate usb_device
>
> And:
>
> [ 0.753372] xhci_hcd 0000:00:14.0: ERROR: unexpected setup context command completion code 0x0.
> [ 0.753373] usb 1-3: hub failed to enable device, error -22
> [ 0.753400] xhci_hcd 0000:00:14.0: Error while assigning device slot ID
> [ 0.753402] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 32.
> [ 0.753403] usb usb1-port3: couldn't allocate usb_device
>
> And:
>
> [ 11.018386] usb 1-3: device descriptor read/all, error -110
>
> And:
>
> [ 5.753838] xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command
>
> Tested with 200 reboots, resulting in no USB hub init related errors.
>
> Fixes: 638139eb95d2 ("usb: hub: allow to process more usb hub events in parallel")
> Link: https://lkml.kernel.org/g/CAP-bSRb=A0iEYobdGCLpwynS7pkxpt_9ZnwyZTPVAoy0Y=Zo3Q@xxxxxxxxxxxxxx
> Signed-off-by: Chris Bainbridge <chris.bainbridge@xxxxxxxxx>
> ---
This would work, but I think we can get rid of the global xhci->slot_id and xhci->addr_dev
altogether. Then we wouldn't need to add the new mutex either.
Atleast for the old global xhci->addr_dev completion we can use the per command completions introduced
some time ago in xhci instead.
But as Petr Mladek said there might be other issues with his earlier patch so it should probably be reverted anyway.
I'll take a better look at this if I can't get rid of the xhci->addr_dev and xhci->slot_id first.
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
