Re: [PATCH] locking: Generic ticket lock

From: Arnd Bergmann <arnd@arndb.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Arnd Bergmann" <arnd@arndb.de>, "Will Deacon" <will@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Waiman Long" <longman@redhat.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Guo Ren" <guoren@kernel.org>,
	"Palmer Dabbelt" <palmerdabbelt@google.com>,
	"Anup Patel" <anup@brainfault.org>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	"Christoph Müllner" <christophm30@gmail.com>,
	"Stafford Horne" <shorne@gmail.com>
Subject: Re: [PATCH] locking: Generic ticket lock
Date: Thu, 21 Oct 2021 17:31:51 +0200	[thread overview]
Message-ID: <CAK8P3a14NUvo40GFY5DfQcF28OO22=BiHJO1TzBTEMK0RAwtHg@mail.gmail.com> (raw)
In-Reply-To: <YXGD5OFbI7TEDFTr@hirez.programming.kicks-ass.net>

On Thu, Oct 21, 2021 at 5:14 PM Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Oct 21, 2021 at 03:49:51PM +0200, Arnd Bergmann wrote:
> I think for a load-store arch this thing should generate pretty close to
> optimal code. x86 can do ticket_unlock() slightly better using a single
> INCW (or ADDW 1) on the owner subword, where this implementation will to
> separate load-add-store instructions.
>
> If that is actually measurable is something else entirely.

Ok, so I guess such an architecture could take the generic implementation
and override just arch_spin_unlock() or just arch_spin_lock(), if that
makes a difference for them.

Should we perhaps turn your modified openrisc asm/spinlock.h
and asm/spin_lock_types.h the fallback in asm-generic, and
remove the ones for the architectures that have no overrides
at all?

Or possibly a version that can do both based on
CONFIG_ARCH_USE_QUEUED_SPINLOCKS? That would
let us remove even more architecture specific headers, but
it increases the risk of some architecture using qspinlock
when they really should not.

> > or a trivial test-and-set?
>
> If your SMP arch is halfway sane (no fwd progress issues etc..) then
> ticket should behave well and avoid the starvation/variablilty of TaS
> lock.

Ok, and I guess we still need to keep the parisc and sparc32 versions
anyway.

> The big exception there is virtualized architectures, ticket is
> absolutely horrendous for 'guests' (any fair lock is for that matter).

This might be useful information to put into the header, at least
I had no idea about this distinction.

       Arnd