Messages in this thread Patch in this message | | | Date | Thu, 25 Apr 2019 08:59:23 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC][PATCH 1/5] mips/atomic: Fix cmpxchg64 barriers |
| |
On Wed, Apr 24, 2019 at 09:00:25PM +0000, Paul Burton wrote: > Hi Peter, > > On Wed, Apr 24, 2019 at 02:36:57PM +0200, Peter Zijlstra wrote: > > There were no memory barriers on cmpxchg64() _at_all_. Fix this. > > This does looks problematic, but it's worth noting that this code path > is only applicable to 32b kernels running on 64b CPUs which is pretty > rare. The commit message as-is suggests to me that all configurations > are broken, which isn't the case (at least, not in this respect :) ).
OK, I hadn't gone through the ifdef selection process. I just encountered this cmpxchg implementation and noted a significant lack of barriers.
> > > > Cc: Paul Burton <paul.burton@mips.com> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > --- > > arch/mips/include/asm/cmpxchg.h | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > --- a/arch/mips/include/asm/cmpxchg.h > > +++ b/arch/mips/include/asm/cmpxchg.h > > @@ -290,9 +290,11 @@ static inline unsigned long __cmpxchg64( > > * will cause a build error unless cpu_has_64bits is a \ > > * compile-time constant 1. \ > > */ \ > > - if (cpu_has_64bits && kernel_uses_llsc) \ > > + if (cpu_has_64bits && kernel_uses_llsc) { \ > > + smp_mb__before_llsc(); \ > > __res = __cmpxchg64((ptr), __old, __new); \ > > - else \ > > + smp_llsc_mb(); \ > > + } else \ > > __res = __cmpxchg64_unsupported(); \ > > It would be good to also add braces around the else block, and I believe > checkpatch should be complaining about that ("braces {} should be used > on all arms of this statement").
You're right, I'll fix up.
| |