linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: mingo@kernel.org, peterz@infradead.org, oleg@redhat.com
Cc: john.stultz@linaro.org, dimitrysh@google.com,
	linux-kernel@vger.kernel.org, dave@stgolabs.net,
	Davidlohr Bueso <dbueso@suse.de>
Subject: [PATCH 3/3] locking/percpu-rwsem: Avoid unnecessary writer wakeups
Date: Fri, 18 Nov 2016 10:54:37 -0800	[thread overview]
Message-ID: <1479495277-9075-4-git-send-email-dave@stgolabs.net> (raw)
In-Reply-To: <1479495277-9075-1-git-send-email-dave@stgolabs.net>

There is obviously no point in doing the wakeup if the
reader wakee task can do the active readers check on
behalf of the writer on its way to unlocking itself.

The downside is that when readers_active_check() does
return true we end up iterating the per-CPU counter twice.
This trade-off, however, does seem reasonable in that if
we are here, (i) we have already lost any hope for reader
side performance and; (ii) because this lock is used mainly
for sharing, it is not crazy to expect to have readers
incoming for the lock during this window -- therefore
sending writers right back to sleep for every reader up().

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/locking/percpu-rwsem.c | 72 ++++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index cb71201855f2..8e6fbf117f14 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -8,6 +8,44 @@
 #include <linux/sched.h>
 #include <linux/errno.h>
 
+#define per_cpu_sum(var)						\
+({									\
+	typeof(var) __sum = 0;						\
+	int cpu;							\
+	compiletime_assert_atomic_type(__sum);				\
+	for_each_possible_cpu(cpu)					\
+		__sum += per_cpu(var, cpu);				\
+	__sum;								\
+})
+
+/*
+ * Return true if the modular sum of the sem->read_count per-CPU variable is
+ * zero.  If this sum is zero, then it is stable due to the fact that if any
+ * newly arriving readers increment a given counter, they will immediately
+ * decrement that same counter.
+ */
+static bool readers_active_check(struct percpu_rw_semaphore *sem)
+{
+	if (per_cpu_sum(*sem->read_count) != 0)
+		return false;
+
+	/*
+	 * If we observed the decrement; ensure we see the entire critical
+	 * section. In the case of __readers_active_check we avoid the
+	 * critical section sync, as the writer wakee will fully re-check
+	 * to continue.
+	 */
+
+	smp_mb(); /* C matches B */
+
+	return true;
+}
+
+static bool __readers_active_check(struct percpu_rw_semaphore *sem)
+{
+	return !(per_cpu_sum(*sem->read_count) !=0);
+}
+
 int __percpu_init_rwsem(struct percpu_rw_semaphore *sem,
 			const char *name, struct lock_class_key *rwsem_key)
 {
@@ -103,41 +141,11 @@ void __percpu_up_read(struct percpu_rw_semaphore *sem)
 	__this_cpu_dec(*sem->read_count);
 
 	/* Prod writer to recheck readers_active */
-	swake_up(&sem->writer);
+	if (__readers_active_check(sem))
+		swake_up(&sem->writer);
 }
 EXPORT_SYMBOL_GPL(__percpu_up_read);
 
-#define per_cpu_sum(var)						\
-({									\
-	typeof(var) __sum = 0;						\
-	int cpu;							\
-	compiletime_assert_atomic_type(__sum);				\
-	for_each_possible_cpu(cpu)					\
-		__sum += per_cpu(var, cpu);				\
-	__sum;								\
-})
-
-/*
- * Return true if the modular sum of the sem->read_count per-CPU variable is
- * zero.  If this sum is zero, then it is stable due to the fact that if any
- * newly arriving readers increment a given counter, they will immediately
- * decrement that same counter.
- */
-static bool readers_active_check(struct percpu_rw_semaphore *sem)
-{
-	if (per_cpu_sum(*sem->read_count) != 0)
-		return false;
-
-	/*
-	 * If we observed the decrement; ensure we see the entire critical
-	 * section.
-	 */
-
-	smp_mb(); /* C matches B */
-
-	return true;
-}
-
 void percpu_down_write(struct percpu_rw_semaphore *sem)
 {
 	/* Notify readers to take the slow path. */
-- 
2.6.6

  parent reply	other threads:[~2016-11-18 18:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-18 18:54 [PATCH -tip 0/3] locking/percpu-rwsem: writer-side optimizations Davidlohr Bueso
2016-11-18 18:54 ` [PATCH 1/3] locking/percpu-rwsem: Move text file into Documentation/locking/ Davidlohr Bueso
2016-11-18 18:54 ` [PATCH 2/3] locking/percpu-rwsem: Replace bulky wait-queues with swait Davidlohr Bueso
2016-11-21 12:55   ` Oleg Nesterov
2016-11-21 17:26     ` Davidlohr Bueso
2016-12-03  2:18   ` [PATCH v2 2/3] locking/percpu-rwsem: Rework writer block/wake to not use wait-queues Davidlohr Bueso
2016-12-05  8:36     ` Peter Zijlstra
2016-12-05 11:26       ` Oleg Nesterov
2016-12-05 11:32         ` Oleg Nesterov
2016-12-05 17:37         ` Davidlohr Bueso
2016-12-05 17:19       ` Oleg Nesterov
2016-12-05 17:13     ` Oleg Nesterov
2016-11-18 18:54 ` Davidlohr Bueso [this message]
2016-11-21 12:23   ` [PATCH 3/3] locking/percpu-rwsem: Avoid unnecessary writer wakeups Oleg Nesterov
2016-11-21 12:29     ` Peter Zijlstra
2016-11-21 12:47       ` Oleg Nesterov
2016-11-21 15:07         ` Oleg Nesterov
2016-11-22  3:59           ` Davidlohr Bueso
2016-11-23 14:43             ` Oleg Nesterov

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=1479495277-9075-4-git-send-email-dave@stgolabs.net \
    --to=dave@stgolabs.net \
    --cc=dbueso@suse.de \
    --cc=dimitrysh@google.com \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@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;
as well as URLs for NNTP newsgroup(s).