From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754361AbcEXK54 (ORCPT <rfc822;w@1wt.eu>);
	Tue, 24 May 2016 06:57:56 -0400
Received: from bombadil.infradead.org ([198.137.202.9]:45298 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752427AbcEXK5y (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 24 May 2016 06:57:54 -0400
Date: Tue, 24 May 2016 12:57:44 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Boqun Feng <boqun.feng@gmail.com>, Waiman Long <Waiman.Long@hpe.com>,
        Ingo Molnar <mingo@kernel.org>, ggherdovich@suse.com,
        Mel Gorman <mgorman@techsingularity.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Paul McKenney <paulmck@linux.vnet.ibm.com>,
        Will Deacon <will.deacon@arm.com>, 1vier1@web.de
Subject: Re: sem_lock() vs qspinlocks
Message-ID: <20160524105744.GV3193@twins.programming.kicks-ass.net>
References: <20160520115819.GF3193@twins.programming.kicks-ass.net>
 <20160520140533.GA20726@insomnia>
 <20160520152149.GH3193@twins.programming.kicks-ass.net>
 <20160520160436.GQ3205@twins.programming.kicks-ass.net>
 <CA+55aFyHQqrBAnod2BUA3X88bNdamzLZrFyZcZDLwudxjaib1Q@mail.gmail.com>
 <20160520210618.GK3193@twins.programming.kicks-ass.net>
 <CA+55aFwtNsR0RVy_=kmSezyfV5eQiH4yHft7bwR21SZT9-zSZw@mail.gmail.com>
 <20160521004839.GA28231@linux-uzut.site>
 <20160521073739.GB15728@worktop.ger.corp.intel.com>
 <fad29add-e019-9af2-0220-40b9f4205568@colorfullife.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <fad29add-e019-9af2-0220-40b9f4205568@colorfullife.com>
User-Agent: Mutt/1.5.21 (2012-12-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.