All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
To: linux-wireless@vger.kernel.org
Cc: Daniel Drake <dsd@gentoo.org>,
	"John W. Linville" <linville@tuxdriver.com>,
	Ulrich Kunitz <kune@deine-taler.de>
Subject: [PATCH 01/22] zd1211rw: use urb anchors for tx and fix tx-queue disabling
Date: Mon, 31 Jan 2011 20:47:08 +0200	[thread overview]
Message-ID: <20110131184708.10044.23351.stgit@fate.lan> (raw)
In-Reply-To: <20110131184657.10044.98610.stgit@fate.lan>

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: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
---
 drivers/net/wireless/zd1211rw/zd_usb.c |  108 +++++++++++---------------------
 drivers/net/wireless/zd1211rw/zd_usb.h |    8 +-
 2 files changed, 41 insertions(+), 75 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index 06041cb..c32a247 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -779,19 +779,20 @@ void zd_usb_disable_tx(struct zd_usb *usb)
 {
 	struct zd_usb_tx *tx = &usb->tx;
 	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);
-	list_for_each_safe(pos, n, &tx->free_urb_list) {
-		list_del(pos);
-		usb_free_urb(list_entry(pos, struct urb, urb_list));
-	}
-	tx->enabled = 0;
+	WARN_ON(tx->submitted_urbs != 0);
 	tx->submitted_urbs = 0;
+	spin_unlock_irqrestore(&tx->lock, flags);
+
 	/* The stopped state is ignored, relying on ieee80211_wake_queues()
 	 * 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)
 	struct zd_usb_tx *tx = &usb->tx;
 
 	spin_lock_irqsave(&tx->lock, flags);
-	tx->enabled = 1;
+	atomic_set(&tx->enabled, 1);
 	tx->submitted_urbs = 0;
 	ieee80211_wake_queues(zd_usb_to_hw(usb));
 	tx->stopped = 0;
 	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)
 {
 	struct zd_usb_tx *tx = &usb->tx;
@@ -905,6 +856,16 @@ static void tx_urb_complete(struct urb *urb)
 	struct sk_buff *skb;
 	struct ieee80211_tx_info *info;
 	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) {
 	case 0:
@@ -922,20 +883,15 @@ static void tx_urb_complete(struct urb *urb)
 		goto resubmit;
 	}
 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);
-	free_tx_urb(usb, urb);
+	usb_free_urb(urb);
 	tx_dec_submitted_urbs(usb);
 	return;
 resubmit:
+	usb_anchor_urb(urb, &tx->submitted);
 	r = usb_submit_urb(urb, GFP_ATOMIC);
 	if (r) {
+		usb_unanchor_urb(urb);
 		dev_dbg_f(urb_dev(urb), "error resubmit urb %p %d\n", urb, r);
 		goto free_urb;
 	}
@@ -958,8 +914,14 @@ int zd_usb_tx(struct zd_usb *usb, struct sk_buff *skb)
 	int r;
 	struct usb_device *udev = zd_usb_to_usbdev(usb);
 	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) {
 		r = -ENOMEM;
 		goto out;
@@ -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),
 		          skb->data, skb->len, tx_urb_complete, skb);
 
+	usb_anchor_urb(urb, &tx->submitted);
 	r = usb_submit_urb(urb, GFP_ATOMIC);
-	if (r)
+	if (r) {
+		usb_unanchor_urb(urb);
 		goto error;
+	}
 	tx_inc_submitted_urbs(usb);
 	return 0;
 error:
-	free_tx_urb(usb, urb);
+	usb_free_urb(urb);
 out:
 	return r;
 }
@@ -1005,9 +970,9 @@ static inline void init_usb_tx(struct zd_usb *usb)
 {
 	struct zd_usb_tx *tx = &usb->tx;
 	spin_lock_init(&tx->lock);
-	tx->enabled = 0;
+	atomic_set(&tx->enabled, 0);
 	tx->stopped = 0;
-	INIT_LIST_HEAD(&tx->free_urb_list);
+	init_usb_anchor(&tx->submitted);
 	tx->submitted_urbs = 0;
 }
 
@@ -1240,6 +1205,7 @@ static void disconnect(struct usb_interface *intf)
 	ieee80211_unregister_hw(hw);
 
 	/* Just in case something has gone wrong! */
