From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:44926 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752108AbcCJDee (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Wed, 9 Mar 2016 22:34:34 -0500
Date: Thu, 10 Mar 2016 03:34:28 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Drokin, Oleg" <oleg.drokin@intel.com>
Cc: "Dilger, Andreas" <andreas.dilger@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"<linux-fsdevel@vger.kernel.org>" <linux-fsdevel@vger.kernel.org>,
	Mark Fasheh <mfasheh@suse.com>
Subject: Re: races in ll_splice_alias() and elsewhere (ext4, ocfs2)
Message-ID: <20160310033428.GH17997@ZenIV.linux.org.uk>
References: <20160308160537.GV17997@ZenIV.linux.org.uk>
 <498D5A19-9E55-48D7-B5CF-34CA5769FF7F@intel.com>
 <20160308211148.GX17997@ZenIV.linux.org.uk>
 <A5CA4682-77B1-4DF4-A784-7C7C2FAFC9AF@intel.com>
 <20160309003416.GY17997@ZenIV.linux.org.uk>
 <7C3EBB6F-54AC-4744-BEC1-33EA82216F85@intel.com>
 <20160309012658.GZ17997@ZenIV.linux.org.uk>
 <34C2B1C3-2B7F-490B-A03A-3BCDDFC8BE48@intel.com>
 <20160310022041.GF17997@ZenIV.linux.org.uk>
 <73CAA5F1-C749-42ED-9A34-D96B5854A9B9@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <73CAA5F1-C749-42ED-9A34-D96B5854A9B9@intel.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Mar 10, 2016 at 03:08:57AM +0000, Drokin, Oleg wrote:
> > Note, BTW, that d_splice_alias() will not look for aliases in case of
> > non-directories - for those it's the same d_add(), since there we can
> > legitimately have many dentry aliases over the same inode.  For directories
> > we *can't*.
> 
> Ah! Btw this highlights the missed case for d_exact_alias + d_splice_alias
> in Lustre with current collection of patches you carry.
> 
> Suppose we have a dentry pointing to an inode all nicely covered by a lustre
> lock (to ensure it is valid).
> Now some other client does something that invalidates the name (rename,
> or just open(O_CREAT) even), this causes our local lock to disappear
> and causes the dentry to be declared "lustre invalid" via
> ll_md_blocking_ast()->ll_invalidate_aliases()->d_lustre_invalidate()
> 
> This sets the "invalid" flag in __d_lustre_invalidate() and
> also would try to unhash the dentry in some cases:
>         if (d_count(dentry) == 0 && !(dentry->d_flags & DCACHE_DISCONNECTED))
>                 __d_drop(dentry);
> 
> Now if none of those conditions hit, the dentry stays hashed.
> But the problem is if it's not a directory, then d_splice_alias would
> also ignore it, while d_exact_alias would ignore it due to it being still hashed
> and this dentry would just hang around uselessly taking RAM.
> 
> Is there an easy way to rectify this, I wonder?

Wait a minute.  If it's hashed, has the right name and the right parent,
why the hell are we calling ->lookup() on a new dentry in the first place?
Why hadn't we simply picked it from dcache?