Re: [PATCH] [e1000]: Remove unnecessary tx_lock

[jamal <hadi@cyberus.ca>]

On Sun, 2006-06-08 at 09:05 +1000, Herbert Xu wrote:
> On Sat, Aug 05, 2006 at 12:56:42PM -0400, jamal wrote:
> >
> > > > The real risk is that the transmit routine will decide it's busy, but
> > > > at the same time an IRQ on another CPU will clean up the whole ring
> > > > and make the queue available again. Then the transmit routine exits and
> > > > says its busy, and the higher level will decide it is permanently stopped.
> 
> This is exactly the scenario where the current code can fail.  It is
> also exactly the one that my patch addresses.
>  

So would the holding of the tx lock at the top layer/qdisc_restart level
not help?

> > I noticed that the e1000 in some cases returns NETDEV_TX_OK even when it
> > has stopped the queue. This results in wasted calls into the qdisc.
> 
> Actually it's OK.  qdisc_restart will return after each NETDEV_TX_OK and
> its caller will check netif_queue_stopped before calling it again.
>  

yes, of course ;->

Actually the stat i captured was not related to this, rather it was the
number of times we entered qdisc_is_running and were unable to get a
packet. I am using the simple pfifo so it cant be the scheduler refusing
to give us a packet. I have a feeling it is just enthusiastic wake up of
the tx softirq. Thoughts on this? I will add checks to be sure. 

> > > If we could avoid NETDEV_TX_BUSY et al. from happening, the idea is
> > > that we could eliminate all of the requeueing logic in the packet
> > > scheduler layer.  It is a pipe dream from many years ago.
> > 
> > Still doable IMO - maybe a motivation for something to experiment with
> > this long weekend. 
> 
> A precondition to eliminating it altogether is to eliminate lockless
> drivers which would be a very good thing :)
> 

They have certainly complicated things. 

cheers,
jamal