Re: Another ldcw inline assembler patch

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Sun, 2008-06-29 at 14:55 -0600, Grant Grundler wrote:
> On Sat, Jun 28, 2008 at 05:34:07PM -0600, Matthew Wilcox wrote:
> > On Sat, Jun 28, 2008 at 06:07:57PM -0400, John David Anglin wrote:
> > > There are two reasons to expose the memory *a in the asm:
> > > 
> > > 1) To prevent the compiler from discarding a preceeding write to *a, and
> > > 2) to prevent it from caching *a in a register over the asm.
> > 
> > Do either of those scenarios apply, given that every usage of this is
> > preceded by an asm clobbering memory?
> > 
> > I believe the correct thing to do is to take out the two mb()s in the
> > various spin_lock routines and make the __ldcw() macro itself clobber
> > memory.
> 
> I agree. Do you want jda to submit another patch or did you want kyle to
> take jda's patch and apply a second one to remove the mb()'s?

I really wouldn't do that.  Parisc is fairly unique in that we have C
spinlocks (most other architectures have asm coded ones).  The
requirement of the spinlock routines are that they be atomic memory
clobbers (as in the sequence of statements that does one shouldn't be
moved by the compiler)---which is why our C ones have mb before and
after.

if you make __ldcw() a memory clobber, that will pretty much cover the
atomic memory clobber requirements of __raw_spin_lock_flags() ... but
look at the rest of them ... it won't cover for them.  If we have to
have these mb()s in the rest, it makes sense to have them in
__raw_spin_lock_flags() as well, just to avoid problems if someone tries
to optimise again.  There probably should be a comment in this file to
that effect.

James