Messages in this thread | | | Date | Wed, 31 Jan 2001 20:15:42 +0100 | From | Manfred Spraul <> |
| |
Alan Cox wrote: > > > And one more point for the Janitor's list: > > Get rid of superflous irqsave()/irqrestore()'s - in 90% of the cases > > either spin_lock_irq() or spin_lock() is sufficient. That's both faster > > and better readable. > > Expect me to drop any submissions that do this. I'd rather take the two > clock hit in most cases because the effect of spin_lock_irq() being used > and people then changing which functions call each other and producing > impossible to debug irq mishandling cases is unacceptable. >
IMHO the main problem of spin_lock_irqsave is not the lost cpu cycles, but readability:
void public_function() { spin_lock_irqsave(); if(rare_event) internal_function() spin_unlock_irqrestore(); }
static void internal_function() { ... spin_unlock_irq(); kmalloc(GFP_KERNEL); spin_lock_irq(); }
IMHO functions that are not irq safe somewhere hidden in internal functions should never use spin_lock_irqsave(). make_request() in 2.2 falls into that category, and the irqsave() was removed.
Obviously spin_lock_irq() instead of spin_lock_irqsave() should only be done if the implementation doesn't support disabled interrupts, not if currently noone calls a function with disabled interrupts.
(make_request(), down(), smp_call_function()...)
> The original Linux network code did this with sti() not save/restore flags. > I've been there before, I am not going to allow a rerun of that disaster for > a few cycles
I hope that during 2.5 we can add debugging into spin_lock_irq(): BUG() if it's called with disabled interrupts. It's not yet possible due to schedule() with disabled interrupts (I tried it a few months ago)
-- Manfred - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
| |