On Thu, 20 Jan 2005 01:47:53 +0100
Francois Romieu <romieu@xxxxxxxxxxxxx> wrote:
> David S. Miller <davem@xxxxxxxxxxxxx> :
> [...]
> > Originally, dev->xmit_lock was added so that drivers that were SMP dumb
> > could stay that way. Thus preserving the guarentee that there would be
> > only one active call into the dev->hard_start_xmit method across the
> > entire system. I don't think any of that is relevant any longer. All
> > of our network drivers are pretty clean in this regard.
>
> (nit)
>
> Almost all. I used the fact that dev->hard_start_xmit was issued in
> a bh disabled context to exchange spinlock_irqsave for ordered ops
> on ring indexes so as to sync hard_start_xmit and the irq handler in
> the r8169 driver. It is a bit sick but Jon Mason reported it made a
> noticeable difference to avoid the irqsave on its 4 way ppc64 and
> nobody complained about it.
Hmmm... ok then. Unfortunately, my prototype patch I just posted
will make IRQs get disabled in the ->hard_start_xmit() path.
BTW, in your close() routine you do this:
/* Give a racing hard_start_xmit a few cycles to complete. */
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(1);
This is no guarentee of progress. You might instead want to
do a synchronize_kernel() or similar, which does in fact
guarentee a quiescent state.
Or if my patch goes in use spin_unlock_wait(&netdev->xmit_lock) ;-)
|