rt2x00: Serialize TX operations on a queue.

PATCH New Ivo van Doorn 1 participant 6 messages Jul 6, 2011 Networking drivers (wireless)

lore

Share & tools

lore

Ivo van Doorn

author

2011-07-06 21:00 UTC #0 lore raw

PATCH 1/6 rt2x00: Serialize TX operations on a queue.

From: Gertjan van Wingerde <[email protected]>

The rt2x00 driver gets frequent occurrences of the following error message when operating under load:
phy0 -> rt2x00queue_write_tx_frame: Error - Arrived at non-free entry in the non-full queue 2.

This is caused by simultaneous attempts from mac80211 to send a frame via rt2x00, which are not properly serialized inside rt2x00queue_write_tx_frame, causing the second frame to fail sending with the above mentioned error message.

Fix this by introducing a per-queue spinlock to serialize the TX operations on that queue.

Reported-by Andreas Hartmann <[email protected]>

Signed-off-by Gertjan van Wingerde <[email protected]>

Acked-by Helmut Schaa <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2x00queue.c |   21 ++++++++++++++++-----
 drivers/net/wireless/rt2x00/rt2x00queue.h |    2 ++
 2 files changed, 18 insertions(+), 5 deletions(-)

drivers/net/wireless/rt2x00/rt2x00queue.c +16 -5

