lkml.org 
[lkml]   [2015]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC][PATCH] mips: Fix arch_spin_unlock()
On Thu, Nov 12, 2015 at 02:50:00PM +0000, Måns Rullgård wrote:
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes:
>
> > On Thu, Nov 12, 2015 at 01:31:23PM +0100, Peter Zijlstra wrote:
> >> Hi
> >>
> >> I think the MIPS arch_spin_unlock() is borken.
> >>
> >> spin_unlock() must have RELEASE semantics, these require that no LOADs
> >> nor STOREs leak out from the critical section.
> >>
> >> >From what I know MIPS has a relaxed memory model which allows reads to
> >> pass stores, and as implemented arch_spin_unlock() only issues a wmb
> >> which doesn't order prior reads vs later stores.
> >>
> >> Therefore upgrade the wmb() to smp_mb().
> >>
> >> (Also, why the unconditional wmb, as opposed to smp_wmb() ?)
> >
> > One guess is that they want to order I/O accesses within the critical
> > section?
>
> Isn't that what mmiowb() is for?

Indeed it is. Perhaps they didn't trust the device drivers that they
care about to use it? Anyway, just my guess. Just out of curiosity,
what is your guess?

Thanx, Paul

> >> Maybe-Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >> ---
> >> diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
> >> index 40196bebe849..b2ca13f06152 100644
> >> --- a/arch/mips/include/asm/spinlock.h
> >> +++ b/arch/mips/include/asm/spinlock.h
> >> @@ -140,7 +140,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
> >> static inline void arch_spin_unlock(arch_spinlock_t *lock)
> >> {
> >> unsigned int serving_now = lock->h.serving_now + 1;
> >> - wmb();
> >> + smp_mb();
> >> lock->h.serving_now = (u16)serving_now;
> >> nudge_writes();
> >> }
> >>
> >
>
> --
> Måns Rullgård
> mans@mansr.com
>



\
 
 \ /
  Last update: 2015-11-12 16:21    [W:0.097 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site