Commit 78fc800f authored by Jussi Kivilinna's avatar Jussi Kivilinna Committed by John W. Linville

zd1211rw: use urb anchors for tx and fix tx-queue disabling

When stress testing AP-mode I hit OOPS when unpluging or rmmodding
driver.

It appears that when tx-queue is disabled, tx-urbs might be left pending.
These can cause ehci to call non-existing tx_urb_complete() (after rmmod)
or uninitialized/reseted private structure (after disconnect()). Add skb
queue for submitted packets and unlink pending urbs on zd_usb_disable_tx().

Part of the problem seems to be usb->free_urb_list that isn't always
working as it should, causing machine freeze when trying to free the list
in zd_usb_disable_tx(). Caching free urbs isn't what other drivers seem
to be doing (usbnet for example) so strip free_usb_list.

Patch makes tx-urb handling saner with use of urb anchors.
Signed-off-by: default avatarJussi Kivilinna <jussi.kivilinna@mbnet.fi>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 681d1190
...@@ -779,19 +779,20 @@ void zd_usb_disable_tx(struct zd_usb *usb) ...@@ -779,19 +779,20 @@ void zd_usb_disable_tx(struct zd_usb *usb)
{ {
struct zd_usb_tx *tx = &usb->tx; struct zd_usb_tx *tx = &usb->tx;
unsigned long flags; unsigned long flags;
struct list_head *pos, *n;
atomic_set(&tx->enabled, 0);
/* kill all submitted tx-urbs */
usb_kill_anchored_urbs(&tx->submitted);
spin_lock_irqsave(&tx->lock, flags); spin_lock_irqsave(&tx->lock, flags);
list_for_each_safe(pos, n, &tx->free_urb_list) { WARN_ON(tx->submitted_urbs != 0);
list_del(pos);
usb_free_urb(list_entry(pos, struct urb, urb_list));
}
tx->enabled = 0;
tx->submitted_urbs = 0; tx->submitted_urbs = 0;
spin_unlock_irqrestore(&tx->lock, flags);
/* The stopped state is ignored, relying on ieee80211_wake_queues() /* The stopped state is ignored, relying on ieee80211_wake_queues()
* in a potentionally following zd_usb_enable_tx(). * in a potentionally following zd_usb_enable_tx().
*/ */
spin_unlock_irqrestore(&tx->lock, flags);
} }
/** /**
...@@ -807,63 +808,13 @@ void zd_usb_enable_tx(struct zd_usb *usb) ...@@ -807,63 +808,13 @@ void zd_usb_enable_tx(struct zd_usb *usb)
struct zd_usb_tx *tx = &usb->tx; struct zd_usb_tx *tx = &usb->tx;
spin_lock_irqsave(&tx->lock, flags); spin_lock_irqsave(&tx->lock, flags);
tx->enabled = 1; atomic_set(&tx->enabled, 1);
tx->submitted_urbs = 0; tx->submitted_urbs = 0;
ieee80211_wake_queues(zd_usb_to_hw(usb)); ieee80211_wake_queues(zd_usb_to_hw(usb));
tx->stopped = 0; tx->stopped = 0;
spin_unlock_irqrestore(&tx->lock, flags); spin_unlock_irqrestore(&tx->lock, flags);
} }
/**
* alloc_tx_urb - provides an tx URB
* @usb: a &struct zd_usb pointer
*
* Allocates a new URB. If possible takes the urb from the free list in
* usb->tx.
*/
static struct urb *alloc_tx_urb(struct zd_usb *usb)
{
struct zd_usb_tx *tx = &usb->tx;
unsigned long flags;
struct list_head *entry;
struct urb *urb;
spin_lock_irqsave(&tx->lock, flags);
if (list_empty(&tx->free_urb_list)) {
urb = usb_alloc_urb(0, GFP_ATOMIC);
goto out;
}
entry = tx->free_urb_list.next;
list_del(entry);
urb = list_entry(entry, struct urb, urb_list);
out:
spin_unlock_irqrestore(&tx->lock, flags);
return urb;
}
/**
* free_tx_urb - frees a used tx URB
* @usb: a &struct zd_usb pointer
* @urb: URB to be freed
*
* Frees the transmission URB, which means to put it on the free URB
* list.
*/
static void free_tx_urb(struct zd_usb *usb, struct urb *urb)
{
struct zd_usb_tx *tx = &usb->tx;
unsigned long flags;
spin_lock_irqsave(&tx->lock, flags);
if (!tx->enabled) {
usb_free_urb(urb);
goto out;
}
list_add(&urb->urb_list, &tx->free_urb_list);
out:
spin_unlock_irqrestore(&tx->lock, flags);
}
static void tx_dec_submitted_urbs(struct zd_usb *usb) static void tx_dec_submitted_urbs(struct zd_usb *usb)
{ {
struct zd_usb_tx *tx = &usb->tx; struct zd_usb_tx *tx = &usb->tx;
...@@ -905,6 +856,16 @@ static void tx_urb_complete(struct urb *urb) ...@@ -905,6 +856,16 @@ static void tx_urb_complete(struct urb *urb)
struct sk_buff *skb; struct sk_buff *skb;
struct ieee80211_tx_info *info; struct ieee80211_tx_info *info;
struct zd_usb *usb; struct zd_usb *usb;
struct zd_usb_tx *tx;
skb = (struct sk_buff *)urb->context;
info = IEEE80211_SKB_CB(skb);
/*
* grab 'usb' pointer before handing off the skb (since
* it might be freed by zd_mac_tx_to_dev or mac80211)
*/
usb = &zd_hw_mac(info->rate_driver_data[0])->chip.usb;
tx = &usb->tx;
switch (urb->status) { switch (urb->status) {
case 0: case 0:
...@@ -922,20 +883,15 @@ static void tx_urb_complete(struct urb *urb) ...@@ -922,20 +883,15 @@ static void tx_urb_complete(struct urb *urb)
goto resubmit; goto resubmit;
} }
free_urb: free_urb:
skb = (struct sk_buff *)urb->context;
/*
* grab 'usb' pointer before handing off the skb (since
* it might be freed by zd_mac_tx_to_dev or mac80211)
*/
info = IEEE80211_SKB_CB(skb);
usb = &zd_hw_mac(info->rate_driver_data[0])->chip.usb;
zd_mac_tx_to_dev(skb, urb->status); zd_mac_tx_to_dev(skb, urb->status);
free_tx_urb(usb, urb); usb_free_urb(urb);
tx_dec_submitted_urbs(usb); tx_dec_submitted_urbs(usb);
return; return;
resubmit: resubmit:
usb_anchor_urb(urb, &tx->submitted);
r = usb_submit_urb(urb, GFP_ATOMIC); r = usb_submit_urb(urb, GFP_ATOMIC);
if (r) { if (r) {
usb_unanchor_urb(urb);
dev_dbg_f(urb_dev(urb), "error resubmit urb %p %d\n", urb, r); dev_dbg_f(urb_dev(urb), "error resubmit urb %p %d\n", urb, r);
goto free_urb; goto free_urb;
} }
...@@ -958,8 +914,14 @@ int zd_usb_tx(struct zd_usb *usb, struct sk_buff *skb) ...@@ -958,8 +914,14 @@ int zd_usb_tx(struct zd_usb *usb, struct sk_buff *skb)
int r; int r;
struct usb_device *udev = zd_usb_to_usbdev(usb); struct usb_device *udev = zd_usb_to_usbdev(usb);
struct urb *urb; struct urb *urb;
struct zd_usb_tx *tx = &usb->tx;
urb = alloc_tx_urb(usb); if (!atomic_read(&tx->enabled)) {
r = -ENOENT;
goto out;
}
urb = usb_alloc_urb(0, GFP_ATOMIC);
if (!urb) { if (!urb) {
r = -ENOMEM; r = -ENOMEM;
goto out; goto out;
...@@ -968,13 +930,16 @@ int zd_usb_tx(struct zd_usb *usb, struct sk_buff *skb) ...@@ -968,13 +930,16 @@ int zd_usb_tx(struct zd_usb *usb, struct sk_buff *skb)
usb_fill_bulk_urb(urb, udev, usb_sndbulkpipe(udev, EP_DATA_OUT), usb_fill_bulk_urb(urb, udev, usb_sndbulkpipe(udev, EP_DATA_OUT),
skb->data, skb->len, tx_urb_complete, skb); skb->data, skb->len, tx_urb_complete, skb);
usb_anchor_urb(urb, &tx->submitted);
r = usb_submit_urb(urb, GFP_ATOMIC); r = usb_submit_urb(urb, GFP_ATOMIC);
if (r) if (r) {
usb_unanchor_urb(urb);
goto error; goto error;
}
tx_inc_submitted_urbs(usb); tx_inc_submitted_urbs(usb);
return 0; return 0;
error: error:
free_tx_urb(usb, urb); usb_free_urb(urb);
out: out:
return r; return r;
} }
...@@ -1005,9 +970,9 @@ static inline void init_usb_tx(struct zd_usb *usb) ...@@ -1005,9 +970,9 @@ static inline void init_usb_tx(struct zd_usb *usb)
{ {
struct zd_usb_tx *tx = &usb->tx; struct zd_usb_tx *tx = &usb->tx;
spin_lock_init(&tx->lock); spin_lock_init(&tx->lock);
tx->enabled = 0; atomic_set(&tx->enabled, 0);
tx->stopped = 0; tx->stopped = 0;
INIT_LIST_HEAD(&tx->free_urb_list); init_usb_anchor(&tx->submitted);
tx->submitted_urbs = 0; tx->submitted_urbs = 0;
} }
...@@ -1240,6 +1205,7 @@ static void disconnect(struct usb_interface *intf) ...@@ -1240,6 +1205,7 @@ static void disconnect(struct usb_interface *intf)
ieee80211_unregister_hw(hw); ieee80211_unregister_hw(hw);
/* Just in case something has gone wrong! */ /* Just in case something has gone wrong! */
zd_usb_disable_tx(usb);
zd_usb_disable_rx(usb); zd_usb_disable_rx(usb);
zd_usb_disable_int(usb); zd_usb_disable_int(usb);
......
...@@ -184,18 +184,18 @@ struct zd_usb_rx { ...@@ -184,18 +184,18 @@ struct zd_usb_rx {
/** /**
* struct zd_usb_tx - structure used for transmitting frames * struct zd_usb_tx - structure used for transmitting frames
* @enabled: atomic enabled flag, indicates whether tx is enabled
* @lock: lock for transmission * @lock: lock for transmission
* @free_urb_list: list of free URBs, contains all the URBs, which can be used * @submitted: anchor for URBs sent to device
* @submitted_urbs: atomic integer that counts the URBs having sent to the * @submitted_urbs: atomic integer that counts the URBs having sent to the
* device, which haven't been completed * device, which haven't been completed
* @enabled: enabled flag, indicates whether tx is enabled
* @stopped: indicates whether higher level tx queues are stopped * @stopped: indicates whether higher level tx queues are stopped
*/ */
struct zd_usb_tx { struct zd_usb_tx {
atomic_t enabled;
spinlock_t lock; spinlock_t lock;
struct list_head free_urb_list; struct usb_anchor submitted;
int submitted_urbs; int submitted_urbs;
int enabled;
int stopped; int stopped;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment