Re: [parisc-linux] Re: [PATCH] Fix the cpumask rewrite

[Linus Torvalds]

On Thu, 1 Jul 2004, Alan Cox wrote:
>
> On Iau, 2004-07-01 at 14:11, Pavel Machek wrote:
> > Heh, with vojtech we introduced locking into input layer
> > (there was none before)... using test_bit/set_bit.
> > 
> > (I just hope set_bit etc implies memory barrier... or we'll have to do
> > it once more)
> 		
> It doesn't - the ppp layer got burned badly by this long ago. set_bit is
> not a memory barrier. OTOH you can just add an mb()

The "test_and_xxx()" things are memory barriers, but set_bit/clear_bit 
aren't (since ther aren't really supposed to be usable for locking).

It _happens_ to be one on x86, so sadly it works on 99% of the machines 
out there. And to avoid the extra (suprefluous) mb(), there are

	smp_mb__before_clear_bit()
	smp_mb__after_clear_bit()

that only works with "clear_bit()", on the assumption that the way you'd 
do locking is:

	lock:
		while (test_and_set_bit(..)) /* This is a memory barrier */
			while (test_bit(..))
				cpu_relax();

	.. protected region ..

	unlock:
		smp_mb__before_clear_bit();
		clear_bit(..);

but the fact is, the above is broken too, for a totally _unrelated_ 
reason, namely preemption. And then you have the SMP case etc still.

The fact is, you shouldn't use the bitops for locking. You _will_ get it 
wrong. Use a spinlock, and if you _really_ really need just a single bit, 
use

	bit_spin_lock(bitnum,addr)
	..
	bit_spin_unlock(bitnum,addr)

which should get all of this right, and if we ever chaneg the consistency 
rules, we'll make sure the bitlocks still work.

So please..

		Linus
-
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/