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

From: Linus Torvalds <torvalds@linux-foundation.org>
To: John Ogness <john.ogness@linutronix.de>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>, Christoph Hellwig <hch@lst.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry_kill()
Date: Fri, 16 Feb 2018 16:06:36 -0800	[thread overview]
Message-ID: <CA+55aFySL+exqUJX0JKmJY4Pa0CKwn9y4Voqs1QjWY29hkzhnQ@mail.gmail.com> (raw)
In-Reply-To: <87lgfs3374.fsf@linutronix.de>

On Fri, Feb 16, 2018 at 3:49 PM, John Ogness <john.ogness@linutronix.de> wrote:
>
> After reading your initial feedback my idea was to change both
> lock_parent() and dentry_lock_inode() to not only communicate _if_ the
> lock was successful, but also if d_lock was dropped in the process. (For
> example, with a tristate rather than boolean return value.) Then callers
> would know if they needed to recheck the dentry contents.

So I think that would work well for your dentry_lock_inode() helper,
and might indeed solve my reaction to your dentry_kill() patch.

I suspect it doesn't work for lock_parent(), because that has to also
return the parent itself. So you'd have to add another way to say
"didn't need to drop dentry lock". I suspect it gets ugly real fast.

But yes, making dentry_lock_inode() return 0/1/2 for "fail/fast/slow"
(or whatever) sounds doable. And then your dentry_kill() patch can use
a "switch ()" to handle the cases, and the whole "need to revalidate"
might become pretty clear and clean.

I'd suggest you ignore lock_parent() for now. Unless you come up with
something clever.

        Linus