Netdev List
 help / color / mirror / Atom feed
From: jamal <hadi@cyberus.ca>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Robert Olsson <Robert.Olsson@data.slu.se>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: [NET]: Prevent multiple qdisc runs
Date: Tue, 20 Jun 2006 10:42:06 -0400	[thread overview]
Message-ID: <1150814526.5270.64.camel@jzny2> (raw)
In-Reply-To: <20060619223326.GA20354@gondor.apana.org.au>

Herbert,
Thanks for your patience.

On Tue, 2006-20-06 at 08:33 +1000, Herbert Xu wrote:

> First of all you could receive an IRQ in between dropping xmit_lock
> and regaining the queue lock.  

Indeed you could. Sorry, I overlooked that in my earlier email. This
issue has been there forever though - and i dont mean to dilute its
existence by saying the chances of it happening are very very slim. I
claim though that you will be _unable to reproduce this in an
experimental setup_ i.e thats how complex it is. 

> Secondly we now have lockless drivers where this assumption also 
> does not hold.

Ok, forgot about lockless drivers;
The chances are certainly much higher with lockless driver for a very
simple reason. We used to have lock ordering that is now changed for
lockless drivers. i.e we had:

1) grab qlock, 
2)  dq
3)  grab txlock, 
4) release qlock, 
5)    transmit, 
6) release txlock

to the new sequence #1,#2,#4,#3,#5,#6
and at times that same replacement txlock being also used in the rx path
to guard the tx DMA. 
A possible solution is to alias the tx lock to be dev->txlock
(DaveM had pointed out he didnt like this approach, I cant remember the
details.)

Heres where i am coming from (you may have suspected it already):
My concern is i am not sure what the performance implications are on 
this change (yes, there goes that soup^Wperformance nazi again) or what
the impact on how good the qos granularity is any longer[1].
If it is to make lock-less drivers happy, then someone oughta validate
if this performance benefit that lockless drivers give still exists. I
almost feel like we gained the 5% from lockless driving and lost 10% for
everyone else trying to fix the sins of lockless driving. So i am unsure
of the net gain. 

I apologize for hand-waving with % numbers above and using gut feeling
instead of experimental facts - I dont have time to chase it. I have
CCed Robert who may have time to see if this impacts forwarding
performance for one. I will have more peace of mind to find out there is
no impact.

cheers,
jamal

[1] By having both the forwarding path and tx softirq from multiple CPUs
enter this qdiscrun path, the chances that a packet will be dequeued
successfully and sent out within reasonable time are higher.
The tx_collision vs tx success are a good measure of how lucky you get.
This improves timeliness and granularity of qos for one. What your patch
does is reduce the granularity/possibility that we may enter
that region sooner.


  reply	other threads:[~2006-06-20 14:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-19 12:15 [NET]: Prevent multiple qdisc runs Herbert Xu
2006-06-19 13:33 ` jamal
2006-06-19 13:42   ` Herbert Xu
2006-06-19 14:23     ` jamal
2006-06-19 14:29       ` Herbert Xu
2006-06-19 14:36         ` jamal
2006-06-19 22:33           ` Herbert Xu
2006-06-20 14:42             ` jamal [this message]
2006-06-20 23:52               ` Herbert Xu
2006-06-22 19:31                 ` jamal
2006-06-22 22:43                   ` Herbert Xu
2006-06-23  0:52                     ` jamal
2006-06-23  3:35                       ` Herbert Xu
2006-06-24 13:50                         ` Jamal Hadi Salim
2006-06-20  6:57 ` David Miller
2006-06-20  7:00   ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1150814526.5270.64.camel@jzny2 \
    --to=hadi@cyberus.ca \
    --cc=Robert.Olsson@data.slu.se \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox