2.4.18 raid1 - fix SMP locking/interrupt
Quoted 5 lines
However a bare
spin_unlock_irq()in a function means that callers which wish to keep interrupts disabled are subtly
subverted. We've had bugs from this before.
It is trivial to catch such bugs at runtime. I tried it a year ago, and immediately run into sleep_on() users that legitimately call spin_lock_irq() with disabled interrupts. Perhaps they are gone now, I'll retest my patch.
Quoted 5 lines
So the irqrestore functions are much more robust. I believe that they should be the default choice. The non-restore versions should be viewed as a micro-optimised version, to be used with caution. The additional expense of the save/restore is quite tiny - 20-30 cycles, perhaps.
OTHO, if a function doesn't work correctly if it's called with disabled interrupts, then it should not use spin_lock_irqsave() - it's misleading.
e.g. if it calls kmalloc(GFP_KERNEL), down(), schedule(), etc.
--
Manfred
Quoted 10 lines · Manfred Spraul
Quoted 6 lines
However a bare
spin_unlock_irq()in a function means that callers which wish to keep interrupts disabled are subtly
subverted. We've had bugs from this before.It is trivial to catch such bugs at runtime. I tried it a year ago, and immediately run into
sleep_on()users that legitimately callspin_lock_irq()with disabled interrupts. Perhaps they are gone now, I'll retest my patch.
heh. Yes, I tried that too a year or so ago. Basically just add
if (!(eflags & 0x0200)) whine();
to spin_lock_irq(). There was a *lot* of whining.
Quoted 10 lines
Quoted 5 lines
So the irqrestore functions are much more robust. I believe that they should be the default choice. The non-restore versions should be viewed as a micro-optimised version, to be used with caution. The additional expense of the save/restore is quite tiny - 20-30 cycles, perhaps.
OTHO, if a function doesn't work correctly if it's called with disabled interrupts, then it should not use
spin_lock_irqsave()- it's misleading.
e.g. if it callskmalloc(GFP_KERNEL),down(),schedule(), etc.
mm? Those are legal (albeit unpleasant) inside local_irq_save(), but illegal inside global_cli() in 2.5. Aren't they? If not, then release_kernel_lock() needs talking to.
-
From: "Andrew Morton" <[email protected]>
Quoted 1 line
Quoted 1 line
OTHO, if a function doesn't work correctly if it's called with
disabled
Quoted 8 lines
Quoted 3 lines
interrupts, then it should not use
spin_lock_irqsave()- it's misleading.
e.g. if it callskmalloc(GFP_KERNEL),down(),schedule(), etc.mm? Those are legal (albeit unpleasant) inside
local_irq_save(), but illegal insideglobal_cli()in 2.5. Aren't they? If not, thenrelease_kernel_lock()needs talking to.
If a function is called with disabled interrupts, then the caller probably expects that the interrupts remain disabled - otherwise he would have reenabled them before calling. schedule reenables interrupts. The calls might be legal, but usually they indicate a bug.
--
Manfred