Messages in this thread Patch in this message | | | Date | Thu, 23 Dec 2010 17:47:58 -0500 | From | Steven Rostedt <> | Subject | [RFC][RT][PATCH 3/4] rtmutex: Revert Optimize rt lock wakeup |
| |
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
| |