/* Disable interrupts while playing with the Tx Cmd list. */
save_flags(flags);
cli();
...
and then it re-enables interrupts _before_ it has obviously stopped
playing with the TX ring entries. Looks suspicious. In particular, it
looks like a TX interrupt could occur while the new TX entry hasn't been
filled in yet (this same thing is done in three places in set_rx_mode().
The above may not be the reason for the problems, but it looks like one
potential thing to look at for somebody who knows the driver.
The locking in general looks fairly strange. There's a very simple scheme
that works beautifully for both SMP and UP, and on UP generates minimal
code:
- add a per-controller "controller spinlock":
spinlock_t eepro100_lock = SPIN_LOCK_UNLOCKED;
- the per-controller interrupt handler does a
spin_lock(&eepro100_lock);
... do all controller accesses inside this lock ...
spin_unlock(&eepro100_lock);
which on UP compiles to no code at all, because on UP a normal spinlock
just doesn't need to do anything. So you don't have to worry about any
performance impact.
NOTE! The above only works if it's a per-interrupt lock. If you have a
driver that supports multiple controllers, the lock has to be a
per-controller lock and be associated with only one interrupt. Or you
have to protect against deadlocks by disabling interrupts locally.
- A non-interrupt context does
unsigned long flags;
spin_lock_irqsave(&eepro100_lock, flags);
... do all controller accesses inside this lock ...
spin_unlock_irqrestore(&eepro100_lock, flags);
which on UP just boils down to a normal save_flags+cli thing, ie the
same as what you have now. On SMP, it's a much faster spinlock rather
than the global IRQ lock.
So the above is equivalent to what you do now on UP, and much faster and
more robust than what the current code does now on SMP.
The reason the spin-lock has to be a per-controller thing is that when you
get the lock without disabling local interrupts (which is good, because
it's much faster that way) you have to guarantee that you can't have
recursion. For a single interrupt source, the interrupt handling
guarantees that, but if you try to share the lock across multiple
controllers with multiple interrupt sources you could get deadlock
situations when you get "nested" interrupts on different controllers.
Alternatively, you can have one global lock for many drivers, but then you
have to use the "irqsave" version for all lock accesses. Which is slower,
and not "as pretty".
Oh, btw, it looks like "set_rx_mode()" is called from within timers. Which
makes it all the more imperative to use spinlocks, because the global
interrupt lock can't protect against interrupts on another CPU from an
interrupt context like a timer (more deadlock reasons - imagine two
interrupt contexts on two different CPU's trying to do a global lock at
the same time, and both wanting to wait for the other to go away).
Donald, do you have any SMP machines available to you? I would have
assumed that SMP was the next obvious step for beowulf.. I could try to
see if my intel contacts are interested.
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/