Merge GT/MV642xx Support into MV643xx Driver [8/8]
Subsystems (2)
PATCH 1/1 Merge GT/MV642xx Support into MV643xx Driver [8/8]
Fix long standing panic with regards to descriptors and locking.
---
drivers/net/mv643xx_eth.c +14 -9
@@ -350,13 +350,6 @@while (mp->tx_desc_count >0 ) {spin_lock_irqsave (&mp->lock, flags); - -/* tx_desc_count might have changed before acquiring the lock */ -if (mp->tx_desc_count <=0 ) { -spin_unlock_irqrestore (&mp->lock, flags); -return released; - } - tx_index = mp->tx_used_desc_q; desc = &mp->p_tx_desc_area[tx_index];#ifdef CONFIG_GT64260
@@ -367,7 +360,7 @@if (!force && (cmd_sts Ð_BUFFER_OWNED_BY_DMA )) {spin_unlock_irqrestore (&mp->lock, flags); -return released; +continue ; } mp->tx_used_desc_q = (tx_index +1 ) % mp->tx_ring_size;
@@ -1488,7 +1481,19 @@BUG_ON (netif_queue_stopped (dev));BUG_ON (skb ==NULL ); -if (mp->tx_ring_size - mp->tx_desc_count <MAX_DESCS_PER_SKB ) { +if (mp->tx_ring_size - mp->tx_desc_count <=MAX_DESCS_PER_SKB ) { +/* +* We are completely out of TX descriptors, however, +* if 'tx_used_desc_q' is zero, most likely the port +* has been configured and is up, but there is no link. +* We attempt to free a single descriptor to keep the +* 'sendto' call and rest of the stack happy. If this +* check is taken out, expect an error and kernel panic +* from 'kernel/softirq.c:141' inside 'local_bh_enable'. +*/ +if (mp->tx_used_desc_q ==0 ) +mv643xx_eth_free_all_tx_descs (dev); +printk (KERN_ERR "%s: transmit with queue full\n" , dev->name);netif_stop_queue (dev);return 1 ;
Quoted 9 lines · Steven J. Hill
Fix long standing panic with regards to descriptors and locking.
Signed-off-by Steven J. Hill <[email protected]>---
diff -ur
linux-2.6.22.1/drivers/net/mv643xx_eth.clinux-2.6.22.1-rci/drivers/net/mv643xx_eth.c
drivers/net/mv643xx_eth.c line 359
Quoted 10 lines
drivers/net/mv643xx_eth.c +0
-7@@ -350,13 +350,6 @@while (mp->tx_desc_count >0 ) {spin_lock_irqsave (&mp->lock, flags); - -/* tx_desc_count might have changed before acquiring the lock */ -if (mp->tx_desc_count <=0 ) { -spin_unlock_irqrestore (&mp->lock, flags); -return released; - } -
The code you remove above is needed to protect against a nasty bug. It was added in January and needs to stay in.
drivers/net/mv643xx_eth.c line 364
Quoted 7 lines
drivers/net/mv643xx_eth.c +1
-1@@ -367,7 +360,7 @@if (!force && (cmd_sts Ð_BUFFER_OWNED_BY_DMA )) {spin_unlock_irqrestore (&mp->lock, flags); -return released; +continue ; }
No. Continuing at that point results in the driver spinning, wasting cpu while polling for the hardware to release the DMA descriptor.
Quoted 7 lines
drivers/net/mv643xx_eth.c +2
-1@@ -1488,7 +1481,19 @@BUG_ON (netif_queue_stopped (dev));BUG_ON (skb ==NULL ); -if (mp->tx_ring_size - mp->tx_desc_count <MAX_DESCS_PER_SKB ) { +if (mp->tx_ring_size - mp->tx_desc_count <=MAX_DESCS_PER_SKB ) { +/*
drivers/net/mv643xx_eth.c line 1495
Quoted 10 lines
drivers/net/mv643xx_eth.c +10
-0+* We are completely out of TX descriptors, however, +* if 'tx_used_desc_q' is zero, most likely the port +* has been configured and is up, but there is no link. +* We attempt to free a single descriptor to keep the +* 'sendto' call and rest of the stack happy. If this +* check is taken out, expect an error and kernel panic +* from 'kernel/softirq.c:141' inside 'local_bh_enable'. +*/ +if (mp->tx_used_desc_q ==0 ) +mv643xx_eth_free_all_tx_descs (dev);
I don't understand the bug, but regardless, we need a better fix. Freeing all the descriptors when the HW xmit queue is full is not a solution.
-Dale