Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

gobi: drop locks when code path might sleep

drop locks while submitting urbs, and reset interupt context when
notifying clients.  In general, ensure that might_sleep is never
called when a spinlock is held.

Signed-off-by: Jason Glasgow <jglasgow@chromium.org>

BUG=chrome-os-partner:5222
TEST=run kernel with CONFIG_DEBUG_SPINLOCK_SLEEP=y

Change-Id: I730644d9bd97590ef896da8e1876c6f739360c03
Reviewed-on: http://gerrit.chromium.org/gerrit/6110
Tested-by: Jason Glasgow <jglasgow@chromium.org>
Reviewed-by: Elly Jones <ellyjones@chromium.org>
  • Loading branch information...
commit 43982003ed20cbd2e691f46a730ab058a19d45ad 1 parent 6e4b466
Jason Glasgow authored
Showing with 79 additions and 61 deletions.
  1. +79 −61 drivers/net/usb/gobi/qmidevice.c
View
140 drivers/net/usb/gobi/qmidevice.c
@@ -32,6 +32,7 @@ struct readreq {
struct notifyreq {
struct list_head node;
void (*func)(struct qcusbnet *, u16, void *);
+ u16 cid;
u16 tid;
void *data;
};
@@ -66,7 +67,11 @@ static bool client_delread(struct qcusbnet *dev, u16 cid, u16 tid, void **data,
static bool client_addnotify(struct qcusbnet *dev, u16 cid, u16 tid,
void (*hook)(struct qcusbnet *, u16 cid, void *),
void *data);
-static bool client_notify(struct qcusbnet *dev, u16 cid, u16 tid);
+static struct notifyreq *client_remove_notify(struct client *client, u16 tid);
+static void client_notify_and_free(struct qcusbnet *dev,
+ struct notifyreq *notify);
+static void client_notify_list(struct qcusbnet *dev,
+ struct list_head *notifies);
static bool client_addurb(struct qcusbnet *dev, u16 cid, struct urb *urb);
static struct urb *client_delurb(struct qcusbnet *dev, u16 cid,
struct urb *urb);
@@ -173,6 +178,7 @@ static void read_callback(struct urb *urb)
struct qcusbnet *dev;
unsigned long flags;
u16 tid;
+ LIST_HEAD(notifies);
BUG_ON(!urb);
@@ -215,30 +221,31 @@ static void read_callback(struct urb *urb)
else
tid = *(u16 *)(data + result + 1);
spin_lock_irqsave(&dev->qmi.clients_lock, flags);
-
list_for_each(node, &dev->qmi.clients) {
client = list_entry(node, struct client, node);
if (client->cid == cid || (client->cid | 0xff00) == cid) {
+ struct notifyreq *notify;
+
copy = kmalloc(size, GFP_ATOMIC);
memcpy(copy, data, size);
if (!client_addread(dev, client->cid, tid, copy, size)) {
- kfree(copy);
- spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
GOBI_ERROR("failed to add read; discarding "
"read of cid=0x%04x tid=0x%04x",
cid, tid);
- resubmit_int_urb(dev->qmi.inturb);
- return;
+ kfree(copy);
+ break;
}
- client_notify(dev, client->cid, tid);
-
+ notify = client_remove_notify(client, tid);
+ if (notify)
+ list_add(&notify->node, &notifies);
if (cid >> 8 != 0xff)
break;
}
}
-
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
+
+ client_notify_list(dev, &notifies);
resubmit_int_urb(dev->qmi.inturb);
}
@@ -621,7 +628,6 @@ static int write_sync(struct qcusbnet *dev, struct buffer *data_buf, u16 cid)
}
spin_lock_irqsave(&dev->qmi.clients_lock, flags);
-
if (!client_addurb(dev, cid, urb)) {
usb_free_urb(urb);
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
@@ -630,24 +636,27 @@ static int write_sync(struct qcusbnet *dev, struct buffer *data_buf, u16 cid)
kref_put(&ctx->ref, writectx_release);
return -EINVAL;
}
+ spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
result = usb_submit_urb(urb, GFP_KERNEL);
if (result < 0) {
GOBI_ERROR("failed to submit urb: %d (cid=0x%04x)",
result, cid);
+ spin_lock_irqsave(&dev->qmi.clients_lock, flags);
removed_urb = client_delurb(dev, cid, urb);
- BUG_ON(urb != removed_urb);
- usb_free_urb(urb);
-
+ if (urb == removed_urb)
+ usb_free_urb(urb);
+ else
+ GOBI_ERROR("didn't get write urb back (cid=0x%04x)",
+ cid);
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
+
usb_autopm_put_interface(dev->iface);
kref_put(&ctx->ref, writectx_release);
return result;
}
- spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
-
result = down_interruptible(&ctx->sem);
kref_put(&ctx->ref, writectx_release);
if (!device_valid(dev)) {
@@ -833,25 +842,30 @@ static void client_free(struct qcusbnet *dev, u16 cid)
spin_lock_irqsave(&dev->qmi.clients_lock, flags);
list_for_each_safe(node, tmp, &dev->qmi.clients) {
+ struct notifyreq *notify;
+
client = list_entry(node, struct client, node);
- if (client->cid == cid) {
- while (client_notify(dev, cid, 0)) {
- ;
- }
+ if (client->cid != cid)
+ continue;
+ while ((notify = client_remove_notify(client, 0)) != NULL) {
+ /* release lock during notification */
+ spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
+ client_notify_and_free(dev, notify);
+ spin_lock_irqsave(&dev->qmi.clients_lock, flags);
+ }
+ urb = client_delurb(dev, cid, NULL);
+ while (urb != NULL) {
+ usb_kill_urb(urb);
+ usb_free_urb(urb);
urb = client_delurb(dev, cid, NULL);
- while (urb != NULL) {
- usb_kill_urb(urb);
- usb_free_urb(urb);
- urb = client_delurb(dev, cid, NULL);
- }
-
- while (client_delread(dev, cid, 0, &data, &size))
- kfree(data);
-
- list_del(&client->node);
- kfree(client);
}
+ while (client_delread(dev, cid, 0, &data, &size))
+ kfree(data);
+
+ list_del(&client->node);
+ kfree(client);
+ break;
}
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
}
@@ -965,53 +979,50 @@ static bool client_addnotify(struct qcusbnet *dev, u16 cid, u16 tid,
list_add_tail(&req->node, &client->notifies);
req->func = hook;
req->data = data;
+ req->cid = cid;
req->tid = tid;
return true;
}
-static bool client_notify(struct qcusbnet *dev, u16 cid, u16 tid)
+static struct notifyreq *client_remove_notify(struct client *client, u16 tid)
{
- struct client *client;
- struct notifyreq *delnotify, *notify;
+ struct notifyreq *notify;
struct list_head *node;
- assert_locked(dev);
-
- client = client_bycid(dev, cid);
- if (!client) {
- GOBI_ERROR("failed to find client (cid=0x%04x, tid=0x%04x)",
- cid, tid);
- return false;
- }
-
- delnotify = NULL;
-
list_for_each(node, &client->notifies) {
notify = list_entry(node, struct notifyreq, node);
if (!tid || !notify->tid || tid == notify->tid) {
- delnotify = notify;
- break;
+ list_del(&notify->node);
+ return notify;
}
-
GOBI_DEBUG("skipping data TID = %x", notify->tid);
}
+ GOBI_WARN("no notify to call (cid=0x%04x, tid=0x%04x)",
+ client->cid, tid);
+ return NULL;
+}
- if (delnotify) {
- list_del(&delnotify->node);
- if (delnotify->func) {
- /* TODO(ttuttle): This looks sketchy. */
- spin_unlock(&dev->qmi.clients_lock);
- delnotify->func(dev, cid, delnotify->data);
- spin_lock(&dev->qmi.clients_lock);
- }
- kfree(delnotify);
- return true;
- }
+static void client_notify_and_free(struct qcusbnet *dev,
+ struct notifyreq *notify)
+{
+ if (notify->func)
+ notify->func(dev, notify->cid, notify->data);
+ kfree(notify);
+}
- GOBI_WARN("no notify to call (cid=0x%04x, tid=0x%04x)", cid, tid);
+static void client_notify_list(struct qcusbnet *dev,
+ struct list_head *notifies)
+{
+ struct list_head *node, *tmp;
+ struct notifyreq *notify;
- return false;
+ /* The calling thread must not hold any qmidevice spinlocks */
+ list_for_each_safe(node, tmp, notifies) {
+ notify = list_entry(node, struct notifyreq, node);
+ list_del(&notify->node);
+ client_notify_and_free(dev, notify);
+ }
}
static bool client_addurb(struct qcusbnet *dev, u16 cid, struct urb *urb)
@@ -1480,9 +1491,16 @@ static bool qmi_ready(struct qcusbnet *dev, u16 timeout)
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
}
} else {
+ struct notifyreq *notify = NULL;
+ struct client *client;
+
spin_lock_irqsave(&dev->qmi.clients_lock, flags);
- client_notify(dev, QMICTL, tid);
+ client = client_bycid(dev, QMICTL);
+ if (client)
+ notify = client_remove_notify(client, tid);
spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
+ if (notify)
+ client_notify_and_free(dev, notify);
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.