Re: linux-next crash in xhci_add_ep_to_interval

From: Dan Williams
Date: Thu Jun 05 2014 - 15:05:53 EST


On Thu, 2014-06-05 at 13:29 -0400, Valdis.Kletnieks@xxxxxx wrote:
> On Thu, 05 Jun 2014 09:35:48 -0700, Dan Williams said:
>
> > Actually, on second look I bet xhci_alloc_tt_info() is being called
> > while hdev->maxchild is not set. Let me throw together a debug
> > patch...
>
> Sure, no problem - just let me know what variant of linux-next you
> want it applied against. :)

This should apply to -next. Use "git am -c", but I have attached it for
convenience as well. Boots for me, but more importantly does it boot
for you :-).

8<------
Subject: usb: fix ->update_hub_device() vs hdev->maxchild

From: Dan Williams <dan.j.williams@xxxxxxxxx>

Commit d8521afe3586 "usb: assign default peer ports for root hubs"
delayed marking a hub valid (set hdev->maxchild) until it had been fully
configured and to enable the publishing of valid hubs to be serialized
by usb_port_peer_mutex.

However, xhci_update_hub_device() in some cases depends on
hdev->maxchild already being set. Do the minimal fix and move it after
the setting of hdev->maxchild.

Cc: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@xxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/usb/core/hub.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index db6287025c06..971d95745e2a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1526,18 +1526,6 @@ static int hub_configure(struct usb_hub *hub,
dev_dbg(hub_dev, "%umA bus power budget for each child\n",
hub->mA_per_port);

- /* Update the HCD's internal representation of this hub before khubd
- * starts getting port status changes for devices under the hub.
- */
- if (hcd->driver->update_hub_device) {
- ret = hcd->driver->update_hub_device(hcd, hdev,
- &hub->tt, GFP_KERNEL);
- if (ret < 0) {
- message = "can't update HCD hub info";
- goto fail;
- }
- }
-
ret = hub_hub_status(hub, &hubstatus, &hubchange);
if (ret < 0) {
message = "can't get hub status";
@@ -1593,6 +1581,18 @@ static int hub_configure(struct usb_hub *hub,
if (ret < 0)
goto fail;

+ /* Update the HCD's internal representation of this hub before khubd
+ * starts getting port status changes for devices under the hub.
+ */
+ if (hcd->driver->update_hub_device) {
+ ret = hcd->driver->update_hub_device(hcd, hdev,
+ &hub->tt, GFP_KERNEL);
+ if (ret < 0) {
+ message = "can't update HCD hub info";
+ goto fail;
+ }
+ }
+
usb_hub_adjust_deviceremovable(hdev, hub->descriptor);

hub_activate(hub, HUB_INIT);

usb: fix ->update_hub_device() vs hdev->maxchild

From: Dan Williams <dan.j.williams@xxxxxxxxx>

Commit d8521afe3586 "usb: assign default peer ports for root hubs"
delayed marking a hub valid (set hdev->maxchild) until it had been fully
configured and to enable the publishing of valid hubs to be serialized
by usb_port_peer_mutex.

However, xhci_update_hub_device() in some cases depends on
hdev->maxchild already being set. Do the minimal fix and move it after
the setting of hdev->maxchild.

Cc: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@xxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/usb/core/hub.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index db6287025c06..971d95745e2a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1526,18 +1526,6 @@ static int hub_configure(struct usb_hub *hub,
dev_dbg(hub_dev, "%umA bus power budget for each child\n",
hub->mA_per_port);

- /* Update the HCD's internal representation of this hub before khubd
- * starts getting port status changes for devices under the hub.
- */
- if (hcd->driver->update_hub_device) {
- ret = hcd->driver->update_hub_device(hcd, hdev,
- &hub->tt, GFP_KERNEL);
- if (ret < 0) {
- message = "can't update HCD hub info";
- goto fail;
- }
- }
-
ret = hub_hub_status(hub, &hubstatus, &hubchange);
if (ret < 0) {
message = "can't get hub status";
@@ -1593,6 +1581,18 @@ static int hub_configure(struct usb_hub *hub,
if (ret < 0)
goto fail;

+ /* Update the HCD's internal representation of this hub before khubd
+ * starts getting port status changes for devices under the hub.
+ */
+ if (hcd->driver->update_hub_device) {
+ ret = hcd->driver->update_hub_device(hcd, hdev,
+ &hub->tt, GFP_KERNEL);
+ if (ret < 0) {
+ message = "can't update HCD hub info";
+ goto fail;
+ }
+ }
+
usb_hub_adjust_deviceremovable(hdev, hub->descriptor);

hub_activate(hub, HUB_INIT);