Re: [PATCH][RFC]: mutex: adaptive spin

Linus Torvalds
2009-01-06T22:23:43Z

On Tue, 6 Jan 2009, Peter Zijlstra wrote:
>
> One of the things the previous patch did wrong was that it never tracked
> the owner in the mutex fast path -- I initially didn't notice because I
> had all debugging infrastructure enabled, and that short circuits all
> the fast paths.

Why even worry?

Just set the owner non-atomically. We can consider a NULL owner in the
slow-path to be a "let's schedule" case. After all, it's just a
performance tuning thing after all, and the window is going to be very
small, so it won't even be relevant from a performance tuning angle.

So there's _no_ reason why the fast-path can't just set owner, and no
reason to disable preemption.

I also think the whole preempt_disable/enable around the
mutex_spin_or_schedule() is pure garbage.

In fact, I suspect that's the real bug you're hitting: you're enabling
preemption while holding a spinlock. That is NOT a good idea.

So

- remove all the "preempt_disable/enable" crud. It's wrong.

- remove the whole

+ if (!owner)
+ return;

thing. It's also wrong. Just go to the schedule case for that.

It all looks like unnecessary complexity that you added, and I think
_that_ is what bites you.

Aim for _simple_. Not clever. Not complex. What you should aim for is to
keep the _exact_ same code that we had before in mutex, and the absolute
*only* change is replacing that "unlock+schedule()+relock" sequence with
"mutex_spin_or_schedule()".

That should be your starting point.

Yeah, you'll need to set the owner for the fast case, but there are no
locking or preemption issues there. Just forget them. You're confusing
yourself and the code by trying to look for problems that aren't
relevant.

Linus