Message ID | 20180513213017.31269-5-viro@ZenIV.linux.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, May 13, 2018 at 10:30:07PM +0100, Al Viro wrote: > From: Al Viro <viro@zeniv.linux.org.uk> > > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> It looks a little cleaner, so this is fine, but is there also any other reason? A little changelog would be nice.
On Mon, May 14, 2018 at 10:41:02AM +0200, Christoph Hellwig wrote: > On Sun, May 13, 2018 at 10:30:07PM +0100, Al Viro wrote: > > From: Al Viro <viro@zeniv.linux.org.uk> > > > > Cc: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > > It looks a little cleaner, so this is fine, but is there also any > other reason? A little changelog would be nice. The same changelog would be duplicated in a bunch of commits in that series and I'm not sure how much is too much. Let me put it that way: * d_splice_alias() has better calling conventions for use in ->lookup() - it tends to reduce the number of branches nicely. * d_add() in ->lookup() is no-go for anything exportable; we did conversion for exported stuff back then, and it's documented in Documentation/filesystems/nfs/Exporting, but people fail to RTFM when converting more filesystems to exportability. Preempting that kind of bugs is a good idea. * the fewer stray d_add() we have sitting around, the better - each needs a proof that this particular invocation won't end up with multiple aliases of a directory inode. d_splice_alias() is *NOT* a universal replacement, but in a lot of ->lookup() instances we can use it sanely; it can't be done quite blindly (the stuff that hangs something off the dentry needs a careful look), but most of the time it's fine - on any filesystem that doesn't play with ->d_fsdata, for starters. And in all cases such replacement in ->lookup() is "it's no worse than before" - it's just that some cases need more than just that conversion before they can be made exportable (see e.g. 9p for example of that kind of extra work; res = d_splice_alias(inode, dentry); if (!res) v9fs_fid_add(dentry, fid); else if (!IS_ERR(res)) v9fs_fid_add(res, fid); else p9_client_clunk(fid); in there, when we need to decide which dentry to slap the fid onto) The bunch in this series is the trivial part of conversions. Next group is where a bit more attention is needed and the last is procfs nest of horrors. I'm honestly not sure how much of the above makes sense as a boilerplate to go into all those commits. Suggestions?
diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c index ce4785fd81c6..a51425634f65 100644 --- a/fs/freevxfs/vxfs_lookup.c +++ b/fs/freevxfs/vxfs_lookup.c @@ -193,13 +193,9 @@ vxfs_lookup(struct inode *dip, struct dentry *dp, unsigned int flags) return ERR_PTR(-ENAMETOOLONG); ino = vxfs_inode_by_name(dip, dp); - if (ino) { + if (ino) ip = vxfs_iget(dip->i_sb, ino); - if (IS_ERR(ip)) - return ERR_CAST(ip); - } - d_add(dp, ip); - return NULL; + return d_splice_alias(ip, dp); } /**