[Cluster-devel] [PATCH 1/3] gfs2: bugger off early if O_CREAT open finds a directory

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> On Wed, Nov 19, 2014 at 03:33:29PM -0500, Bob Peterson wrote:
> > ----- Original Message -----
> > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > > ---
> > >  fs/gfs2/inode.c |    5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > > index c4ed823..310e248 100644
> > > --- a/fs/gfs2/inode.c
> > > +++ b/fs/gfs2/inode.c
> > > @@ -624,6 +624,11 @@ static int gfs2_create_inode(struct inode *dir,
> > > struct
> > > dentry *dentry,
> > >  	inode = gfs2_dir_search(dir, &dentry->d_name, !S_ISREG(mode) || excl);
> > >  	error = PTR_ERR(inode);
> > >  	if (!IS_ERR(inode)) {
> > > +		if (S_ISDIR(inode->i_mode)) {
> > > +			iput(inode);
> > > +			inode = ERR_PTR(-EISDIR);
> > > +			goto fail_gunlock;
> > > +		}
> > >  		d = d_splice_alias(inode, dentry);
> > >  		error = PTR_ERR(d);
> > >  		if (IS_ERR(d)) {
> 
> > Hm. Seems wrong that it should return 0 if the dirent exists (mkdir of a
> > directory that already exists) but it looks like it already behaves that
> > way.
> > So I guess so. It may warrant further investigation.
> 
> It doesn't.  Note that !S_ISREG(mode) || excl in there - *anything* other
> than ->create() will treat existing entries with the same name as EEXIST.
> The only case when we can possibly get into that if (!IS_ERR(inode)) is
> ->create() hitting an existing file.  And with this change it narrows to
> "->create() hitting an existing non-directory".  That allows the next patch
> to use d_instantiate() instead of d_splice_alias() - for non-directories
> it's the same thing, since dentry is already hashed here and we don't need
> to avoid multiple aliases.  Which kills one of the two places in the
> tree where d_splice_alias() is called for an already hashed dentry (another
> is d_add_ci() and that call also goes down - see vfs.git#for-next for that
> one).
> 
Okay, thanks for clearing that up.

ACK

Bob Peterson
Red Hat File Systems