On Thu, Aug 29, 2013 at 8:12 PM, Waiman Long <waiman.long@xxxxxx> wrote:
> On 08/29/2013 07:42 PM, Linus Torvalds wrote:
>>
>> Waiman? Mind looking at this and testing? Linus
>
> Sure, I will try out the patch tomorrow morning and see how it works out for
> my test case.
Ok, thanks, please use this slightly updated patch attached here.
It improves on the previous version in actually handling the
"unlazy_walk()" case with native lockref handling, which means that
one other not entirely odd case (symlink traversal) avoids the d_lock
contention.
It also refactored the __d_rcu_to_refcount() to be more readable, and
adds a big comment about what the heck is going on. The old code was
clever, but I suspect not very many people could possibly understand
what it actually did. Plus it used nested spinlocks because it wanted
to avoid checking the sequence count twice. Which is stupid, since
nesting locks is how you get really bad contention, and the sequence
count check is really cheap anyway. Plus the nesting *really* didn't
work with the whole lockref model.
With this, my stupid thread-lookup thing doesn't show any spinlock
contention even for the "look up symlink" case.
It also avoids the unnecessary aligned u64 for when we don't actually
use cmpxchg at all.
It's still one single patch, since I was working on lots of small
cleanups. I think it's pretty close to done now (assuming your testing
shows it performs fine - the powerpc numbers are promising, though),
so I'll split it up into proper chunks rather than random commit
points. But I'm done for today at least.
NOTE NOTE NOTE! My test coverage really has been pretty pitiful. You
may hit cases I didn't test. I think it should be *stable*, but maybe
there's some other d_lock case that your tuned waiting hid, and that
my "fastpath only for unlocked case" version ends up having problems
with.
Linus
Attachment:
patch.diff
Description: Binary data
