On Thu, 2010-12-23 at 21:45 -0700, Gregory Haskins wrote:> Hey Steve,> > >>> On 12/23/2010 at 05:47 PM, in message <20101223225116.729981172@goodmis.org>,> Steven Rostedt <rostedt@goodmis.org> wrote: > > 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.> > Just a quick note to say I am a bit skeptical of this patch. I know you are offline next week, so lets plan on hashing it out after the new year before I ack it.>

We shouldn't be too quick to merely rip this out without a littlethinking. Clearly this is broken, however the intent was clear.

That being that if a waiter is spinning, we don't need to wake it up.

The wake up path is substantially more than a barrier; it includes abarrier as well as grabbing the task_rq_lock only to find that the taskis oncpu. Then various accounting is updated, etc.

We know definitively that a waiter can only spin if the owner is oncpu,by definition of adaptive spinning. We also know that only the ownercan release the lock to a waiter (spinning or not). So it seems clearthat avoiding unnecessary contention on the rq lock would be a GoodThing(tm).

Perhaps this cannot be done safely, but if you saw an improvement indbench by merely removing a barrier, imagine the improvement by removingthe contention on the lock.