wl1271: Fix queue stopping/waking for TX path
authorJuuso Oikarinen <juuso.oikarinen@nokia.com>
Mon, 22 Feb 2010 06:38:33 +0000 (08:38 +0200)
committerJohn W. Linville <linville@tuxdriver.com>
Tue, 9 Mar 2010 20:03:02 +0000 (15:03 -0500)
The queue stopping/waking functionality was broken in a way that could
cause huge latencies in TX transfers and even cause the TX to stall in the
right circumstances. Correct these problems.

Signed-off-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
Reviewed-by: Kalle Valo <kalle.valo@nokia.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
drivers/net/wireless/wl12xx/wl1271.h
drivers/net/wireless/wl12xx/wl1271_main.c
drivers/net/wireless/wl12xx/wl1271_tx.c

index cc974eae009e311d8f4ba16cd1db8cf9b803d52d..7f03f899b2b01c101f2968dbc6254fd7c9a2167c 100644 (file)
@@ -491,7 +491,8 @@ int wl1271_plt_stop(struct wl1271 *wl);
 
 #define WL1271_DEFAULT_POWER_LEVEL 0
 
-#define WL1271_TX_QUEUE_MAX_LENGTH 20
+#define WL1271_TX_QUEUE_LOW_WATERMARK  10
+#define WL1271_TX_QUEUE_HIGH_WATERMARK 25
 
 /* WL1271 needs a 200ms sleep after power on, and a 20ms sleep before power
    on in case is has been shut down shortly before */
index 81fb02e829235d14303e4ad9618a847b4bf8b054..184264a53b2046272d31f4cdeed14b5b74f0d8a1 100644 (file)
@@ -791,15 +791,13 @@ static int wl1271_op_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
         * The workqueue is slow to process the tx_queue and we need stop
         * the queue here, otherwise the queue will get too long.
         */
-       if (skb_queue_len(&wl->tx_queue) >= WL1271_TX_QUEUE_MAX_LENGTH) {
-               ieee80211_stop_queues(wl->hw);
+       if (skb_queue_len(&wl->tx_queue) >= WL1271_TX_QUEUE_HIGH_WATERMARK) {
+               wl1271_debug(DEBUG_TX, "op_tx: stopping queues");
 
-               /*
-                * FIXME: this is racy, the variable is not properly
-                * protected. Maybe fix this by removing the stupid
-                * variable altogether and checking the real queue state?
-                */
+               spin_lock_irqsave(&wl->wl_lock, flags);
+               ieee80211_stop_queues(wl->hw);
                set_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags);
+               spin_unlock_irqrestore(&wl->wl_lock, flags);
        }
 
        return NETDEV_TX_OK;
index 2e057b0e325706d7a6966e2cd01b038eb5afc853..7926471cd0953973854b7136a29f9449db6745c1 100644 (file)
@@ -278,22 +278,12 @@ void wl1271_tx_work(struct work_struct *work)
 
                ret = wl1271_tx_frame(wl, skb);
                if (ret == -EBUSY) {
-                       /* firmware buffer is full, stop queues */
-                       wl1271_debug(DEBUG_TX, "tx_work: fw buffer full, "
-                                    "stop queues");
-                       ieee80211_stop_queues(wl->hw);
-                       set_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags);
+                       /* firmware buffer is full, lets stop transmitting. */
                        skb_queue_head(&wl->tx_queue, skb);
                        goto out_ack;
                } else if (ret < 0) {
                        dev_kfree_skb(skb);
                        goto out_ack;
-               } else if (test_and_clear_bit(WL1271_FLAG_TX_QUEUE_STOPPED,
-                                             &wl->flags)) {
-                       /* firmware buffer has space, restart queues */
-                       wl1271_debug(DEBUG_TX,
-                                    "complete_packet: waking queues");
-                       ieee80211_wake_queues(wl->hw);
                }
        }
 
@@ -380,8 +370,6 @@ void wl1271_tx_complete(struct wl1271 *wl)
        u32 count, fw_counter;
        u32 i;
 
-       wl1271_debug(DEBUG_TX, "tx_complete received, packets: %d", count);
-
        /* read the tx results from the chipset */
        wl1271_read(wl, le32_to_cpu(memmap->tx_result),
                    wl->tx_res_if, sizeof(*wl->tx_res_if), false);
@@ -393,6 +381,7 @@ void wl1271_tx_complete(struct wl1271 *wl)
                                tx_result_host_counter), fw_counter);
 
        count = fw_counter - wl->tx_results_count;
+       wl1271_debug(DEBUG_TX, "tx_complete received, packets: %d", count);
 
        /* verify that the result buffer is not getting overrun */
        if (unlikely(count > TX_HW_RESULT_QUEUE_LEN))
@@ -409,6 +398,19 @@ void wl1271_tx_complete(struct wl1271 *wl)
 
                wl->tx_results_count++;
        }
+
+       if (test_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags) &&
+           skb_queue_len(&wl->tx_queue) <= WL1271_TX_QUEUE_LOW_WATERMARK) {
+               unsigned long flags;
+
+               /* firmware buffer has space, restart queues */
+               wl1271_debug(DEBUG_TX, "tx_complete: waking queues");
+               spin_lock_irqsave(&wl->wl_lock, flags);
+               ieee80211_wake_queues(wl->hw);
+               clear_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags);
+               spin_unlock_irqrestore(&wl->wl_lock, flags);
+               ieee80211_queue_work(wl->hw, &wl->tx_work);
+       }
 }
 
 /* caller must hold wl->mutex */