Re: [IA64] Effective __clear_bit_unlock V2

From: Zoltan Menyhart <Zoltan.Menyhart_at_bull.net>
Date: 2007-10-22 19:01:39
Christoph Lameter wrote:

> --- linux-2.6.orig/include/asm-ia64/bitops.h	2007-10-19 04:05:38.000000000 -0700
> +++ linux-2.6/include/asm-ia64/bitops.h	2007-10-19 10:50:34.000000000 -0700

...

>  /**
> + * __clear_bit_unlock - Non-atomically clear a bit with release
> + */
> +static __inline__ void
> +__clear_bit_unlock (int nr, volatile void *addr)
> +{
> +	barrier();
> +	__clear_bit(nr, addr);
> +}
> +
> +/**

Well, the "old" __clear_bit() went like this:

__clear_bit (int nr, volatile void *addr)
{
	*((__u32 *) addr + (nr >> 5)) &= ~(1 << (nr & 31));
}

(I really do not see why "addr" is a pointer to a volatile stuff.
If it really can change independently from the current thread of
control, then a non atomic operation is not much useful.
Luckily :-) "addr" is casted into a pointer to a non volatile stuff.)

It has been changed into:

__clear_bit (int nr, volatile void *addr)
{
        volatile __u32 *p = (__u32 *) addr + (nr >> 5);
        __u32 m = 1 << (nr & 31);
        *p &= ~m;
}

I guess because of __clear_bit_unlock() needs only.
__set_bit() still keeps its ".rel" / ".acq" free store / load instruction.

__set_bit (int nr, volatile void *addr)
{
        *((__u32 *) addr + (nr >> 5)) |= (1 << (nr & 31));
}

Note that most of the __clear_bit() / __set_bit() usage are some bit map
or indicator manipulations. They do not need any ".rel" / ".acq"
semantics.

Let's put back the old ".rel" / ".acq" free __clear_bit() and
let's use specific variant of ...clear_bit() where ".rel" semantics
is requred.
Let's avoid adding ".acq" semantics to ...clear_bit() where is is not
required.


Zoltan
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Mon Oct 22 18:59:52 2007

This archive was generated by hypermail 2.1.8 : 2007-10-22 19:00:22 EST