Zoltan Menyhart <[email protected]> wrote:
>
> I'm afraid "unlock_buffer()" works incorrectly
> (at least on the ia64 architecture).
>
> As far As I can see, nothing makes it sure that data modifications
> issued inside the critical section be globally visible before the
> "BH_Lock" bit gets cleared.
>
> When we acquire a resource, we need the "acq" semantics, when we
> release the resource, we need the "rel" semantics (obviously).
>
> Some architectures (at least the ia64) require explicit operations
> to ensure these semantics, the ordinary "loads" and "stores" do not
> guarantee any ordering.
>
> For the "stand alone" bit operations, these semantics do not matter.
> They are implemented by use of atomic operations in SMP mode, which
> operations need to follow either the "acq" semantics or the "rel"
> semantics (at least the ia64). An arbitrary choice was made to use
> the "acq" semantics.
>
> We use bit operations to implement buffer locking.
> As a consequence, the requirement of the "rel" semantics, when we
> release the resource, is not met (at least on the ia64).
>
> - Either an "smp_mb__before_clear_bit()" is lacking
> (if we want to keep the existing definition of "clear_bit()"
> with its "acq" semantics).
> Note that "smp_mb__before_clear_bit()" is a bidirectional fence
> operation on ia64, it is less efficient than the simple
> "rel" semantics.
>
> - Or a new bit clearing service needs to be added that includes
> the "rel" semantics, say "release_N_clear_bit()"
> (since we are actually _releasing_ a lock :-))
>
> Thanks,
>
> Zoltan Menyhart
>
>
>
> buffer.c:
>
> void fastcall unlock_buffer(struct buffer_head *bh)
> {
> clear_buffer_locked(bh);
> smp_mb__after_clear_bit();
> wake_up_bit(&bh->b_state, BH_Lock);
> }
>
>
> asm-ia64/bitops.h:
>
> /*
> * clear_bit() has "acquire" semantics.
> */
> #define smp_mb__before_clear_bit() smp_mb()
> #define smp_mb__after_clear_bit() do { /* skip */; } while (0)
>
> /**
> * clear_bit - Clears a bit in memory
> * @nr: Bit to clear
> * @addr: Address to start counting from
> *
> * clear_bit() is atomic and may not be reordered. However, it does
> * not contain a memory barrier, so if it is used for locking purposes,
> * you should call smp_mb__before_clear_bit() and/or
> smp_mb__after_clear_bit()
> * in order to ensure changes are visible on other processors.
> */
alpha and powerpc define both of these as smp_mb(). sparc64 is similar,
but smarter.
atomic_ops.txt says:
/* All memory operations before this call will
* be globally visible before the clear_bit().
*/
smp_mb__before_clear_bit();
clear_bit( ... );
/* The clear_bit() will be visible before all
* subsequent memory operations.
*/
smp_mb__after_clear_bit();
So it would appear that to make all the modifications which were made to
the buffer visible after the clear_bit(), yes, we should be using
smp_mb__before_clear_bit();
unlock_page() does both...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