+	zd_usb_disable_tx(usb);
 	zd_usb_disable_rx(usb);
 	zd_usb_disable_int(usb);
 
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.h b/drivers/net/wireless/zd1211rw/zd_usb.h
index 1b1655c..233ce82 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.h
+++ b/drivers/net/wireless/zd1211rw/zd_usb.h
@@ -184,18 +184,18 @@ struct zd_usb_rx {
 
 /**
  * struct zd_usb_tx - structure used for transmitting frames
+ * @enabled: atomic enabled flag, indicates whether tx is enabled
  * @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
  *	device, which haven't been completed
- * @enabled: enabled flag, indicates whether tx is enabled
  * @stopped: indicates whether higher level tx queues are stopped
  */
 struct zd_usb_tx {
+	atomic_t enabled;
 	spinlock_t lock;
-	struct list_head free_urb_list;
+	struct usb_anchor submitted;
 	int submitted_urbs;
-	int enabled;
 	int stopped;
 };
 


  reply	other threads:[~2011-01-31 18:47 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-31 18:46 [PATCH 00/22] zd1211rw: add support for AP-mode Jussi Kivilinna
2011-01-31 18:47 ` Jussi Kivilinna [this message]
2011-01-31 18:47 ` [PATCH 02/22] zd1211rw: cancel process_intr work on zd_chip_disable_int() Jussi Kivilinna
2011-01-31 18:47 ` [PATCH 03/22] zd1211rw: add locking for mac->process_intr Jussi Kivilinna
2011-01-31 18:47 ` [PATCH 04/22] zd1211rw: fix beacon interval setup Jussi Kivilinna
2011-01-31 18:47 ` [PATCH 05/22] zd1211rw: move set_multicast_hash and set_rx_filter from workers to configure_filter Jussi Kivilinna
2011-01-31 18:47 ` [PATCH 06/22] zd1211rw: move set_rts_cts_work to bss_info_changed Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 07/22] zd1211rw: support setting BSSID for AP mode Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 08/22] zd1211rw: fix ack_pending in filter_ack causing tx-packet ordering problem on monitor Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 09/22] zd1211rw: let zd_set_beacon_interval() set dtim_period and add AP-beacon flag Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 10/22] zd1211rw: implement beacon fetching and handling ieee80211_get_buffered_bc() Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 11/22] mac80211: fix race between next beacon dtim and ieee80211_get_buffered_bc Jussi Kivilinna
2011-01-31 18:48 ` [PATCH 12/22] zd1211rw: add beacon watchdog and setting HW beacon more failsafe Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 13/22] zd1211rw: batch beacon config commands together Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 14/22] [v2] zd1211rw: use stack and preallocated memory for small cmd-buffers Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 15/22] zd1211rw: change interrupt URB buffer to DMA buffer Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 16/22] zd1211rw: lower hw command timeouts Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 17/22] zd1211rw: collect driver settings and add function to restore theim Jussi Kivilinna
2011-01-31 18:49 ` [PATCH 18/22] zd1211rw: add TX watchdog and device resetting Jussi Kivilinna
2011-01-31 18:50 ` [PATCH 19/22] zd1211rw: reset device when CR_BCN_FIFO_SEMAPHORE freezes in beacon setup Jussi Kivilinna
2011-01-31 18:50 ` [PATCH 20/22] zd1211rw: reset rx urbs after idle period of 30 seconds Jussi Kivilinna
2011-01-31 18:50 ` [PATCH 21/22] zd1211rw: enable NL80211_IFTYPE_AP Jussi Kivilinna
2011-01-31 18:50 ` [PATCH 22/22] zd1211rw: add useful debug output Jussi Kivilinna
2011-02-04 14:06 ` [PATCH 00/22] zd1211rw: add support for AP-mode Jussi Kivilinna
2011-02-04 21:25   ` John W. Linville

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110131184708.10044.23351.stgit@fate.lan \
    --to=jussi.kivilinna@mbnet.fi \
    --cc=dsd@gentoo.org \
    --cc=kune@deine-taler.de \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.