Re: Another fragmentation multiqueue kludge

["Tomas Winkler" <tomasw@gmail.com>]

On Thu, Jul 24, 2008 at 12:41 AM, Tomas Winkler <tomasw@gmail.com> wrote:
> Here is the scenario. This is happening with what we have in
> wireless-testing code
>
> When hw queue get full we call
>  ieeee80211_stop_queue(dev, q);
>
> When this is happens in the middle of the fragmentation . This is
> easiest to reproduces  with fragmentation but probably happens also in
> other cases.
>
> int __ieee80211_tx()
> ....
>
> if (netif_subqueue_stopped(local->mdev,
>                                                   tx->extra_frag[i]))
>                                return IEEE80211_TX_FRAG_AGAIN;
>
>
> The fragment should  stored into local->queues_pending[queue]
> (Actually I'm not sure it is)
>
> now when we HW queue got cleared a bit we call wake queues where
> tx_pending_tasklet is woken up ( queue is not woken up here)
>
> void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
> {
>        struct ieee80211_local *local = hw_to_local(hw);
>
>        if (test_bit(queue, local->queues_pending)) {
>                tasklet_schedule(&local->tx_pending_tasklet);
>        }
>      ..... <snip>
> }
>
> The check if the queue  is open causes that queue is actually never
> woken up so traffic is stalled. However removing this line causes some
> other panic I didn't have time to process.
>
> void ieee80211_tx_pending(unsigned long data)
> {
> <snip>
>
>        netif_tx_lock_bh(dev);
>        for (i = 0; i < ieee80211_num_regular_queues(&local->hw); i++) {
>                /* Check that this queue is ok */
>                if (__netif_subqueue_stopped(local->mdev, i))
> --- This line causes that fact that the actual wake queue is never
> called.
>                        continue;
>
>                if (!test_bit(i, local->queues_pending)) {
>                        ieee80211_wake_queue(&local->hw, i);
>                        continue;
>                }
> </snip>



This patch solves the problem. Just I'm not sure about not locking a cross
   netif_start_subqueue(local->mdev, queue);
pending assumes that no new packets are to be transmitted before
pending packets are cleared out.


This patch make mac80211 transmit correctly fragmented packet after
stopping queues.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 net/mac80211/tx.c   |    8 +++++---
 net/mac80211/util.c |    7 +++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2b912cf..db6540e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1061,13 +1061,15 @@ static int ieee80211_tx_prepare(struct
ieee80211_tx_data *tx,
 static int __ieee80211_tx(struct ieee80211_local *local, struct sk_buff *skb,
                          struct ieee80211_tx_data *tx)
 {
-       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+       struct ieee80211_tx_info *info;
        int ret, i;

-       if (netif_subqueue_stopped(local->mdev, skb))
-               return IEEE80211_TX_AGAIN;

        if (skb) {
+               if (netif_subqueue_stopped(local->mdev, skb))
+                       return IEEE80211_TX_AGAIN;
+               info = IEEE80211_SKB_CB(skb);
+
                ieee80211_dump_frame(wiphy_name(local->hw.wiphy),
                                     "TX to low-level driver", skb);
                ret = local->ops->tx(local_to_hw(local), skb);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 89ce4e0..8028744 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -361,6 +361,13 @@ void ieee80211_wake_queue(struct ieee80211_hw
*hw, int queue)
        struct ieee80211_local *local = hw_to_local(hw);

        if (test_bit(queue, local->queues_pending)) {
+               /* don't call scheduler just open the queue */
+               if (ieee80211_is_multiqueue(local)) {
+                       netif_start_subqueue(local->mdev, queue);
+               } else {
+                       WARN_ON(queue != 0);
+                       netif_start_queue(local->mdev);
+               }
                tasklet_schedule(&local->tx_pending_tasklet);
        } else {
                if (ieee80211_is_multiqueue(local)) {




Tomas