On 2018-02-16, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >> lock_parent() already has the problem you are referring to. Callers >> are required to recheck the dentry contents and check the returned >> parent because they do not know if the trylock succeeded. See >> d_prune_aliases(), for example. > > What are you talking about? > > lock_parent() does the nice "spin_trylock succeeded" special case. > > Yes, it will then do the "unlock dentry, do the parent first, then > re-check" too, and callers may need to worry about it. > > But that's not what I'm complaining about in your patch. You remove > the simple case, and make dentry_kill() do the "recheck in case I > dropped" every single time. dentry_lock_inode() uses the same semantics as lock_parent(). The caller does not know if the trylock succeeded. Any caller using lock_parent() must "recheck in case I dropped", just as with dentry_lock_inode(). This is what you have pointed out. > The fact that there are _other_ complex cases doesn't make it any > better. The whole "but Bobby does it too" thing is not a defense. > Would you jump off a bridge just because your friend did it? dentry_kill() calls both dentry_lock_inode() and lock_parent() in the common case. So by changing the semantics of lock_parent(), I am removing two "recheck in case I dropped" in the common case rather than just the one you pointed out. John Ogness
