linux-kernel - Re: [PATCH -v4 5/7] locking, arch: Update spin_unlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160602151107.GA23133@insomnia>
Date:	Thu, 2 Jun 2016 23:11:07 +0800
From:	Boqun Feng <boqun.feng@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	manfred@...orfullife.com, dave@...olabs.net,
	paulmck@...ux.vnet.ibm.com, will.deacon@....com,
	Waiman.Long@....com, tj@...nel.org, pablo@...filter.org,
	kaber@...sh.net, davem@...emloft.net, oleg@...hat.com,
	netfilter-devel@...r.kernel.org, sasha.levin@...cle.com,
	hofrat@...dl.org, jejb@...isc-linux.org, chris@...kel.net,
	rth@...ddle.net, dhowells@...hat.com, schwidefsky@...ibm.com,
	mpe@...erman.id.au, ralf@...ux-mips.org, linux@...linux.org.uk,
	rkuo@...eaurora.org, vgupta@...opsys.com, james.hogan@...tec.com,
	realmz6@...il.com, ysato@...rs.sourceforge.jp, tony.luck@...el.com,
	cmetcalf@...lanox.com
Subject: Re: [PATCH -v4 5/7] locking, arch: Update spin_unlock_wait()

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@gmail.com
> 
> 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

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)