From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Lai Jiangshan <laijs@cn.fujitsu.com>,
Gregory Haskins <ghaskins@novell.com>,
Peter Morreale <pmorreale@novell.com>
Subject: [RFC][RT][PATCH 3/4] rtmutex: Revert Optimize rt lock wakeup
Date: Thu, 23 Dec 2010 17:47:58 -0500 [thread overview]
Message-ID: <20101223225116.729981172@goodmis.org> (raw)
In-Reply-To: 20101223224755.078983538@goodmis.org
[-- Attachment #1: 0003-rtmutex-Revert-Optimize-rt-lock-wakeup.patch --]
[-- Type: text/plain, Size: 2377 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
The commit: rtmutex: Optimize rt lock wakeup
Does not do what it was suppose to do.
This is because the adaptive waiter sets its state to TASK_(UN)INTERRUPTIBLE
before going into the loop. Thus, the test in wakeup_next_waiter()
will always fail on an adaptive waiter, as it only tests to see if
the pending waiter never has its state set ot TASK_RUNNING unless
something else had woke it up.
The smp_mb() added to make this test work is just as expensive as
just calling wakeup. And since we we fail to wake up anyway, we are
doing both a smp_mb() and wakeup as well.
I tested this with dbench and we run faster without this patch.
I also tried a variant that instead fixed the loop, to change the state
only if the spinner was to go to sleep, and that still did not show
any improvement.
Cc: Gregory Haskins <ghaskins@novell.com>
Cc: Peter Morreale <pmorreale@novell.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/rtmutex.c | 29 ++---------------------------
1 files changed, 2 insertions(+), 27 deletions(-)
diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index 318d7ed..e218873 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -554,33 +554,8 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)
*/
if (!savestate)
wake_up_process(pendowner);
- else {
- /*
- * We can skip the actual (expensive) wakeup if the
- * waiter is already running, but we have to be careful
- * of race conditions because they may be about to sleep.
- *
- * The waiter-side protocol has the following pattern:
- * 1: Set state != RUNNING
- * 2: Conditionally sleep if waiter->task != NULL;
- *
- * And the owner-side has the following:
- * A: Set waiter->task = NULL
- * B: Conditionally wake if the state != RUNNING
- *
- * As long as we ensure 1->2 order, and A->B order, we
- * will never miss a wakeup.
- *
- * Therefore, this barrier ensures that waiter->task = NULL
- * is visible before we test the pendowner->state. The
- * corresponding barrier is in the sleep logic.
- */
- smp_mb();
-
- /* If !RUNNING && !RUNNING_MUTEX */
- if (pendowner->state & ~TASK_RUNNING_MUTEX)
- wake_up_process_mutex(pendowner);
- }
+ else
+ wake_up_process_mutex(pendowner);
rt_mutex_set_owner(lock, pendowner, RT_MUTEX_OWNER_PENDING);
--
1.7.2.3
next prev parent reply other threads:[~2010-12-23 22:51 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-23 22:47 [RFC][RT][PATCH 0/4] rtmutex: Simplify PI code Steven Rostedt
2010-12-23 22:47 ` [RFC][RT][PATCH 1/4] rtmutex: Only save lock depth once in spin_slowlock Steven Rostedt
2010-12-23 22:47 ` [RFC][RT][PATCH 2/4] rtmutex: Try to take lock early in rt_spin_lock_slowlock() Steven Rostedt
2010-12-23 22:47 ` Steven Rostedt [this message]
2010-12-24 4:45 ` [RFC][RT][PATCH 3/4] rtmutex: Revert Optimize rt lock wakeup Gregory Haskins
2010-12-24 4:54 ` Steven Rostedt
2010-12-28 14:06 ` Gregory Haskins
2011-01-03 19:06 ` Steven Rostedt
2011-01-03 20:22 ` Steven Rostedt
2011-01-04 15:19 ` Peter W. Morreale
2011-01-04 15:47 ` Steven Rostedt
2011-01-04 17:15 ` Peter W. Morreale
2011-01-04 17:27 ` Steven Rostedt
2011-01-04 17:45 ` Peter W. Morreale
2011-01-04 18:06 ` Steven Rostedt
2011-01-04 20:48 ` Peter W. Morreale
2011-01-04 17:24 ` Peter W. Morreale
2011-01-10 14:49 ` Gregory Haskins
[not found] ` <4D13DF250200005A000793E1@novell.com>
2010-12-24 15:47 ` Peter W. Morreale
2010-12-23 22:47 ` [RFC][RT][PATCH 4/4] rtmutex: Ensure only the top waiter or higher priority task can take the lock Steven Rostedt
2011-01-04 4:02 ` Steven Rostedt
2011-01-05 7:12 ` Lai Jiangshan
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=20101223225116.729981172@goodmis.org \
--to=rostedt@goodmis.org \
--cc=ghaskins@novell.com \
--cc=laijs@cn.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=pmorreale@novell.com \
--cc=tglx@linutronix.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.