Re: [PATCH v1 31/38] nfs: replace d_add with d_splice_alias in atomic_open

[Jeff Layton <jlayton@poochiereds.net>]

On Thu, 19 Nov 2015 15:06:51 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Tue, Nov 17, 2015 at 06:52:53AM -0500, Jeff Layton wrote:
> > From: Peng Tao <tao.peng@primarydata.com>
> > 
> > It's a trival change but follows knfsd export document that asks
> > for d_splice_alias during lookup.
> 
> This is a bug even before you start exporting, isn't it?
> 
> OK, I see, in the atomic_open case we're probably only dealing with a
> positive dentry for a regular file at this point, in which case
> d_splice_alias is really just d_add....
> 
> I'm not sure this patch is really necessary, then.
> 
> --b.
> 
> 
> > 
> > Signed-off-by: Peng Tao <tao.peng@primarydata.com>
> > ---
> >  fs/nfs/dir.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index ce5a21861074..a4df1137878e 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -1534,7 +1534,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
> >  		switch (err) {
> >  		case -ENOENT:
> >  			d_drop(dentry);
> > -			d_add(dentry, NULL);
> > +			d_splice_alias(NULL, dentry);
> >  			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
> >  			break;
> >  		case -EISDIR:
> > -- 
> > 2.4.3

Ahh right -- good point. d_splice_alias does this:

        if (!inode) {
                __d_instantiate(dentry, NULL);
                goto out;
        }
[...]
out:
	security_d_instantiate(dentry, inode);
        d_rehash(dentry);
        return NULL;

...which is exactly what d_add does. So yeah, that change isn't
strictly necessary. Still, it might be more future-proof to use
d_splice_alias there if the semantics ever change...

-- 
Jeff Layton <jlayton@poochiereds.net>