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]:36183 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751592AbcCIB1C (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Tue, 8 Mar 2016 20:27:02 -0500
Date: Wed, 9 Mar 2016 01:26:58 +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>
Subject: Re: races in ll_splice_alias()
Message-ID: <20160309012658.GZ17997@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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7C3EBB6F-54AC-4744-BEC1-33EA82216F85@intel.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Wed, Mar 09, 2016 at 12:53:20AM +0000, Drokin, Oleg wrote:

> I have not tried this exact one yet (will try in a moment).
> But it appears we do need ll_d_init() call if we got a hit
> in d_exact_alias().
> A particular test case fails due to lack of the d_fsdata being null.

Very interesting.  I'd suggest checking if we hit that d_instantiate()
in your namei.c with NULL ->d_fsdata.  That really shouldn't happen, AFAICS...


> @@ -374,9 +368,9 @@ struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
>  		}
>  	}
>  	rc = ll_d_init(de);
> +	d_add(de, inode);
>  	if (rc < 0)
>  		return ERR_PTR(rc);
> -	d_add(de, inode);

Uhh...  Why would you reorder them like that?

> Also any particular reason to prefer d_splice_alias over d_add? Did not we already determine there
> were no other aliases.

No unhashed aliases with the same name and parent.  Seriously, d_splice_alias()
will do exactly the same thing as d_add() except for the situations where
d_add() would've created multiple dentries over the same directory inode.
And it's not more costly outside of that case...