index c7fc9de..551cee1 100644
@@ -555,15 +555,21 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 			       bool local)
 {
 	struct ieee80211_tx_info *tx_info;
-	struct queue_entry *entry = rt2x00queue_get_entry(queue, Q_INDEX);
+	struct queue_entry *entry;
 	struct txentry_desc txdesc;
 	struct skb_frame_desc *skbdesc;
 	u8 rate_idx, rate_flags;
+	int ret = 0;
+
+	spin_lock(&queue->tx_lock);
+
+	entry = rt2x00queue_get_entry(queue, Q_INDEX);
 
 	if (unlikely(rt2x00queue_full(queue))) {
 		ERROR(queue->rt2x00dev,
 		      "Dropping frame due to full tx queue %d.\n", queue->qid);
-		return -ENOBUFS;
+		ret = -ENOBUFS;
+		goto out;
 	}
 
 	if (unlikely(test_and_set_bit(ENTRY_OWNER_DEVICE_DATA,

@@ -572,7 +578,8 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 		      "Arrived at non-free entry in the non-full queue %d.\n"
 		      "Please file bug report to %s.\n",
 		      queue->qid, DRV_PROJECT);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	/*

@@ -634,7 +641,8 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 	if (unlikely(rt2x00queue_write_tx_data(entry, &txdesc))) {
 		clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
 		entry->skb = NULL;
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	set_bit(ENTRY_DATA_PENDING, &entry->flags);

@@ -643,7 +651,9 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 	rt2x00queue_write_tx_descriptor(entry, &txdesc);
 	rt2x00queue_kick_tx_queue(queue, &txdesc);
 
-	return 0;
+out:
+	spin_unlock(&queue->tx_lock);
+	return ret;
 }
 
 int rt2x00queue_clear_beacon(struct rt2x00_dev *rt2x00dev,

@@ -1184,6 +1194,7 @@ static void rt2x00queue_init(struct rt2x00_dev *rt2x00dev,
 			     struct data_queue *queue, enum data_queue_qid qid)
 {
 	mutex_init(&queue->status_lock);
+	spin_lock_init(&queue->tx_lock);
 	spin_lock_init(&queue->index_lock);
 
 	queue->rt2x00dev = rt2x00dev;

drivers/net/wireless/rt2x00/rt2x00queue.h +2 -0

index 5900474..f2100f4 100644
@@ -432,6 +432,7 @@ enum data_queue_flags {
  * @flags: Entry flags, see &enum queue_entry_flags.
  * @status_lock: The mutex for protecting the start/stop/flush
  *	handling on this queue.
+ * @tx_lock: Spinlock to serialize tx operations on this queue.
  * @index_lock: Spinlock to protect index handling. Whenever @index, @index_done or
  *	@index_crypt needs to be changed this lock should be grabbed to prevent
  *	index corruption due to concurrency.

@@ -458,6 +459,7 @@ struct data_queue {
 	unsigned long flags;
 
 	struct mutex status_lock;
+	spinlock_t tx_lock;
 	spinlock_t index_lock;
 
 	unsigned int count;

Signature · 1 line

1.7.2.3

Ivo van Doorn

author

2011-07-06 21:00 UTC #1 lore raw

PATCH 2/6 rt2x00: Don't use queue entry as parameter when creating TX descriptor.

From: Gertjan van Wingerde <[email protected]>

The functions that create the tx descriptor structure do not operate on a queue entry at all. Signal this fact in the code by not providing a queue entry as a parameter, but the rt2x00 device structure and the skb directly.

This patch is a preparation for reducing the time a queue is locked for a tx operation.

Signed-off-by Gertjan van Wingerde <[email protected]>

Acked-by Helmut Schaa <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2x00crypto.c |    6 ++--
 drivers/net/wireless/rt2x00/rt2x00lib.h    |    3 +-
 drivers/net/wireless/rt2x00/rt2x00queue.c  |   52 +++++++++++++++-------------
 3 files changed, 33 insertions(+), 28 deletions(-)

drivers/net/wireless/rt2x00/rt2x00crypto.c +3 -3

index 1bb9d46..1ca4c7f 100644
@@ -45,11 +45,11 @@ enum cipher rt2x00crypto_key_to_cipher(struct ieee80211_key_conf *key)
 	}
 }
 
-void rt2x00crypto_create_tx_descriptor(struct queue_entry *entry,
+void rt2x00crypto_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
+				       struct sk_buff *skb,
 				       struct txentry_desc *txdesc)
 {
-	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(entry->skb);
+	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
 
 	if (!test_bit(CAPABILITY_HW_CRYPTO, &rt2x00dev->cap_flags) || !hw_key)

drivers/net/wireless/rt2x00/rt2x00lib.h +2 -1

index 322cc4f..15cdc7e 100644
@@ -336,7 +336,8 @@ static inline void rt2x00debug_update_crypto(struct rt2x00_dev *rt2x00dev,
  */
 #ifdef CONFIG_RT2X00_LIB_CRYPTO
 enum cipher rt2x00crypto_key_to_cipher(struct ieee80211_key_conf *key);
-void rt2x00crypto_create_tx_descriptor(struct queue_entry *entry,
+void rt2x00crypto_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
+				       struct sk_buff *skb,
 				       struct txentry_desc *txdesc);
 unsigned int rt2x00crypto_tx_overhead(struct rt2x00_dev *rt2x00dev,
 				      struct sk_buff *skb);

drivers/net/wireless/rt2x00/rt2x00queue.c +28 ~~-24~~

126 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Ivo van Doorn

author

2011-07-06 21:00 UTC #2 lore raw

PATCH 3/6 rt2x00: Reduce window of a queue's tx lock.

From: Gertjan van Wingerde <[email protected]>

Currently a lot of actions that can be done without the queue's tx lock being held are done inside the locked area.
Move them out to have a leaner and meaner code that operates while the tx lock is being held.

Signed-off-by Gertjan van Wingerde <[email protected]>

Acked-by Helmut Schaa <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2x00queue.c |   51 +++++++++++++++--------------
 1 files changed, 26 insertions(+), 25 deletions(-)

drivers/net/wireless/rt2x00/rt2x00queue.c +26 ~~-25~~

index a70e700..29edb9f 100644
@@ -565,33 +565,11 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 	u8 rate_idx, rate_flags;
 	int ret = 0;
 
-	spin_lock(&queue->tx_lock);
-
-	entry = rt2x00queue_get_entry(queue, Q_INDEX);
-
-	if (unlikely(rt2x00queue_full(queue))) {
-		ERROR(queue->rt2x00dev,
-		      "Dropping frame due to full tx queue %d.\n", queue->qid);
-		ret = -ENOBUFS;
-		goto out;
-	}
-
-	if (unlikely(test_and_set_bit(ENTRY_OWNER_DEVICE_DATA,
-				      &entry->flags))) {
-		ERROR(queue->rt2x00dev,
-		      "Arrived at non-free entry in the non-full queue %d.\n"
-		      "Please file bug report to %s.\n",
-		      queue->qid, DRV_PROJECT);
-		ret = -EINVAL;
-		goto out;
-	}
-
 	/*
 	 * Copy all TX descriptor information into txdesc,
 	 * after that we are free to use the skb->cb array
 	 * for our information.
 	 */
-	entry->skb = skb;
 	rt2x00queue_create_tx_descriptor(queue->rt2x00dev, skb, &txdesc);
 
 	/*

@@ -604,7 +582,6 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 	rate_flags = tx_info->control.rates[0].flags;
 	skbdesc = get_skb_frame_desc(skb);
 	memset(skbdesc, 0, sizeof(*skbdesc));
-	skbdesc->entry = entry;
 	skbdesc->tx_rate_idx = rate_idx;
 	skbdesc->tx_rate_flags = rate_flags;

@@ -633,9 +610,33 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 	 * for PCI devices.
 	 */
 	if (test_bit(REQUIRE_L2PAD, &queue->rt2x00dev->cap_flags))
-		rt2x00queue_insert_l2pad(entry->skb, txdesc.header_length);
+		rt2x00queue_insert_l2pad(skb, txdesc.header_length);
 	else if (test_bit(REQUIRE_DMA, &queue->rt2x00dev->cap_flags))
-		rt2x00queue_align_frame(entry->skb);
+		rt2x00queue_align_frame(skb);
+
+	spin_lock(&queue->tx_lock);
+
+	if (unlikely(rt2x00queue_full(queue))) {
+		ERROR(queue->rt2x00dev,
+		      "Dropping frame due to full tx queue %d.\n", queue->qid);
+		ret = -ENOBUFS;
+		goto out;
+	}
+
+	entry = rt2x00queue_get_entry(queue, Q_INDEX);
+
+	if (unlikely(test_and_set_bit(ENTRY_OWNER_DEVICE_DATA,
+				      &entry->flags))) {
+		ERROR(queue->rt2x00dev,
+		      "Arrived at non-free entry in the non-full queue %d.\n"
+		      "Please file bug report to %s.\n",
+		      queue->qid, DRV_PROJECT);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	skbdesc->entry = entry;
+	entry->skb = skb;
 
 	/*
 	 * It could be possible that the queue was corrupted and this

Signature · 1 line

1.7.2.3

Ivo van Doorn

author

2011-07-06 21:00 UTC #3 lore raw

PATCH 4/6 rt2x00: Add device ID for RT539F device.

From: Gertjan van Wingerde <[email protected]>

Reported-by Wim Vander Schelden <[email protected]>

Signed-off-by Gertjan van Wingerde <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2800pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/net/wireless/rt2x00/rt2800pci.c +1 -0

index 5513edf..fd99449 100644
@@ -1160,6 +1160,7 @@ static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
 #endif
 #ifdef CONFIG_RT2800PCI_RT53XX
 	{ PCI_DEVICE(0x1814, 0x5390) },
+	{ PCI_DEVICE(0x1814, 0x539f) },
 #endif
 	{ 0, }
 };

Signature · 1 line

1.7.2.3

Ivo van Doorn

author

2011-07-06 21:01 UTC #4 lore raw

PATCH 5/6 rt2x00: Properly identify rt2800usb devices.

From: Gertjan van Wingerde <[email protected]>

Sitecom WLA4000 (USB ID 0x0df6:0x0060) is an RT3072 chipset. Sitecom WLA5000 (USB ID 0x0df6:0x0062) is an RT3572 chipset.

Signed-off-by Gertjan van Wingerde <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2800usb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/net/wireless/rt2x00/rt2800usb.c +2 -2

index 6e92298..af23058 100644
@@ -1020,6 +1020,7 @@ static struct usb_device_id rt2800usb_device_table[] = {
 	{ USB_DEVICE(0x0df6, 0x0048) },
 	{ USB_DEVICE(0x0df6, 0x0051) },
 	{ USB_DEVICE(0x0df6, 0x005f) },
+	{ USB_DEVICE(0x0df6, 0x0060) },
 	/* SMC */
 	{ USB_DEVICE(0x083a, 0x6618) },
 	{ USB_DEVICE(0x083a, 0x7511) },

@@ -1076,6 +1077,7 @@ static struct usb_device_id rt2800usb_device_table[] = {
 	{ USB_DEVICE(0x148f, 0x3572) },
 	/* Sitecom */
 	{ USB_DEVICE(0x0df6, 0x0041) },
+	{ USB_DEVICE(0x0df6, 0x0062) },
 	/* Toshiba */
 	{ USB_DEVICE(0x0930, 0x0a07) },
 	/* Zinwell */

@@ -1174,8 +1176,6 @@ static struct usb_device_id rt2800usb_device_table[] = {
 	{ USB_DEVICE(0x0df6, 0x004a) },
 	{ USB_DEVICE(0x0df6, 0x004d) },
 	{ USB_DEVICE(0x0df6, 0x0053) },
-	{ USB_DEVICE(0x0df6, 0x0060) },
-	{ USB_DEVICE(0x0df6, 0x0062) },
 	/* SMC */
 	{ USB_DEVICE(0x083a, 0xa512) },
 	{ USB_DEVICE(0x083a, 0xc522) },

Signature · 1 line

1.7.2.3

Ivo van Doorn

author

2011-07-06 21:01 UTC #5 lore raw

PATCH 6/6 rt2x00: Implement tx_frames_pending mac80211 callback function.

From: Gertjan van Wingerde <[email protected]>

Implementing this callback function will cause mac80211 refrain from going to powersave state when there are still untransmitted TX frames in the queues.
This would exactly mimic the behaviour of the legacy vendor driver which also doesn't go in powersave mode if there are still TX frames that are not transmitted.
This should make powersaving and rt2x00 a better couple.

Signed-off-by Gertjan van Wingerde <[email protected]>

Acked-by Helmut Schaa <[email protected]>

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2400pci.c |    1 +
 drivers/net/wireless/rt2x00/rt2500pci.c |    1 +
 drivers/net/wireless/rt2x00/rt2500usb.c |    1 +
 drivers/net/wireless/rt2x00/rt2800pci.c |    1 +
 drivers/net/wireless/rt2x00/rt2800usb.c |    1 +
 drivers/net/wireless/rt2x00/rt2x00.h    |    1 +
 drivers/net/wireless/rt2x00/rt2x00mac.c |   14 ++++++++++++++
 drivers/net/wireless/rt2x00/rt61pci.c   |    1 +
 drivers/net/wireless/rt2x00/rt73usb.c   |    1 +
 9 files changed, 22 insertions(+), 0 deletions(-)

drivers/net/wireless/rt2x00/rt2400pci.c +1 -0

index 937f9e8..76bcc354 100644
@@ -1723,6 +1723,7 @@ static const struct ieee80211_ops rt2400pci_mac80211_ops = {
 	.set_antenna		= rt2x00mac_set_antenna,
 	.get_antenna		= rt2x00mac_get_antenna,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2x00lib_ops rt2400pci_rt2x00_ops = {

drivers/net/wireless/rt2x00/rt2500pci.c +1 -0

index d27d7b8..c288d95 100644
@@ -2016,6 +2016,7 @@ static const struct ieee80211_ops rt2500pci_mac80211_ops = {
 	.set_antenna		= rt2x00mac_set_antenna,
 	.get_antenna		= rt2x00mac_get_antenna,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2x00lib_ops rt2500pci_rt2x00_ops = {

drivers/net/wireless/rt2x00/rt2500usb.c +1 -0

index 15237c2..53c5f87 100644
@@ -1827,6 +1827,7 @@ static const struct ieee80211_ops rt2500usb_mac80211_ops = {
 	.set_antenna		= rt2x00mac_set_antenna,
 	.get_antenna		= rt2x00mac_get_antenna,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2x00lib_ops rt2500usb_rt2x00_ops = {

drivers/net/wireless/rt2x00/rt2800pci.c +1 -0

index fd99449..5319ed9 100644
@@ -1031,6 +1031,7 @@ static const struct ieee80211_ops rt2800pci_mac80211_ops = {
 	.flush			= rt2x00mac_flush,
 	.get_survey		= rt2800_get_survey,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2800_ops rt2800pci_rt2800_ops = {

drivers/net/wireless/rt2x00/rt2800usb.c +1 -0

index af23058..59e7779 100644
@@ -757,6 +757,7 @@ static const struct ieee80211_ops rt2800usb_mac80211_ops = {
 	.flush			= rt2x00mac_flush,
 	.get_survey		= rt2800_get_survey,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2800_ops rt2800usb_rt2800_ops = {

drivers/net/wireless/rt2x00/rt2x00.h +1 -0

index c446db6..db435ab 100644
@@ -1276,6 +1276,7 @@ int rt2x00mac_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx_ant);
 int rt2x00mac_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant);
 void rt2x00mac_get_ringparam(struct ieee80211_hw *hw,
 			     u32 *tx, u32 *tx_max, u32 *rx, u32 *rx_max);
+bool rt2x00mac_tx_frames_pending(struct ieee80211_hw *hw);
 
 /*
  * Driver allocation handlers.

drivers/net/wireless/rt2x00/rt2x00mac.c +14 -0

index 93bec14..8efab39 100644
@@ -818,3 +818,17 @@ void rt2x00mac_get_ringparam(struct ieee80211_hw *hw,
 	*rx_max = rt2x00dev->rx->limit;
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_get_ringparam);
+
+bool rt2x00mac_tx_frames_pending(struct ieee80211_hw *hw)
+{
+	struct rt2x00_dev *rt2x00dev = hw->priv;
+	struct data_queue *queue;
+
+	tx_queue_for_each(rt2x00dev, queue) {
+		if (!rt2x00queue_empty(queue))
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(rt2x00mac_tx_frames_pending);

drivers/net/wireless/rt2x00/rt61pci.c +1 -0

index 9d35ec1..53110b8 100644
@@ -2982,6 +2982,7 @@ static const struct ieee80211_ops rt61pci_mac80211_ops = {
 	.set_antenna		= rt2x00mac_set_antenna,
 	.get_antenna		= rt2x00mac_get_antenna,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2x00lib_ops rt61pci_rt2x00_ops = {

drivers/net/wireless/rt2x00/rt73usb.c +1 -0

index ad20953..6a93939 100644
@@ -2314,6 +2314,7 @@ static const struct ieee80211_ops rt73usb_mac80211_ops = {
 	.set_antenna		= rt2x00mac_set_antenna,
 	.get_antenna		= rt2x00mac_get_antenna,
 	.get_ringparam		= rt2x00mac_get_ringparam,
+	.tx_frames_pending	= rt2x00mac_tx_frames_pending,
 };
 
 static const struct rt2x00lib_ops rt73usb_rt2x00_ops = {

Signature · 1 line

1.7.2.3