On Mon, Jun 20, 2016 at 06:35:30AM +0100, Al Viro wrote: > On Mon, Jun 20, 2016 at 01:34:14PM +0900, J. R. Okajima wrote: > > > > Al Viro: > > > How would processB get past d_wait_lookup()? It would have to have > > > > By the first d_unhashed() test in the loop, processB doesn't reach > > d_wait_lookup(). > > Huh? What first d_unhashed()... <stares> > > That check is definitely bogus and I'm completely at loss as to WTF is it > doing there. Thanks for catching that; this kind of idiotic braino can > escape notice when rereading the code again and again, unfortunately ;-/ > > Fixed, will push to Linus tonight or tomorrow. FWIW, I understand how it got there; it was a garbage from cut'n'paste from lockless primary hash lookups (cut'n'paste was for the sake of "compare the name" logics). It was absolutely wrong - dentry is never added to the primary hash until it has been removed from in-lookup one. And we are walking the in-lookup hash chain with its bitlock held, so there's no chance of that. In effect that junk prevented d_alloc_parallel() from *ever* spotting in-lookup matches. What's more, removing it has instantly uncovered another bug in the match-handling code - dget() done under the chain bitlock, which nests inside ->d_lock. Trivially fixed, of course (we just hold rcu_read_lock() through the in-lookup hash search and instead of dget() while holding the chain bitlock do lockref_get_not_dead() after dropping the bitlock), but... *ouch* It's going through the local tests right now; seems to be OK so far; I'll send a pull request once it's through those. But this demonstrates why RTFS (and by somebody other than the author of TFS being R) is really, _really_ important. I have read through that loop many times and kept missing that turdlet ;-/ Al, wearing a brown paperbag ;-/ -- 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
