e100: NAPI fixes
Subsystems (2)
PATCH 4/13 e100: NAPI fixes
From: "John W. Linville" <[email protected]>
Just curious...wanna try this patch I got from Intel? I think it may help...
---
25-akpm/drivers/net/e100.c | 69 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 57 insertions(+), 12 deletions(-)
drivers/net/e100.c renamed from drivers/net/e100.c~e100-napi-fixes +57 -12
@@ -269,6 +269,12 @@ enum scb_status { rus_mask = 0x3C, }; +enum ru_state { +RU_SUSPENDED =0 , +RU_RUNNING =1 , +RU_UNINITIALIZED =-1 , +}; +enum scb_stat_ack { stat_ack_not_ours =0x00 , stat_ack_sw_gen =0x04 ,
@@ -510,7 +516,7 @@ struct nic {struct rx *rx_to_use;struct rx *rx_to_clean;struct rfd blank_rfd; -int ru_running; +enum ru_state ru_running;spinlock_t cb_lock____cacheline_aligned ;spinlock_t cmd_lock;
@@ -1415,12 +1421,18 @@ static int e100_alloc_cbs(struct nic *nireturn 0 ; } -static inline void e100_start_receiver (struct nic *nic) +static inline void e100_start_receiver (struct nic *nic,struct rx *rx) { +if (!nic->rxs)return ; +if (RU_SUSPENDED != nic->ru_running)return ; + +/* handle init time starts */ +if (!rx) rx = nic->rxs; +/* (Re)start RU if suspended or idle and RFA is non-NULL */ -if (!nic->ru_running && nic->rx_to_clean->skb) { -e100_exec_cmd (nic, ruc_start, nic->rx_to_clean->dma_addr); - nic->ru_running =1 ; +if (rx->skb) { +e100_exec_cmd (nic, ruc_start, rx->dma_addr); + nic->ru_running =RU_RUNNING ; } }
@@ -1471,7 +1483,7 @@ static inline int e100_rx_indicate(struc/* If data isn't ready, nothing to indicate */ if (unlikely (!(rfd_status & cb_complete))) -return -EAGAIN ; +return -ENODATA ;/* Get actual data size */ actual_size =le16_to_cpu (rfd->actual_size) &0x3FFF ;
@@ -1482,6 +1494,10 @@ static inline int e100_rx_indicate(strucpci_unmap_single (nic->pdev, rx->dma_addr,RFD_BUF_LEN ,PCI_DMA_FROMDEVICE ); +/* this allows for a fast restart without re-enabling interrupts */ +if (le16_to_cpu (rfd->command) & cb_el) + nic->ru_running =RU_SUSPENDED ; +/* Pull off the RFD and put the actual data (minus eth hdr) */ skb_reserve (skb,sizeof (struct rfd ));skb_put (skb, actual_size);
@@ -1514,20 +1530,45 @@ static inline void e100_rx_clean(struct unsigned int work_to_do) {struct rx *rx; +int restart_required =0 ; +struct rx *rx_to_start =NULL ; + +/* are we already rnr? then pay attention!!! this ensures that +* the state machine progression never allows a start with a +* partially cleaned list, avoiding a race between hardware +* and rx_to_clean when in NAPI mode */ +if (RU_SUSPENDED == nic->ru_running) + restart_required =1 ;/* Indicate newly arrived packets */ for (rx = nic->rx_to_clean; rx->skb; rx = nic->rx_to_clean = rx->next) { -if (e100_rx_indicate (nic, rx, work_done, work_to_do)) +int err =e100_rx_indicate (nic, rx, work_done, work_to_do); +if (-EAGAIN == err) { +/* hit quota so have more work to do, restart once +* cleanup is complete */ + restart_required =0 ; +break ; + }else if (-ENODATA == err)break ;/* No more to clean */ } +/* save our starting point as the place we'll restart the receiver */ +if (restart_required) + rx_to_start = nic->rx_to_clean; +/* Alloc new skbs to refill list */ for (rx = nic->rx_to_use; !rx->skb; rx = nic->rx_to_use = rx->next) {if (unlikely (e100_rx_alloc_skb (nic, rx)))break ;/* Better luck next time (see watchdog) */ } -e100_start_receiver (nic); +if (restart_required) { +// ack the rnr? +writeb (stat_ack_rnr, &nic->csr->scb.stat_ack); +e100_start_receiver (nic, rx_to_start); +if (work_done) + (*work_done)++; + } }static void e100_rx_clean_list (struct nic *nic)
@@ -1535,6 +1576,8 @@ static void e100_rx_clean_list(struct nistruct rx *rx;unsigned int i, count = nic->params.rfds.count; + nic->ru_running =RU_UNINITIALIZED ; +if (nic->rxs) {for (rx = nic->rxs, i =0 ; i < count; rx++, i++) {if (rx->skb) {
@@ -1548,7 +1591,6 @@ static void e100_rx_clean_list(struct ni } nic->rx_to_use = nic->rx_to_clean =NULL ; - nic->ru_running =0 ; }static int e100_rx_alloc_list (struct nic *nic)
@@ -1557,6 +1599,7 @@ static int e100_rx_alloc_list(struct nicunsigned int i, count = nic->params.rfds.count; nic->rx_to_use = nic->rx_to_clean =NULL ; + nic->ru_running =RU_UNINITIALIZED ;if (!(nic->rxs =kmalloc (sizeof (struct rx ) * count,GFP_ATOMIC )))return -ENOMEM ;
@@ -1572,6 +1615,7 @@ static int e100_rx_alloc_list(struct nic } nic->rx_to_use = nic->rx_to_clean = nic->rxs; + nic->ru_running =RU_SUSPENDED ;return 0 ; }
@@ -1593,7 +1637,7 @@ static irqreturn_t e100_intr(int irq, vo/* We hit Receive No Resource (RNR); restart RU after cleaning */ if (stat_ack & stat_ack_rnr) - nic->ru_running =0 ; + nic->ru_running =RU_SUSPENDED ;e100_disable_irq (nic);netif_rx_schedule (netdev);
@@ -1609,6 +1653,7 @@ static int e100_poll(struct net_device *int tx_cleaned;e100_rx_clean (nic, &work_done, work_to_do); +// should we be quota on tx? tx_cleaned =e100_tx_clean (nic);/* If no Rx and Tx cleanup work was done, exit polling mode. */
@@ -1683,7 +1728,7 @@ static int e100_up(struct nic *nic)if ((err =e100_hw_init (nic))) goto err_clean_cbs;e100_set_multicast_list (nic->netdev); -e100_start_receiver (nic); +e100_start_receiver (nic,0 );mod_timer (&nic->watchdog, jiffies);if ((err =request_irq (nic->pdev->irq, e100_intr,SA_SHIRQ , nic->netdev->name, nic->netdev)))
@@ -1749,7 +1794,7 @@ static int e100_loopback_test(struct nicmdio_write (nic->netdev, nic->mii.phy_id,MII_BMCR ,BMCR_LOOPBACK ); -e100_start_receiver (nic); +e100_start_receiver (nic,0 );if (!(skb =dev_alloc_skb (ETH_DATA_LEN ))) { err = -ENOMEM ;
_
Quoted 5 lines · On Tue, Mar 15, 2005 at 02:22:41PM -0800, [email protected] wrote:
From: "John W. Linville" <[email protected]>
Just curious...wanna try this patch I got from Intel? I think it may help...
Hmmm...I didn't really mean for that one to be a submission...
Hopefully the Intel e100 guys will be covering this in one of their updates (if they haven't already)...
John
Signature · 2 lines
John W. Linville [email protected]
Quoted 9 lines · John W. Linville
Quoted 5 lines · On Tue, Mar 15, 2005 at 02:22:41PM -0800, [email protected] wrote:
From: "John W. Linville" <[email protected]>
Just curious...wanna try this patch I got from Intel? I think it may help...
Hmmm...I didn't really mean for that one to be a submission...
Yeah, I know. But the problem it fixes is real. It's one of my "hold onto this patch so I remember we need to fix it" patches.
Quoted 6 lines · Andrew Morton
Quoted 5 lines · John W. Linville
Quoted 3 lines · On Tue, Mar 15, 2005 at 02:22:41PM -0800, [email protected] wrote:
From: "John W. Linville" <[email protected]>
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Just curious...wanna try this patch I got from Intel? I think it may help...
Hmmm...I didn't really mean for that one to be a submission...
Yeah, I know. But the problem it fixes is real. It's one of my "hold onto this patch so I remember we need to fix it" patches.
Well, I can't argue with you there...at least let me repost it w/ proper changlog info?
Patch to follow...
John
Signature · 2 lines
John W. Linville [email protected]
PATCH 1/1 e100: NAPI state machine fix
Improve state machine to fix NAPI-related data corruption.
---
This came direct from Intel (i.e. I am not the author). It has been observed to fix data corruption problems when using the e100 driver on both x86 and ppc systems.
drivers/net/e100.c | 69 +++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 57 insertions(+), 12 deletions(-)
drivers/net/e100.c renamed from drivers/net/e100.c.orig +57 -12
@@ -269,6 +269,12 @@ enum scb_status { rus_mask = 0x3C, }; +enum ru_state { +RU_SUSPENDED =0 , +RU_RUNNING =1 , +RU_UNINITIALIZED =-1 , +}; +enum scb_stat_ack { stat_ack_not_ours =0x00 , stat_ack_sw_gen =0x04 ,
@@ -510,7 +516,7 @@ struct nic {struct rx *rx_to_use;struct rx *rx_to_clean;struct rfd blank_rfd; -int ru_running; +enum ru_state ru_running;spinlock_t cb_lock____cacheline_aligned ;spinlock_t cmd_lock;
@@ -1415,12 +1421,18 @@ static int e100_alloc_cbs(struct nic *nireturn 0 ; } -static inline void e100_start_receiver (struct nic *nic) +static inline void e100_start_receiver (struct nic *nic,struct rx *rx) { +if (!nic->rxs)return ; +if (RU_SUSPENDED != nic->ru_running)return ; + +/* handle init time starts */ +if (!rx) rx = nic->rxs; +/* (Re)start RU if suspended or idle and RFA is non-NULL */ -if (!nic->ru_running && nic->rx_to_clean->skb) { -e100_exec_cmd (nic, ruc_start, nic->rx_to_clean->dma_addr); - nic->ru_running =1 ; +if (rx->skb) { +e100_exec_cmd (nic, ruc_start, rx->dma_addr); + nic->ru_running =RU_RUNNING ; } }
@@ -1471,7 +1483,7 @@ static inline int e100_rx_indicate(struc/* If data isn't ready, nothing to indicate */ if (unlikely (!(rfd_status & cb_complete))) -return -EAGAIN ; +return -ENODATA ;/* Get actual data size */ actual_size =le16_to_cpu (rfd->actual_size) &0x3FFF ;
@@ -1482,6 +1494,10 @@ static inline int e100_rx_indicate(strucpci_unmap_single (nic->pdev, rx->dma_addr,RFD_BUF_LEN ,PCI_DMA_FROMDEVICE ); +/* this allows for a fast restart without re-enabling interrupts */ +if (le16_to_cpu (rfd->command) & cb_el) + nic->ru_running =RU_SUSPENDED ; +/* Pull off the RFD and put the actual data (minus eth hdr) */ skb_reserve (skb,sizeof (struct rfd ));skb_put (skb, actual_size);
@@ -1514,20 +1530,45 @@ static inline void e100_rx_clean(struct unsigned int work_to_do) {struct rx *rx; +int restart_required =0 ; +struct rx *rx_to_start =NULL ; + +/* are we already rnr? then pay attention!!! this ensures that +* the state machine progression never allows a start with a +* partially cleaned list, avoiding a race between hardware +* and rx_to_clean when in NAPI mode */ +if (RU_SUSPENDED == nic->ru_running) + restart_required =1 ;/* Indicate newly arrived packets */ for (rx = nic->rx_to_clean; rx->skb; rx = nic->rx_to_clean = rx->next) { -if (e100_rx_indicate (nic, rx, work_done, work_to_do)) +int err =e100_rx_indicate (nic, rx, work_done, work_to_do); +if (-EAGAIN == err) { +/* hit quota so have more work to do, restart once +* cleanup is complete */ + restart_required =0 ; +break ; + }else if (-ENODATA == err)break ;/* No more to clean */ } +/* save our starting point as the place we'll restart the receiver */ +if (restart_required) + rx_to_start = nic->rx_to_clean; +/* Alloc new skbs to refill list */ for (rx = nic->rx_to_use; !rx->skb; rx = nic->rx_to_use = rx->next) {if (unlikely (e100_rx_alloc_skb (nic, rx)))break ;/* Better luck next time (see watchdog) */ } -e100_start_receiver (nic); +if (restart_required) { +// ack the rnr? +writeb (stat_ack_rnr, &nic->csr->scb.stat_ack); +e100_start_receiver (nic, rx_to_start); +if (work_done) + (*work_done)++; + } }static void e100_rx_clean_list (struct nic *nic)
@@ -1535,6 +1576,8 @@ static void e100_rx_clean_list(struct nistruct rx *rx;unsigned int i, count = nic->params.rfds.count; + nic->ru_running =RU_UNINITIALIZED ; +if (nic->rxs) {for (rx = nic->rxs, i =0 ; i < count; rx++, i++) {if (rx->skb) {
@@ -1548,7 +1591,6 @@ static void e100_rx_clean_list(struct ni } nic->rx_to_use = nic->rx_to_clean =NULL ; - nic->ru_running =0 ; }static int e100_rx_alloc_list (struct nic *nic)
@@ -1557,6 +1599,7 @@ static int e100_rx_alloc_list(struct nicunsigned int i, count = nic->params.rfds.count; nic->rx_to_use = nic->rx_to_clean =NULL ; + nic->ru_running =RU_UNINITIALIZED ;if (!(nic->rxs =kmalloc (sizeof (struct rx ) * count,GFP_ATOMIC )))return -ENOMEM ;
@@ -1572,6 +1615,7 @@ static int e100_rx_alloc_list(struct nic } nic->rx_to_use = nic->rx_to_clean = nic->rxs; + nic->ru_running =RU_SUSPENDED ;return 0 ; }
@@ -1593,7 +1637,7 @@ static irqreturn_t e100_intr(int irq, vo/* We hit Receive No Resource (RNR); restart RU after cleaning */ if (stat_ack & stat_ack_rnr) - nic->ru_running =0 ; + nic->ru_running =RU_SUSPENDED ;e100_disable_irq (nic);netif_rx_schedule (netdev);
@@ -1609,6 +1653,7 @@ static int e100_poll(struct net_device *int tx_cleaned;e100_rx_clean (nic, &work_done, work_to_do); +// should we be quota on tx? tx_cleaned =e100_tx_clean (nic);/* If no Rx and Tx cleanup work was done, exit polling mode. */
@@ -1683,7 +1728,7 @@ static int e100_up(struct nic *nic)if ((err =e100_hw_init (nic))) goto err_clean_cbs;e100_set_multicast_list (nic->netdev); -e100_start_receiver (nic); +e100_start_receiver (nic,0 );mod_timer (&nic->watchdog, jiffies);if ((err =request_irq (nic->pdev->irq, e100_intr,SA_SHIRQ , nic->netdev->name, nic->netdev)))
@@ -1749,7 +1794,7 @@ static int e100_loopback_test(struct nicmdio_write (nic->netdev, nic->mii.phy_id,MII_BMCR ,BMCR_LOOPBACK ); -e100_start_receiver (nic); +e100_start_receiver (nic,0 );if (!(skb =dev_alloc_skb (ETH_DATA_LEN ))) { err = -ENOMEM ;
Signature · 2 lines
John W. Linville [email protected]