Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2007-10-19 04:36:04
Also in:
lkml
What may happen is that action can either float upwards to give spin_lock action set IRQ_INPROGRESS spin_unlock spin_lock clear IRQ_INPROGRESS spin_unlock or it can float downwards to give spin_lock set IRQ_INPROGRESS spin_unlock spin_lock clear IRQ_INPROGRESS action spin_unlock
Well, we are generally safer here. That is, unless action is a store, it will not pass spin_lock, at least not on powerpc afaik. In fact, if action, as it is in our case, is something like if (foo) return; We cant execute the store inside spin_lock() without having loaded foo, there is an implicit dependency here. But anyway, Linus patch fixes that too if it was a problem. Now if we grep for while (test_bit and while (!test_bit I'm sure we'll find other similar surprises. That's also one of the reasons why I _love_ nick patches that add a proper lock/unlock API on bits, because at least those who are doing those hand-made pseudo-locks with bitops to save space will be getting a proper lock/unlock API, no more excuse. The network stack is doing more fancy things so it's harder (though I wonder sometimes if it's really worth the risks taken for not using spinlocks... maybe). Ben.