[PATCH] b43: use rx desc underrun interrupt
Larry Finger
Larry.Finger at lwfinger.net
Fri Apr 19 14:17:03 EDT 2013
On 04/19/2013 09:32 AM, Jonas Gorski wrote:
> Hi,
>
> On Mon, Apr 15, 2013 at 10:02 AM, Thommy Jakobsson <thommyj at gmail.com> wrote:
>> Hi,
>>
>> long user of b43 driver under openwrt. Not sure if it has been posted
>> previously but using slow and old hardware (as routers running openwrt
>> most of the time is), the CPU is sometimes slower than the NIC. So
>> recently I, and several others, has had problem of wifi going down.
>> See ticket 7552 at openwrt for more info
>> (https://dev.openwrt.org/ticket/7552)
>>
>> I have tracked down the problem to the nic getting into a rx
>> descriptor underrun. Since the driver don't listen to that (or take
>> care of it in any other way), the Wifi seemingly just dies. The CPU is
>> waiting for data from the nic and the nic is waiting for a
>> confirmation that it can keep sending.
>>
>> I created a patch for handling of that interrupt. I have tested it by
>> reducing the number of rx slots to 16 and sending a couple of 100GB of
>> data. Previously Wifi went down within a minute when maxing out the
>> uplink (so rx for AP), now it has been working flawless for a 1.5week.
>
> Nice, very good to hear. Looks like you found the real issue the rx
> descriptor count increase was just papering over. Some comments
> regarding your patch.
>
>>
>> BR,
>> Thommy Jakobsson
>>
>
> You are missing a "Signed-off-by" - without it the patch can never be
> accepted. Also your patch is has broken tabs (they were converted to
> spaces), as well as is line-broken, so it doesn't apply anymore. You
> can't use gmail's web interface for sending patches.
>
>> Index: drivers/net/wireless/b43/dma.c
>> ===================================================================
>> --- compat-wireless-2012-09-07.orig/drivers/net/wireless/b43/dma.c
>> 2013-04-09 17:33:54.325322046 +0200
>> +++ compat-wireless-2012-09-07/drivers/net/wireless/b43/dma.c
>
> You should make your patch against the newest git tree, so usually
> wireless-testing or Rafał's b43-next.
>
>> 2013-04-09 17:34:43.473565843 +0200
>> @@ -1688,6 +1688,21 @@
>> b43_poison_rx_buffer(ring, skb);
>> sync_descbuffer_for_device(ring, dmaaddr, ring->rx_buffersize);
>> }
>> +void b43_dma_rx_discard(struct b43_dmaring *ring)
>> +{
>> + B43_WARN_ON(ring->tx);
>> +
>> + /* Device has filled all buffers, drop all packets in buffers
>> + * and let TCP decrease speed.
>> + * Set index to one desc after the last one (which is marked)
>> + * so the device will see all slots as free again
>> + */
>> + /*
>> + *TODO: How to increase rx_drop in mac80211
>> + */
>> + b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
>> + sizeof(struct b43_dmadesc32));
>> +}
>>
>> void b43_dma_rx(struct b43_dmaring *ring)
>> {
>>
>> Index: drivers/net/wireless/b43/dma.h
>> ===================================================================
>> --- compat-wireless-2012-09-07.orig/drivers/net/wireless/b43/dma.h
>> 2013-04-09 17:34:09.561397686 +0200
>> +++ compat-wireless-2012-09-07/drivers/net/wireless/b43/dma.h
>> 2013-04-09 17:34:43.461565780 +0200
>> @@ -10,7 +10,8 @@
>> #define B43_DMAIRQ_FATALMASK ((1 << 10) | (1 << 11) | (1 << 12) \
>> | (1 << 14) | (1 << 15))
>> #define B43_DMAIRQ_NONFATALMASK (1 << 13)
>> -#define B43_DMAIRQ_RX_DONE (1 << 16)
>> +#define B43_DMAIRQ_RX_DONE (1 << 16)
>
> Unnecessary whitespace change.
>
>> +#define B43_DMAIRQ_RDESC_UFLOW (1 << 13)
>
> Why not just rename B43_DMAIRQ_NONFATALMASK ?
>
>>
>> /*** 32-bit DMA Engine. ***/
>>
>> @@ -295,6 +296,8 @@
>> void b43_dma_handle_txstatus(struct b43_wldev *dev,
>> const struct b43_txstatus *status);
>>
>> +void b43_dma_rx_discard(struct b43_dmaring *ring);
>> +
>> void b43_dma_rx(struct b43_dmaring *ring);
>>
>> void b43_dma_direct_fifo_rx(struct b43_wldev *dev,
>>
>> Index: drivers/net/wireless/b43/main.c
>> ===================================================================
>> --- compat-wireless-2012-09-07.orig/drivers/net/wireless/b43/main.c
>> 2013-04-09 17:33:54.325322046 +0200
>> +++ compat-wireless-2012-09-07/drivers/net/wireless/b43/main.c
>> 2013-04-09 18:08:01.011471215 +0200
>> @@ -1894,14 +1894,6 @@
>> b43_controller_restart(dev, "DMA error");
>> return;
>> }
>> - if (merged_dma_reason & B43_DMAIRQ_NONFATALMASK) {
>> - b43err(dev->wl, "DMA error: "
>> - "0x%08X, 0x%08X, 0x%08X, "
>> - "0x%08X, 0x%08X, 0x%08X\n",
>> - dma_reason[0], dma_reason[1],
>> - dma_reason[2], dma_reason[3],
>> - dma_reason[4], dma_reason[5]);
>> - }
>
> You should say why you remove this one, and you should remove the
> B43_DMAIRQ_NONFATALMASK from the if (unlikely()) enclosing it.
>
>> }
>>
>> if (unlikely(reason & B43_IRQ_UCODE_DEBUG))
>> @@ -1920,6 +1912,14 @@
>> handle_irq_noise(dev);
>>
>> /* Check the DMA reason registers for received data. */
>> + if (dma_reason[0] & B43_DMAIRQ_RDESC_UFLOW) {
>> + //only print 256 time to not flood log
>> + if(!(dev->stats.rxdesc_underruns++&0xFF)){
>> + b43warn(dev->wl, "Rx descriptor underrun
>> (high cpu load?), throwing packets\n");
>
> Apart from the style problems, maybe doing a
> if (printk_ratelimit())
> b43warn(...);
>
> which will ensure that it won't flood the log, but still won't stop
> reporting them after the 256th time.
>
>
>> + }
>> + b43_dma_rx_discard(dev->dma.rx_ring);
>> +
>> + }
>> if (dma_reason[0] & B43_DMAIRQ_RX_DONE) {
>> if (b43_using_pio_transfers(dev))
>> b43_pio_rx(dev->pio.rx_queue);
>> @@ -1977,7 +1977,7 @@
>> return IRQ_NONE;
>>
>> dev->dma_reason[0] = b43_read32(dev, B43_MMIO_DMA0_REASON)
>> - & 0x0001DC00;
>> + & 0x0001FC00;
>> dev->dma_reason[1] = b43_read32(dev, B43_MMIO_DMA1_REASON)
>> & 0x0000DC00;
>> dev->dma_reason[2] = b43_read32(dev, B43_MMIO_DMA2_REASON)
>> @@ -3081,7 +3081,7 @@
>> b43_write32(dev, 0x018C, 0x02000000);
>> }
>> b43_write32(dev, B43_MMIO_GEN_IRQ_REASON, 0x00004000);
>> - b43_write32(dev, B43_MMIO_DMA0_IRQ_MASK, 0x0001DC00);
>> + b43_write32(dev, B43_MMIO_DMA0_IRQ_MASK, 0x0001FC00);
>> b43_write32(dev, B43_MMIO_DMA1_IRQ_MASK, 0x0000DC00);
>> b43_write32(dev, B43_MMIO_DMA2_IRQ_MASK, 0x0000DC00);
>> b43_write32(dev, B43_MMIO_DMA3_IRQ_MASK, 0x0001DC00);
>>
>> ===================================================================
>> --- compat-wireless-2012-09-07.orig/drivers/net/wireless/b43/b43.h
>> 2013-04-09 17:04:51.552680190 +0200
>> +++ compat-wireless-2012-09-07/drivers/net/wireless/b43/b43.h
>> 2013-04-09 17:46:08.472962612 +0200
>> @@ -671,6 +671,7 @@
>>
>> struct b43_stats {
>> u8 link_noise;
>> + u32 rxdesc_underruns;
>> };
>>
>> struct b43_key {
Chris,
With this patch, can the number of RX descriptors be reduced back to 64 on your
system? I realize it will be be hard to apply with the white space errors and
line wraps, but perhaps we will get a clean version soon.
Larry
More information about the b43-dev
mailing list