| Message ID | 1447761180-4250-32-git-send-email-jeff.layton@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 19, 2015 at 3:06 PM, 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. > d_splice_alias() also instantly reduces to d_add() whenever the 'inode' argument is NULL. Cheers Trond -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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...
On Thu, Nov 19, 2015 at 03:59:31PM -0500, Jeff Layton wrote: > 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... Could be, I don't know. May as well apply it (or not) independently of this series, though. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: