Re: [PATCH 0/3]: via-velocity: Fixes for locking issues

[Simon Kagstrom <simon.kagstrom@netinsight.net>]

Hi David!

Thanks for the comments, I understand the problem more clearly now.

On Tue, 09 Feb 2010 16:31:33 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> The proper thing to do is make the ->poll() handler use the lock
> correctly by disabling interrupts.

In reply to this message is an updated patch series which uses
spin_lock_irqsave/restore in velocity_poll instead.

> If the VIA driver wants to avoid holding interrupts disabled for long
> periods of time in ->poll(), it's locking mechanisms will need to be
> rewritten completely.  The only safe way to do this is to do something
> like how the tg3 driver works, wherein the interrupt handler runs
> lockless, there is a special IRQ quiesce sequence when shutting down
> the chip, and the ->poll() handler et al. use softirq safe locks
> instead of hardirq safe ones.

Yes, I guess that would be the long-term solution for this driver. The
main problem with running the interrupt handler unlocked now is the
error handler, which is executed directly from the interrupt handler.


Anyway, when you are happy with the patch series, I think it is 2.6.33
material as the velocity driver would otherwise contain a regression
compared to 2.6.32.

Changes in the patch series:

* Patch 1: Unchanged

* Patch 2: Unchanged

* Patch 3: Use spin_lock_irqsave in velocity_poll and remove
  spin_trylock from the interrupt handler.


As before, I'm only able to test it on a uniprocessor with spinlock
debugging turned on. With the patch set, I can no longer get it to
lockup when running heavy traffic for both interrupt sources (which was
easy without the patches).

Thanks,
// Simon