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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100210103358.41a45851@marrow.netinsight.se>
Date:	Wed, 10 Feb 2010 10:33:58 +0100
From:	Simon Kagstrom <simon.kagstrom@...insight.net>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, davej@...hat.com, ben@...adent.org.uk
Subject: Re: [PATCH 0/3]: via-velocity: Fixes for locking issues

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@...emloft.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html