On Thu, Jun 02, 2016 at 04:44:24PM +0200, Peter Zijlstra wrote: > On Thu, Jun 02, 2016 at 10:24:40PM +0800, Boqun Feng wrote: > > On Thu, Jun 02, 2016 at 01:52:02PM +0200, Peter Zijlstra wrote: > > About spin_unlock_wait() on ppc, I actually have a fix pending review: > > > > http://lkml.kernel.org/r/1461130033-70898-1-git-send-email-boqun.feng@xxxxxxxxx > > Please use the normal commit quoting style: > > d86b8da04dfa ("arm64: spinlock: serialise spin_unlock_wait against concurrent lockers") > Good point ;-) > > that patch fixed a different problem when people want to pair a > > spin_unlock_wait() with a spin_lock(). > > Argh, indeed, and I think qspinlock is still broken there :/ But my poor > brain is about to give in for the day. > > Let me go ponder that some :/ > An intial thought of the fix is making queued_spin_unlock_wait() an atomic-nop too: static inline void queued_spin_unlock_wait(struct qspinlock *lock) { struct __qspinlock *l = (struct __qspinlock *)lock; while (!cmpxchg(&l->locked, 0, 0)) cpu_relax(); } This could make queued_spin_unlock_wait() a WRITE, with a smp_mb() preceding it, it would act like a RELEASE, which can be paired with spin_lock(). Just food for thought. ;-) > > I think we still need that fix, and there are two conflicts with this > > series: > > > > 1. arch_spin_unlock_wait() code for PPC32 was deleted, and > > consolidated into one. > > Nice. > > > 2. I actually downgraded spin_unlock_wait() to !ACQUIRE ;-) > > Fail ;-) > > > I can think of two ways to solve thoes conflicts: > > > > 1. Modify my patch to make spin_unlock_wait() an ACQUIRE, and it > > can be merged in powerpc tree, and possible go into to mainline > > before 4.7. Then there is no need for this series to have code > > for ppc, therefore no conflict. > > Hardly any other unlock_wait is an acquire, everyone is 'broken' :-/ > > > or > > > > 2. I can rebase my patch on this series, and it can be added in > > this series, and will go into mainline at 4.8. > > > > > > Michael and Peter, any thought? > > I'm fine with it going in early, I can rebase, no problem. OK, I will resend a new patch making spin_unlock_wait() align the semantics in your series. Regards, Boqun
Attachment:
signature.asc
Description: PGP signature