On Sat, Feb 24, 2018 at 12:22:48AM +0000, Al Viro wrote: > On Fri, Feb 23, 2018 at 01:35:52PM -0800, Linus Torvalds wrote: > > > This is too subtle, and your fix to check d_lockref.count < 0 sounds > > wrong to me. If it's really gone, maybe it has been reused and the > > refcount is positive again, but it's something else than a dentry > > entirely? > > > > Hmm. > > > > No, you extended the rcu read section, so I guess your patch is fine. > > And lock_parent already has that pattern, soiit's not new. > > > > Ok, I agree, looks like lock_parent should just re-check that thing > > that it already checked earler, but that now might be true again > > because of we dropped d_lock. > > IMO that's the right thing for backports; whether we keep it after > the getting rid of trylock loops is a different question. Note that > the only case where we do not have __dentry_kill() prevention > guaranteed by the caller (either by holding a reference, or by > holding onto ->i_lock all along) is in shrink_dentry_list(). > And there we have more than enough of other subtle crap. > > Moreover, there we have a good reason to treat "it had been moved" > as "kick it off the shrink list and free if it's already dead", > which might simplify the things. Below is a stab at that: FWIW, a variant of that series is in #work.dcache; it's almost certainly not the final (I want to clean the things up and probably reorder them as well) and it needs one hell of profiling and review. It seems to work, and I don't see any obvious performance regressions (everything seems to be within normal noise), but I'd like it beaten up a lot more. Parts are from John's series, parts are rewritten as discussed upthread. As the result, trylock loops are gone and no new retry loops had been added in their place - lock_parent() still has one, but that's it. I'll play with cleaning up and reordering tomorrow after I get some sleep. In the meanwhile, the current state of that set is at git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.dcache
