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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
