Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry_kill()

From: Linus Torvalds
Date: Fri Feb 16 2018 - 18:31:30 EST


On Fri, Feb 16, 2018 at 3:05 PM, John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
>
> 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.

Ok, that would be lovely, but doesn't that end up being a nasty patch?
You can't just move the trylock into the caller, since then you need
to move all the other stuff too?

Or were you planning on splitting lock_parent() into two, for the
"fast case vs compex case"?

Or maybe I'm entirely missing something and we're miscommunicating.

I'm actually not so much worried about the cost of re-checking (the
real cost tends to be the locked cycle itself) as I am about the code
looking understandable. Your d_delete() patch didn't make me go "that
looks more complicated", probably partl ybecause of the nice helper
function.

So it may be that my dislike of the "re-check after possibly dropping
the lock" is not really about the re-checking, but about just how it
made that function look much more complicated.

Linus