Messages in this thread | | | From | Keith Owens <> | Subject | Re: [PATCH][2.6-mm] i386: enable interrupts on contention in spin_lock_irq | Date | Wed, 26 May 2004 17:29:46 +1000 |
| |
On Wed, 26 May 2004 03:11:07 -0400 (EDT), Zwane Mwaikambo <zwane@fsmlabs.com> wrote: >This little bit was missing from the previous patch. It will enable >interrupts whilst a cpu is spinning on a lock in spin_lock_irq as well as >spin_lock_irqsave. UP/SMP compile and runtime/stress tested on i386, >UP/SMP compile tested on amd64. > >+#define _raw_spin_lock_irq(lock) _raw_spin_lock_flags(lock, X86_EFLAGS_IF)
You are assuming that all uses of spin_lock_irq() are done when interrupts are already enabled. This _should_ be true, because the matching spin_unlock_irq() will unconditionally reenable interrupts. However I have seen buggy code where spin_lock_irq() was issued with interrupts disabled. By unconditionally passing X86_EFLAGS_IF, that buggy code can now run in one of two states :-
state 1 Enter with interrupts disabled Do some work spin_lock_irq() No lock contention, do not enable interrupts Do some more work spin_unlock_irq()
state 2 Enter with interrupts disabled Do some work spin_lock_irq() Lock contention, enable interrupts, get lock, disable interrupts Do some more work spin_unlock_irq()
Your patch opens a window where data that was protected by the disabled interrupt on entry becomes unprotected while waiting for the lock and can therefore change.
It could be that I am worrying unnecessarily, after all any code that calls spin_lock_irq() with interrupts already disabled is probably wrong to start off with. But it does need to be considered as a possible failure mode.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |