Re: [PATCH 2/9] pasemi_mac: Clean TX ring in poll

[olof@lixom.net (Olof Johansson)]

On Mon, Jul 02, 2007 at 08:34:22AM -0400, Jeff Garzik wrote:
> >Index: netdev-2.6/drivers/net/pasemi_mac.c
> >===================================================================
> >--- netdev-2.6.orig/drivers/net/pasemi_mac.c
> >+++ netdev-2.6/drivers/net/pasemi_mac.c
> >@@ -1052,6 +1052,7 @@ static int pasemi_mac_poll(struct net_de
> > 	int pkts, limit = min(*budget, dev->quota);
> > 	struct pasemi_mac *mac = netdev_priv(dev);
> > 
> >+	pasemi_mac_clean_tx(mac);
> > 	pkts = pasemi_mac_clean_rx(mac, limit);
> 
> Why is this needed?  Is your TX interrupt mitigated or delayed somehow?

On some level hardware, we only have the possibility of taking the
TX interrupt after a certain number of packets, and no way of doing a
(hw) timer-based one (i.e.  after no packets have been sent for x ms,
interrupt). Taking an interrupt for every TX packet isn't an interesting
proposal, and doing it solely based on count > 1 can leave stale packets
there for a long time when the network is otherwise idle.

So, I would need to do a software based timer. The poll function seemed like
a good enough solution for now though, since it's called periodically even
when the NAPI mode is such that interrupts are enabled.

> In general, drivers should only clean TX in one place.  Doing so in 
> multiple places tends to indicate a bug somewhere.

Yes, I wish I could just do it in the interrupt handler. I could do it
with a dedicated timer if you prefer, but I'd still need to have two
ways of entering the tx clean function.


-Olof