> The lock hangs off the usb_device rather than the the usb_port since it
> is meant to prevent unintended disconnects. Any portstatus vs khubd
> collisions are handled by pm runtime synchronization since there is no
> expectation that khubd resumes a usb_port.
>
> Once khubd has made a determination that it wants to resume or reset a
> device it will of course need to drop the state lock. Those actions
> will implicitly take the state lock.
>
> Here's the patch (to replace patch 9), now tested. There are of course
> cleanups that can follow this, but holding off until we have agreement
> on closing these races.
>
[..]
Sarah,
I had meant to send this hack out separately. I believe this is the
infinite loop that Tianyu had seen previously. I had not been able to
reproduce until now (new timings now that khubd flushes device pm
ops). This works around a remote wakeup vs port poweroff request
race. A wakeup request sets the timer, but by the time it expires the
port has been turned off, so we hit:
if ((raw_port_status & PORT_RESET) ||
!(raw_port_status & PORT_PE))
return 0xffffffff;
...in xhci_get_port_status. Cancelling the resume seems the right
choice given that userspace has stated "I don't care about remote
wakeups on this port" by clearing pm_qos_no_poweroff.
Thoughts?
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 9992fbfec85f..2333ec573594 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -1004,9 +1004,16 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> xhci_disable_port(hcd, xhci, wIndex,
> port_array[wIndex], temp);
> break;
> - case USB_PORT_FEAT_POWER:
> - writel(temp & ~PORT_POWER, port_array[wIndex]);
> + case USB_PORT_FEAT_POWER: {
> + struct xhci_bus_state *bus_state;
>
> + bus_state = &xhci->bus_state[hcd_index(hcd)];
> + writel(temp & ~PORT_POWER, port_array[wIndex]);
> + if (test_and_clear_bit(wIndex, &bus_state->resuming_ports)) {
> + bus_state->resume_done[wIndex] = 0;
> + xhci_dbg(xhci, "port%d: resume cancelled\n",
> + wIndex);
> + }
> spin_unlock_irqrestore(&xhci->lock, flags);
> temp = usb_acpi_power_manageable(hcd->self.root_hub,
> wIndex);
> @@ -1015,6 +1022,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> wIndex, false);
> spin_lock_irqsave(&xhci->lock, flags);
> break;
> + }
> default:
> goto error;
> }
--
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
