From: Catalin Marinas <catalin.marinas@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Stafford Horne" <shorne@gmail.com>,
"Guo Ren" <guoren@kernel.org>,
"Christoph Müllner" <christophm30@gmail.com>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Anup Patel" <anup@brainfault.org>,
linux-riscv <linux-riscv@lists.infradead.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Guo Ren" <guoren@linux.alibaba.com>,
"Will Deacon" <will@kernel.org>, "Arnd Bergmann" <arnd@arndb.de>,
jonas@southpole.se, stefan.kristiansson@saunalahti.fi
Subject: Re: [RFC][PATCH] locking: Generic ticket-lock
Date: Thu, 15 Apr 2021 10:02:18 +0100 [thread overview]
Message-ID: <20210415090215.GA1015@arm.com> (raw)
In-Reply-To: <YHf00hgpB5C20tH3@hirez.programming.kicks-ass.net>
(fixed Will's email address)
On Thu, Apr 15, 2021 at 10:09:54AM +0200, Peter Zijlstra wrote:
> On Thu, Apr 15, 2021 at 05:47:34AM +0900, Stafford Horne wrote:
> > > How's this then? Compile tested only on openrisc/simple_smp_defconfig.
> >
> > I did my testing with this FPGA build SoC:
> >
> > https://github.com/stffrdhrn/de0_nano-multicore
> >
> > Note, the CPU timer sync logic uses mb() and is a bit flaky. So missing mb()
> > might be a reason. I thought we had defined mb() and l.msync, but it seems to
> > have gotten lost.
> >
> > With that said I could test out this ticket-lock implementation. How would I
> > tell if its better than qspinlock?
>
> Mostly if it isn't worse, it's better for being *much* simpler. As you
> can see, the guts of ticket is like 16 lines of C (lock+unlock) and you
> only need the behaviour of atomic_fetch_add() to reason about behaviour
> of the whole thing. qspinlock OTOH is mind bending painful to reason
> about.
>
> There are some spinlock tests in locktorture; but back when I had a
> userspace copy of the lot and would measure min,avg,max acquire times
> under various contention loads (making sure to only run a single task
> per CPU etc.. to avoid lock holder preemption and other such 'fun'
> things).
>
> It took us a fair amount of work to get qspinlock to compete with ticket
> for low contention cases (by far the most common in the kernel), and it
> took a fairly large amount of CPUs for qspinlock to really win from
> ticket on the contended case. Your hardware may vary. In particular the
> access to the external cacheline (for queueing, see the queue: label in
> queued_spin_lock_slowpath) is a pain-point and the relative cost of
> cacheline misses for your arch determines where (and if) low contention
> behaviour is competitive.
>
> Also, less variance (the reason for the min/max measure) is better.
> Large variance is typically a sign of fwd progress trouble.
IIRC, one issue we had with ticket spinlocks on arm64 was on big.LITTLE
systems where the little CPUs were always last to get a ticket when
racing with the big cores. That was with load/store exclusives (LR/SC
style) and would have probably got better with atomics but we moved to
qspinlocks eventually (the Juno board didn't have atomics).
(leaving the rest of the text below for Will's convenience)
> That's not saying that qspinlock isn't awesome, but I'm arguing that you
> should get there by first trying all the simpler things. By gradually
> increasing complexity you can also find the problem spots (for your
> architecture) and you have something to fall back to in case of trouble.
>
> Now, the obvious selling point of qspinlock is that due to the MCS style
> nature of the thing it doesn't bounce the lock around, but that comes at
> a cost of having to use that extra cacheline (due to the kernel liking
> sizeof(spinlock_t) == sizeof(u32)). But things like ARM64's WFE (see
> smp_cond_load_acquire()) can shift the balance quite a bit on that front
> as well (ARM has a similar thing but less useful, see it's spinlock.h
> and look for wfe() and dsb_sev()).
>
> Once your arch hits NUMA, qspinlock is probably a win. However, low
> contention performance is still king for most workloads. Better high
> contention behaviour is nice.
--
Catalin
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-04-15 9:02 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-24 10:14 [PATCH] riscv: locks: introduce ticket-based spinlock implementation guoren
2021-03-24 11:09 ` Peter Zijlstra
2021-03-24 12:10 ` Guo Ren
[not found] ` <CAM4kBBK7_s9U2vJbq68yC8WdDEfPQTaCOvn1xds3Si5B-Wpw+A@mail.gmail.com>
2021-03-24 12:23 ` Peter Zijlstra
2021-03-24 12:24 ` Guo Ren
2021-03-24 12:31 ` Peter Zijlstra
2021-03-24 12:28 ` Anup Patel
2021-03-24 12:37 ` Peter Zijlstra
2021-03-24 12:53 ` Anup Patel
2021-04-11 21:11 ` Palmer Dabbelt
2021-04-12 13:32 ` Christoph Müllner
2021-04-12 14:51 ` Peter Zijlstra
2021-04-12 21:21 ` Christoph Müllner
2021-04-12 17:33 ` Palmer Dabbelt
2021-04-12 21:54 ` Christoph Müllner
2021-04-13 8:03 ` Peter Zijlstra
2021-04-13 8:17 ` Peter Zijlstra
2021-04-14 2:26 ` Guo Ren
2021-04-14 7:08 ` Peter Zijlstra
2021-04-14 9:05 ` Peter Zijlstra
2021-04-14 10:16 ` [RFC][PATCH] locking: Generic ticket-lock Peter Zijlstra
2021-04-14 12:39 ` Guo Ren
2021-04-14 12:55 ` Peter Zijlstra
2021-04-14 13:08 ` Peter Zijlstra
2021-04-14 15:59 ` David Laight
2021-04-14 12:45 ` Peter Zijlstra
2021-04-14 21:02 ` Stafford Horne
2021-04-14 20:47 ` Stafford Horne
2021-04-15 8:09 ` Peter Zijlstra
2021-04-15 9:02 ` Catalin Marinas [this message]
2021-04-15 9:22 ` Will Deacon
2021-04-15 9:24 ` Peter Zijlstra
2021-04-19 17:35 ` Will Deacon
2021-04-23 6:44 ` Palmer Dabbelt
2021-04-13 9:22 ` [PATCH] riscv: locks: introduce ticket-based spinlock implementation Christoph Müllner
2021-04-13 9:30 ` Catalin Marinas
2021-04-13 9:55 ` Christoph Müllner
2021-04-14 0:23 ` Guo Ren
2021-04-14 9:17 ` Catalin Marinas
2021-04-13 9:35 ` Peter Zijlstra
2021-04-13 10:25 ` Christoph Müllner
2021-04-13 10:45 ` Catalin Marinas
2021-04-13 10:54 ` David Laight
2021-04-14 5:54 ` Guo Ren
2021-04-13 11:04 ` Christoph Müllner
2021-04-13 13:19 ` Guo Ren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210415090215.GA1015@arm.com \
--to=catalin.marinas@arm.com \
--cc=anup@brainfault.org \
--cc=arnd@arndb.de \
--cc=christophm30@gmail.com \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=jonas@southpole.se \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=peterz@infradead.org \
--cc=shorne@gmail.com \
--cc=stefan.kristiansson@saunalahti.fi \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).