linux-kernel - Re: sem_lock() vs qspinlocks

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 24 May 2016 12:57:44 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Manfred Spraul <manfred@...orfullife.com>
Cc:	Davidlohr Bueso <dave@...olabs.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Boqun Feng <boqun.feng@...il.com>,
	Waiman Long <Waiman.Long@....com>,
	Ingo Molnar <mingo@...nel.org>, ggherdovich@...e.com,
	Mel Gorman <mgorman@...hsingularity.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Will Deacon <will.deacon@....com>, 1vier1@....de
Subject: Re: sem_lock() vs qspinlocks

On Sat, May 21, 2016 at 03:49:20PM +0200, Manfred Spraul wrote:

> >I'm tempted to put that trailing smp_rmb() in spin_unlock_wait() too;
> >because I suspect the netfilter code is broken without it.
> >
> >And it seems intuitive to assume that if we return from unlock_wait() we
> >can indeed observe the critical section we waited on.

> Then !spin_is_locked() and spin_unlock_wait() would be different with
> regards to memory barriers.
> Would that really help?

We could fix that I think; something horrible like:

static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
{
	int locked;
	smp_mb();
	locked = atomic_read(&lock->val) & _Q_LOCKED_MASK;
	smp_acquire__after_ctrl_dep();
	return locked;
}

Which if used in a conditional like:

	spin_lock(A);
	if (spin_is_locked(B)) {
		spin_unlock(A);
		spin_lock(B);
		...
	}

would still provide the ACQUIRE semantics required. The only difference
is that it would provide it to _both_ branches, which might be a little
more expensive.

> My old plan was to document the rules, and define a generic
> smp_acquire__after_spin_is_unlocked.
> https://lkml.org/lkml/2015/3/1/153

Yeah; I more or less forgot all that.

Now, I too think having the thing documented is good; _however_ I also
think having primitives that actually do what you assume them to is a
good thing.

spin_unlock_wait() not actually serializing against the spin_unlock() is
really surprising and subtle.