Re: [PATCH] dcache: fix d_splice_alias handling of aliases

From: J. Bruce Fields
Date: Fri Jan 17 2014 - 13:04:59 EST


On Fri, Jan 17, 2014 at 10:04:30AM +0000, Steven Whitehouse wrote:
> Hi,
>
> On Thu, 2014-01-16 at 13:51 -0500, J. Bruce Fields wrote:
> > On Thu, Jan 16, 2014 at 11:54:07AM -0500, Bob Peterson wrote:
> > > ----- Original Message -----
> > > | Something like this?
> > > (snip)
> > > | @@ -779,6 +782,11 @@ static struct dentry *__gfs2_lookup(struct inode *dir,
> > > | struct dentry *dentry,
> > > | }
> > > |
> > > | d = d_splice_alias(inode, dentry);
> > > | + if (IS_ERR(d)) {
> > > | + iput(inode);
> > > | + gfs2_glock_dq_uninit(&gh);
> > > | + return ERR_PTR(error);
> > >
> > > ---------------------------------^
> > > Shouldn't that be ERR_PTR(d)?
> >
> > Oops, yeah--well, actually just "d" I guess.
> >
> > This is what I've got for what it's worth.
> >
> > --b.
> >
>
> I think the patch looks ok to me. Did you give it a spin to test it?

Sure, just
mkfs.gfs2 -p lock_nolock /dev/vdb
mount /dev/vdb /mnt/
cd cthon
NFSTESTDIR=/mnt/TMP ./runtests -a -f

Which doesn't test much of anything (we'd need to inject artificial
errors for now to test the error paths), but at least it doesn't blow up
in some obvious way....

> If so then I will put it in the GFS2 -nmw tree,

Thanks!

--b.

>
> Steve.
>
> > commit 6fba5295019b52a03d76c9e9570952466051a7a6
> > Author: J. Bruce Fields <bfields@xxxxxxxxxx>
> > Date: Thu Jan 16 11:44:53 2014 -0500
> >
> > gfs2: revert "GFS2: d_splice_alias() can't return error"
> >
> > 0d0d110720d7960b77c03c9f2597faaff4b484ae asserts that "d_splice_alias()
> > can't return error unless it was given an IS_ERR(inode)".
> >
> > That was true of the implementation of d_splice_alias, but this is
> > really a problem with d_splice_alias: at a minimum it should be able to
> > return -ELOOP in the case where inserting the given dentry would cause a
> > directory loop.
> >
> > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> >
> > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > index 7119504..3f44902 100644
> > --- a/fs/gfs2/inode.c
> > +++ b/fs/gfs2/inode.c
> > @@ -585,6 +585,9 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
> > error = PTR_ERR(inode);
> > if (!IS_ERR(inode)) {
> > d = d_splice_alias(inode, dentry);
> > + error = PTR_ERR(d);
> > + if (IS_ERR(d))
> > + goto fail_gunlock;
> > error = 0;
> > if (file) {
> > if (S_ISREG(inode->i_mode)) {
> > @@ -779,6 +782,11 @@ static struct dentry *__gfs2_lookup(struct inode *dir, struct dentry *dentry,
> > }
> >
> > d = d_splice_alias(inode, dentry);
> > + if (IS_ERR(d)) {
> > + iput(inode);
> > + gfs2_glock_dq_uninit(&gh);
> > + return d;
> > + }
> > if (file && S_ISREG(inode->i_mode))
> > error = finish_open(file, dentry, gfs2_open_common, opened);
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/