Re: [patch 09/11] forcedeth: improve NAPI logic
On Fri, 27 Apr 2007 20:10:54 -0400 Jeff Garzik <[EMAIL PROTECTED]> wrote: > [EMAIL PROTECTED] wrote: > > From: Ingo Molnar <[EMAIL PROTECTED]> > > > > Another forcedeth.c thing: i noticed that its NAPI handler does not do > > tx-ring processing. The patch below implements this - tested on DESC_VER_2 > > hardware, with CONFIG_FORCEDETH_NAPI=y. > > > > Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]> > > Cc: Ayaz Abdulla <[EMAIL PROTECTED]> > > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> > > --- > > > > drivers/net/forcedeth.c |8 > > 1 file changed, 8 insertions(+) > > See thread comments -- this patch should be expanded. > yeah, I added the comments to the changelog for you all to read next time I send it ;) - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
[EMAIL PROTECTED] wrote: From: Ingo Molnar <[EMAIL PROTECTED]> Another forcedeth.c thing: i noticed that its NAPI handler does not do tx-ring processing. The patch below implements this - tested on DESC_VER_2 hardware, with CONFIG_FORCEDETH_NAPI=y. Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]> Cc: Ayaz Abdulla <[EMAIL PROTECTED]> Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> --- drivers/net/forcedeth.c |8 1 file changed, 8 insertions(+) See thread comments -- this patch should be expanded. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
On Thu, Apr 26, 2007 at 10:53:04AM -0400, Ayaz Abdulla wrote: > Ok. In that case, the patch needs to be improved. > > The following needs to be done when NAPI is enabled: > - remove the tx handling within the ISRs > - mask off the tx interrupts within the ISRs that handle tx processing > - re-enable tx interrupts within the NAPI handler > - add tx handling within the NAPI handler (this patch covers it) I thought a number of drivers handled tx from napi while receives were happening, but went to plain interrupts if no receives were happening. Maybe I misread the code (I have mainly dealt with pcnet32 so far). Certainly for gigabit I would think napi all the time would be much more efficient. -- Len Sorensen - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
Ayaz Abdulla wrote: Jeff Garzik wrote: Ayaz Abdulla wrote: I don't see why the NAPI handler needs to process tx packets. The ISR will handle all tx processing. It is a design choice, not a requirement. Moving non-RX interrupt processing to the NAPI handler can help as loads increase. The basic idea is to do as much work as possible in the NAPI handler with NIC interrupts masked. That mitigates global system per-interrupt overhead even more than an only-RX NAPI scheme. Several net drivers do TX completion handling in the NAPI handler. Ok. In that case, the patch needs to be improved. The following needs to be done when NAPI is enabled: - remove the tx handling within the ISRs - mask off the tx interrupts within the ISRs that handle tx processing - re-enable tx interrupts within the NAPI handler - add tx handling within the NAPI handler (this patch covers it) Agreed. Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
Jeff Garzik wrote: Ayaz Abdulla wrote: I don't see why the NAPI handler needs to process tx packets. The ISR will handle all tx processing. It is a design choice, not a requirement. Moving non-RX interrupt processing to the NAPI handler can help as loads increase. The basic idea is to do as much work as possible in the NAPI handler with NIC interrupts masked. That mitigates global system per-interrupt overhead even more than an only-RX NAPI scheme. Several net drivers do TX completion handling in the NAPI handler. Ok. In that case, the patch needs to be improved. The following needs to be done when NAPI is enabled: - remove the tx handling within the ISRs - mask off the tx interrupts within the ISRs that handle tx processing - re-enable tx interrupts within the NAPI handler - add tx handling within the NAPI handler (this patch covers it) Jeff --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
Ayaz Abdulla wrote: I don't see why the NAPI handler needs to process tx packets. The ISR will handle all tx processing. It is a design choice, not a requirement. Moving non-RX interrupt processing to the NAPI handler can help as loads increase. The basic idea is to do as much work as possible in the NAPI handler with NIC interrupts masked. That mitigates global system per-interrupt overhead even more than an only-RX NAPI scheme. Several net drivers do TX completion handling in the NAPI handler. Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 09/11] forcedeth: improve NAPI logic
I don't see why the NAPI handler needs to process tx packets. The ISR
will handle all tx processing.
[EMAIL PROTECTED] wrote:
From: Ingo Molnar <[EMAIL PROTECTED]>
Another forcedeth.c thing: i noticed that its NAPI handler does not do
tx-ring processing. The patch below implements this - tested on DESC_VER_2
hardware, with CONFIG_FORCEDETH_NAPI=y.
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Cc: Ayaz Abdulla <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
drivers/net/forcedeth.c |8
1 file changed, 8 insertions(+)
diff -puN drivers/net/forcedeth.c~forcedeth-improve-napi-logic
drivers/net/forcedeth.c
--- a/drivers/net/forcedeth.c~forcedeth-improve-napi-logic
+++ a/drivers/net/forcedeth.c
@@ -3108,9 +3108,17 @@ static int nv_napi_poll(struct net_devic
int retcode;
if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+ spin_lock_irqsave(&np->lock, flags);
+ nv_tx_done(dev);
+ spin_unlock_irqrestore(&np->lock, flags);
+
pkts = nv_rx_process(dev, limit);
retcode = nv_alloc_rx(dev);
} else {
+ spin_lock_irqsave(&np->lock, flags);
+ nv_tx_done_optimized(dev, np->tx_ring_size);
+ spin_unlock_irqrestore(&np->lock, flags);
+
pkts = nv_rx_process_optimized(dev, limit);
retcode = nv_alloc_rx_optimized(dev);
}
_
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 09/11] forcedeth: improve NAPI logic
From: Ingo Molnar <[EMAIL PROTECTED]>
Another forcedeth.c thing: i noticed that its NAPI handler does not do
tx-ring processing. The patch below implements this - tested on DESC_VER_2
hardware, with CONFIG_FORCEDETH_NAPI=y.
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Cc: Ayaz Abdulla <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
drivers/net/forcedeth.c |8
1 file changed, 8 insertions(+)
diff -puN drivers/net/forcedeth.c~forcedeth-improve-napi-logic
drivers/net/forcedeth.c
--- a/drivers/net/forcedeth.c~forcedeth-improve-napi-logic
+++ a/drivers/net/forcedeth.c
@@ -3108,9 +3108,17 @@ static int nv_napi_poll(struct net_devic
int retcode;
if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2) {
+ spin_lock_irqsave(&np->lock, flags);
+ nv_tx_done(dev);
+ spin_unlock_irqrestore(&np->lock, flags);
+
pkts = nv_rx_process(dev, limit);
retcode = nv_alloc_rx(dev);
} else {
+ spin_lock_irqsave(&np->lock, flags);
+ nv_tx_done_optimized(dev, np->tx_ring_size);
+ spin_unlock_irqrestore(&np->lock, flags);
+
pkts = nv_rx_process_optimized(dev, limit);
retcode = nv_alloc_rx_optimized(dev);
}
_
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
