Return-path: Received: from mail-ew0-f20.google.com ([209.85.219.20]:35381 "EHLO mail-ew0-f20.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752809AbZAVFlA (ORCPT ); Thu, 22 Jan 2009 00:41:00 -0500 Received: by ewy13 with SMTP id 13so2150603ewy.13 for ; Wed, 21 Jan 2009 21:40:58 -0800 (PST) Message-ID: <497806E8.1060709@gmail.com> (sfid-20090122_064107_158535_791AC497) Date: Thu, 22 Jan 2009 06:40:56 +0100 From: Artur Skawina MIME-Version: 1.0 To: Christian Lamparter CC: linux-wireless@vger.kernel.org Subject: Re: [RFC][RFT][PATCH] p54usb: rx refill revamp References: <200901211450.50880.chunkeey@web.de> <49774794.507@gmail.com> <200901211924.54660.chunkeey@web.de> <4977785B.7020009@gmail.com> In-Reply-To: <4977785B.7020009@gmail.com> Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: 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