[PATCH 4/6] ARM: locks: prefetch the destination word for write prior to strex

[Nicolas Pitre]

On Fri, 26 Jul 2013, Will Deacon wrote:

> Hi Nicolas,
> 
> On Thu, Jul 25, 2013 at 09:24:48PM +0100, Nicolas Pitre wrote:
> > On Thu, 25 Jul 2013, Will Deacon wrote:
> > > > 
> > > > Oh joy. Why is rwlock's lock member marked volatile?
> > > 
> > > Yeah, that was the problematic guy. However, I had to fix that anyway in
> > > this patch because otherwise the definition for prefetchw when
> > > !ARCH_HAS_PREFETCHW (which expands to __builtin_prefetch(x,1)) will explode.
> > 
> > Why not always keep ARCH_HAS_PREFETCHW defined then, and have 
> > __builtin_prefetch([const volatile void *)x,1)) when we can't do better?
> > 
> > Or why not adding this cast to the generic definition?
> 
> Yes, I'd prefer to fix the generic definition as a separate patch. However,
> I think the only case that this generates warnings is when a volatile * is
> passed. If volatile is actually required, I question whether prefetching
> from that location even makes sense in the first place since, in the kernel,
> the keyword seems to be generally associated with IO rather than CPUs accessing
> shared memory via caches.
> 
> What do you think?

The only thing volatile is good for is to tell the compiler not to 
attempt any kind of optimization on the access.  Obviously this is 
needed for IO accessors, but we don't want the compiler to think it can 
cache a previous reading of the lock value either.  So as long as all 
accesses are done through some accessors (you did add ACCESS_ONCE() in 
your patch removing the volatile for that reason) then we don't need 
that keyword in the lock definition.


Nicolas