public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: Waiman Long <longman@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present
Date: Sun, 26 Feb 2017 10:49:24 -0800	[thread overview]
Message-ID: <20170226184924.GE5126@linux-80c1.suse> (raw)
In-Reply-To: <1487786634-22641-2-git-send-email-longman@redhat.com>

On Wed, 22 Feb 2017, Waiman Long wrote:

>We can safely check the wait_list to see if waiters are present without
>lock when there are spinners to fall back on in case we miss a waiter.
>The advantage is that we can save a pair of spin_lock/unlock calls
>when the wait_list is empty. This translates to a reduction in latency
>and hence slightly better performance.

This benefit is only seen in (rare) situations where there are only
writers with short hold times, no? I don't really have any objection
as I doubt the additional load will have any impact on the common case,
but it would still be nice to have more data for other benchmarks where
the lock is at least shared at times -- ie: a good thing to measure is
also fault, mmap related benchmarks.

>+		/*
>+		 * Normally checking wait_list without wait_lock isn't safe
>+		 * as we may miss an incoming waiter. With spinners present,
>+		 * however, we have someone to fall back on in case that
>+		 * happens. This can save a pair of spin_lock/unlock calls
>+		 * when there is no waiter.
>+		 */

I would drop the last part regarding saving the spin_lock, it should be
evident from the code.

Thanks,
Davidlohr

  reply	other threads:[~2017-02-26 19:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 18:03 [PATCH-tip 0/3] locking/rwsem: Minor twists to improve rwsem performance Waiman Long
2017-02-22 18:03 ` [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present Waiman Long
2017-02-26 18:49   ` Davidlohr Bueso [this message]
2017-02-27 15:02     ` Waiman Long
2017-02-22 18:03 ` [PATCH-tip 2/3] locking/rwsem: move down rwsem_down_read_failed() Waiman Long
2017-02-26 18:33   ` Davidlohr Bueso
2017-02-27 14:22     ` Waiman Long
2017-02-22 18:03 ` [PATCH-tip 3/3] locking/rwsem: Stop active read lock ASAP Waiman Long
2017-02-26 18:58   ` Davidlohr Bueso
2017-02-27 15:07     ` 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=20170226184924.GE5126@linux-80c1.suse \
    --to=dave@stgolabs.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.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