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

[jamal <hadi@xxxxxxxxxx>]

On Sun, 2006-06-08 at 08:24 -0400, jamal wrote:
> On Sun, 2006-06-08 at 12:51 +1000, Herbert Xu wrote:
> > On Sat, Aug 05, 2006 at 07:36:50PM -0400, jamal wrote:
> > > 
> > > I know the qlen is >= 0 otherwise i will hit the BUG().
> > > A qlen of 0 will be interesting to find as well.
> > 
> > When the queue is woken it will always to qdisc_run regardless of
> > whether there are packets queued so this might explain what you're
> > seeing.
> > 
> 
> That aligns with what i was thinking as well - i.e what i referred to as
> possibly enthusiasm on part of the tx softirq.
> i.e if there was nothing queued, why is the softirq even woken up?
>
> Note, I observed this behavior to be a lot worse on systems that had
> very little traffic going out. Anyways, I hope to find out.
> 

The reason for this is _mostly_ not the tx softirq as i found out;->
It is actually a result of dev_queue_xmit.
The only way to tell if you can depart qdisc_is_running is by asking the
qdisc for a packet. So if you have slow traffic, its guaranteed that you
enter once to process a packet and the second time to find out that you
have to go away ;-> 

In any case, heres a patch that i believe saves some cycles in case of
a tx scheduling while some CPU is already processing the qdisc. Minimal
testing.

cheers,
jamal

diff --git a/net/core/dev.c b/net/core/dev.c
index d95e262..977e77e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1092,7 +1092,8 @@ static void dev_queue_xmit_nit(struct sk
 
 void __netif_schedule(struct net_device *dev)
 {
-	if (!test_and_set_bit(__LINK_STATE_SCHED, &dev->state)) {
+	if (!test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state) &&
+	    !test_and_set_bit(__LINK_STATE_SCHED, &dev->state)) {
 		unsigned long flags;
 		struct softnet_data *sd;