Re: [PATCH 6/8] i386: bitops: Don't mark memory as clobbered unnecessarily

[Satyam Sharma <ssatyam@cse.iitk.ac.in>]

Hi,

On Mon, 23 Jul 2007, Andi Kleen wrote:

> 
> > Yes, but _that_ address (of the bit-string) is protected already -- by the
> > implicit memory barrier due to the LOCK prefix.
> 
> Compiler barrier != CPU barrier.

Exactly, but the actual _synchronization_ in all users of the bitops API
should (should, at least, otherwise the bugs lie in the callers) depend
upon the _bit-string_ whose address is passed to us. That could be some
flags/lock/etc in some caller, whatever, but all the synchronization in
those users would be based upon _that_ -- and we're handling it alright
with the full CPU barrier for _that_ address. Also note that in all the
atomic/unatomic variants, we always constrain the passed pointer to
memory and for the atomic/locked variants at least, I don't really see
how compiler optimizations would get us into trouble (again, for the
_passed bit-string memory address_).

> The memory clobber is a compiler barrier that prevents its global optimizer
> from moving memory references.
> [...]
> > We shouldn't really be 
> > caring about any other memory addresses, so it doesn't affect the
> > correctness of the bitops API at all.
> 
> The problem is the relationship to other operations.


... I don't want to get argumentative, but IMHO, we don't really need
to care about other (other than the passed memory address) references.
The problem with marking all of memory clobbered is that we're
disallowing legitimate optimizations for other memory references.

> The CPU memory ordering guarantees are completely 
> independent from this.

Of course.

> This is not theoretic and we have had bugs because of this.

Those could've been due to buggy users, not necessarily the bitops.

Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/