On Thu, 2006-03-08 at 17:09 -0700, Michael Chan wrote:
> On Thu, 2006-08-03 at 18:08 -0400, jamal wrote:
>
> >
> > Yes in your case you need to hold the lock but not in the e1000 case.
> > I dont understand though why you need to check for wake in the tx path.
> >
>
> tg3_start_xmit()
>
> if (tx_ring_empty)
> <- tg3_tx()
> netif_stop_queue()
>
>
> At the arrow, tg3_tx() can come in and clean up the entire ring but will
> not see that the queue is stopped and therefore will not call
> netif_wake_queue(). As a result, the tx_queue is stopped forever.
>
Indeed - if you do it that way.
What about (actually e1000 is not exactly like this but closer than tg3
is):
tg3_start_xmit():
prepare skb
grab ring tx lock irq safe
put skb on ring
if (full-ring)
stop
release ring tx lock and restore flags blah
and in tg3_tx():
if likely(stopped) {
// essentially no lock needed here ....
prune tx ring
if prunned > threshold
wake
} else if not stopped {
// need a lock here ...
grab ring tx lock irq safe
prune tx ring
release ring tx lock
}
The above is safe to run tg3_start_xmit on one CPU and tg3_tx an
another.
And you can then take advantage of asynchronous wakes and remove the
paranoia checks you have on tx.
cheers,
jamal
-
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
