lkml.org 
[lkml]   [2001]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Re: Q: natsemi.c spinlocks
Donald Becker wrote:
>
> > > However, natsemi.c's spinlock needs to be retained, and
> > > extended into start_tx(), because this driver has
> > > a race which has cropped up in a few others:
> > > ...
> > > if (np->cur_tx - np->dirty_tx >= TX_QUEUE_LEN - 1) {
> > > /* WINDOW HERE */
> > > np->tx_full = 1;
> > > netif_stop_queue(dev);
> > > }
> > > If the ring is currently full and an interrupt comes in
> > > at the indicated window and reaps ALL the packets in the
> > > ring, the driver ends up in state `tx_full = 1' and tramsmit
> > > disabled, but with no outstanding transmit interrupts.
>
> The better solution, which I've been adding to the drivers, is to check
> again for a just-cleared Tx queue after setting tx_full.
> That trades an extra comparison on a rarely followed path for a spinlock
> that is taken for every transmit and interrupt.
>
Please do not forget the memory barrier(s):

tx_full = 1;
if(condition)
...;

That's exactly the sequence that caused deadlocks with wait_queues -
even a Pentium cpu will evaluate the condition before the write to
tx_full is commited. I have a test program (userspace) that reliably
locks up on my P II. I can send you the details if you are interested.

I think you also need a memory barrier in the tx_interrupt codepath.

>
> Remember: spinlocks are expensive!
>

But memory barriers are extremely error prone.

What about

tx_interrupt()

if(netif_queue_stopped(dev)) {
spin_lock(&np->lock);
if(np->cur_tx - np->dirty_tx <= TX_QUEUE_LEN/2)
netif_wake_queue(dev));
spin_unlock(&np->lock);
}

hard_xmit()

if(np->cur_tx - np->dirty_tx >= TX_QUEUE_LEN-1) {
spin_lock_irq(&np->lock);
if(np->cur_tx - np_dirty_tx >= TX_QUEUE_LEN-1)
netif_stop_queue(dev);
spin_unlock_irq(&np->lock);
}

--
Manfred
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:13    [W:0.033 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site