Re: [PATCH] Fix x86_64 _spin_lock_irqsave()

[Suleiman Souhlal]

Arjan van de Ven wrote:
> On Thu, 2006-08-24 at 14:33 +0200, Suleiman Souhlal wrote:
>
> > Andi Kleen wrote:
> >
> > > On Thursday 24 August 2006 13:04, Suleiman Souhlal wrote:
> > >
> > >
> > > > Andi Kleen wrote:
> > > >
> > > >
> > > > > Edward Falk <efalk@xxxxxxxxxx> writes:
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > Add spin_lock_string_flags and _raw_spin_lock_flags() to
> > > > > > asm-x86_64/spinlock.h so that _spin_lock_irqsave() has the same
> > > > > > semantics on x86_64 as it does on i386 and does *not* have interrupts
> > > > > > disabled while it is waiting for the lock.
> > > > >
> > > > >
> > > > > Did it fix anything for you?
> > > >
> > > > I think this was to work around the fact that some buggy drivers try to 
> > > > grab spinlocks without disabling interrupts when they should, which 
> > > > would cause deadlocks when trying to rendez-vous every cpu via IPIs.
> > >
> > >
> > > That doesn't help them at all because they could then deadlock later.
> >
> > If the driver uses spin_lock() when it knows that the hardware won't 
> > generate the interrupt that would need to be masked, and 
> > spin_lock_irqsave() elsewhere, there shouldn't be any deadlocks unless 
> > IPIs are involved.
>
>
> this still is bad practice and lockdep will also scream about it

Great.

> Can you point at ANY place that does this? 

From a quick inspection, drivers/net/forcedeth.c appears to do this.

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