linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "George Spelvin" <linux@horizon.com>
To: peterz@infradead.org, waiman.long@hp.com
Cc: akpm@linux-foundation.org, andi@firstfloor.org, arnd@arndb.de,
	aswin@hp.com, hpa@zytor.com, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux@horizon.com,
	mingo@redhat.com, paulmck@linux.vnet.ibm.com,
	raghavendra.kt@linux.vnet.ibm.com, riel@redhat.com,
	rostedt@goodmis.org, scott.norton@hp.com, tglx@linutronix.de,
	tim.c.chen@linux.intel.com, torvalds@linux-foundation.org,
	walken@google.com, x86@kernel.org
Subject: Re: [PATCH v11 0/4] Introducing a queue read/write lock implementation
Date: 1 Feb 2014 05:38:20 -0500	[thread overview]
Message-ID: <20140201103820.32043.qmail@science.horizon.com> (raw)
In-Reply-To: <20140131194718.GO5002@laptop.programming.kicks-ass.net>

Peter Zijlstra wrote:
> On Fri, Jan 31, 2014 at 01:59:02PM -0500, Waiman Long wrote:
>> Using a ticket lock instead will have the same scalability problem as the
>> ticket spinlock as all the waiting threads will spin on the lock cacheline
>> causing a lot of cache bouncing traffic. That is the reason why I want to
>> replace ticket spinlock with queue spinlock.

> But but but, just fix such heavily contended locks. Don't make sensible
> code that is lightly contended run slower because of it.

While I agree that zero slowdown for "good" code is the goal, it is
impossible for the kernel to consist of only "good" code.

In particular, obscure error conditions causing locked regions to take
much longer than expected will never be completely expurgated; there's
a point where you just say "I'm not working for a week to save 10 people
per year a 2-minute stall."

What Waiman noted is that ticket locks take O(n^2) cache line transfers
to clear n waiters from the queue.  (Each write must be broadcast to
each spinning reader.)  So if you *do* get most of a large multiprocessor
piled up on a ticket lock, the performance can be disastrous.

It can conceivably send a large system into a "congestion collapse"
where the queue never clears.  And it can affect processors (such as
other partitions of a large machine) that aren't even contending for
the lock.

The MCS lock is O(1) per release and O(n) to clear n waiters.  This is
a noticeable improvement on 4- or 8-way contention, and (Waiman reports)
a huge improvement on 50-way and up.

Yes, if such contention occurs with any frequency at all, it should be
fixed, but it does seem worth mitigating problems in the meantime.

(As an aside, I have in the past heard people criticize the Linux kernel
for being optimized for the average case at the expense of worst-case
corner cases.)

Are we agreed that *not* improving highly-contended performance on the
grounds that it would discourage other optimization is as stupid as not
wearing a seat-belt because that would discourage more careful driving?


While I do think *some* benchmarking on smaller SMP systems is wanted,
given that Waiman has mananged to make the *uncontended* case faster,
and *that* is by far the most common case, it's quite plausible that it
will turn out to be a net performance improvement on 4- and 8-way systems.

  reply	other threads:[~2014-02-01 10:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-24  4:28 [PATCH v11 0/4] Introducing a queue read/write lock implementation Waiman Long
2014-01-24  4:28 ` [PATCH v11 1/4] qrwlock: A " Waiman Long
2014-01-24  8:25   ` Peter Zijlstra
2014-01-25  4:39     ` Waiman Long
2014-01-24  4:28 ` [PATCH v11 2/4] qrwlock, x86: Enable x86 to use queue read/write lock Waiman Long
2014-01-24  4:28 ` [PATCH v11 3/4] qrwlock, x86: Add char and short as atomic data type in x86 Waiman Long
2014-01-24  4:28 ` [PATCH v11 4/4] qrwlock: Use the mcs_spinlock helper functions for MCS queuing Waiman Long
2014-01-24  8:26   ` Peter Zijlstra
2014-01-25  4:30     ` Waiman Long
2014-01-30 13:04 ` [PATCH v11 0/4] Introducing a queue read/write lock implementation Peter Zijlstra
2014-01-30 15:17   ` Peter Zijlstra
2014-01-30 15:43     ` Waiman Long
2014-01-30 15:50       ` Waiman Long
2014-01-30 15:53         ` Peter Zijlstra
2014-01-30 15:44     ` Peter Zijlstra
2014-01-30 17:52       ` Will Deacon
2014-01-30 18:05         ` Peter Zijlstra
2014-01-30 18:11           ` Will Deacon
2014-01-30 18:16             ` Peter Zijlstra
2014-01-31  9:26     ` Peter Zijlstra
2014-01-31 10:03       ` George Spelvin
2014-01-31 10:17         ` Peter Zijlstra
2014-01-31 11:30           ` Peter Zijlstra
2014-02-01 23:22           ` Paul E. McKenney
2014-01-31 18:59       ` Waiman Long
2014-01-31 19:47         ` Peter Zijlstra
2014-02-01 10:38           ` George Spelvin [this message]
2014-01-31 20:14         ` Peter Zijlstra
2014-01-31 21:09           ` Waiman Long
2014-02-01  1:29             ` Davidlohr Bueso
2014-02-02  9:03             ` Ingo Molnar
2014-02-03 11:38               ` Peter Zijlstra
2014-02-06  3:08               ` Waiman Long

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=20140201103820.32043.qmail@science.horizon.com \
    --to=linux@horizon.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=arnd@arndb.de \
    --cc=aswin@hp.com \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=scott.norton@hp.com \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=waiman.long@hp.com \
    --cc=walken@google.com \
    --cc=x86@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).