Re: [RFC][RFT][PATCH] p54usb: rx refill revamp

[Artur Skawina <art.08.09@gmail.com>]

Artur Skawina wrote:
> Christian Lamparter wrote:
>> A updated patch is attached (as file)
> 
> Will test.

Did a quick test of your patch on top of
  current w-t/pending
+
  "p54usb: fix packet loss with first generation devices"
+ this:

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index f754798..20818d6 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -92,7 +92,7 @@ static void p54u_rx_cb(struct urb *urb)
 
        if (unlikely(urb->status)) {
                dev_kfree_skb_irq(skb);
-           return;
+         goto stash_urb;
        }
 
        skb_put(skb, urb->actual_length);
@@ -141,7 +141,7 @@ static void p54u_rx_cb(struct urb *urb)
         * the less critical workqueue thread.
         * This eases the memory pressure and prevents latency spikes.
         */
-
+stash_urb:
        urb->transfer_buffer = NULL;
        urb->context = NULL;
 
That didn't work, various symptoms such as hostapd getting stuck, other unrelated
usb devices breaking, machine frozen etc.

After applying the changes below it seemed to work alright, but i only tested for
a few minutes.
[line #s are wrong]

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 20818d6..643d517 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -160,7 +173,7 @@ stash_urb:
 		 * Huh? mac80211 isn't fully initialized yet?
 		 * Please check your system, something bad is going on.
 		 */
-		WARN_ON(1);
+		printk(KERN_WARNING "p54u_rx_cb workqueue missing\n");
 		return;
 	}
 
@@ -196,9 +210,9 @@ static void p54u_free_urbs(struct ieee80211_hw *dev)
 {
 	struct p54u_priv *priv = dev->priv;
 
 	cancel_work_sync(&priv->rx_refill_work);
-	p54u_free_rx_refill_list(dev);
 	usb_kill_anchored_urbs(&priv->submitted);
+	p54u_free_rx_refill_list(dev);
 }
 
 static int p54u_rx_refill(struct ieee80211_hw *dev)
@@ -281,9 +297,9 @@ static int p54u_init_urbs(struct ieee80211_hw *dev)
 	}
 
 	p54u_rx_refill(dev);
-
 err:
-	p54u_free_rx_refill_list(dev);
+	if (ret)
+		p54u_free_rx_refill_list(dev);
 	return ret;
 }
 
The 'workqueue-missing' warnings appear on module init; there are several
p54u_init_urbs(), p54u_free_urbs() call sequences.
p54u_free_urbs() triggers the unlikely(urb->status) path while killing
the urbs, and the first time this happens you get 32 warnings.
After that everything's fine.

I put the machine under some load and while there was usually 28..31 RX
urbs queued, at one point the refilling stalled and the number went down
from 31 to 1, then 6, slowly grew to ~30, then fell to 13, then recovered.
All of this within ~5 mins of testing, w/ only a small amount of http 
traffic generated by the client. it looks like i will have to try 
allocating in the interrupt after all.

artur