From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Thu, 05 Jan 2012 15:32:17 +0000
Subject: [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked
 inodes on full file systems
In-Reply-To: <cb906015-c1b7-4b92-bf19-b1f5806602e0@zmail16.collab.prod.int.phx2.redhat.com>
References: <cb906015-c1b7-4b92-bf19-b1f5806602e0@zmail16.collab.prod.int.phx2.redhat.com>
Message-ID: <1325777537.2690.36.camel@menhir>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On Thu, 2012-01-05 at 09:57 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | Hi,
> | 
> | This doesn't compile....
> | 
> |   CC [M]  fs/gfs2/inode.o
> | fs/gfs2/inode.c: In function ?gfs2_create_inode?:
> | fs/gfs2/inode.c:741:2: error: implicit declaration of function
> | ?gfs2_dinode_dealloc?
> | make[2]: *** [fs/gfs2/inode.o] Error 1
> | make[1]: *** [fs/gfs2] Error 2
> | make: *** [fs] Error 2
> | 
> | 
> | Steve.
> | 
> | On Wed, 2012-01-04 at 09:06 -0500, Bob Peterson wrote:
> | > Hi,
> | > 
> | > This patch fixes a problem whereby GFS2 does not properly clean
> | > up partially created files when the file system becomes too full.
> | > 
> | > Regards,
> | > 
> | > Bob Peterson
> | > Red Hat File Systems
> 
> My apologies. I didn't catch the fact that the function had moved
> to super.c and become static. Here's a replacement that compiles.
> Unless you want me to move the function back to inode.c since it's
> not static anymore?
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> --
> commit 99259e30d3cc49cee5c55be9ca5ab76e772ad22f
> Author: Bob Peterson <rpeterso@redhat.com>
> Date:   Wed Jan 4 08:57:11 2012 -0600
> 
>     GFS2: fsck.gfs2 finds unlinked inodes on full file systems
>     
>     In some cases, when a GFS2 file system was nearly full, GFS2
>     was able to allocate a block for a new file's dinode but not
>     a second block for linking that new dinode to a directory.
>     In these cases, GFS2 was not properly deallocating the partially-
>     created file. Thus, subsequent runs of fsck.gfs2 found unlinked
>     files. This patch adds a an extra cleanup path to the dinode
>     create function to properly deallocate the partially-created file.
> 
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index cb81898..bc81c9d 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -708,19 +708,19 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
>  
>  	error = gfs2_inode_refresh(GFS2_I(inode));
>  	if (error)
> -		goto fail_gunlock2;
> +		goto fail_dealloc;
>  
>  	error = gfs2_acl_create(dip, inode);
>  	if (error)
> -		goto fail_gunlock2;
> +		goto fail_dealloc;
>  
>  	error = gfs2_security_init(dip, GFS2_I(inode), name);
>  	if (error)
> -		goto fail_gunlock2;
> +		goto fail_dealloc;
>  
>  	error = link_dinode(dip, name, GFS2_I(inode));
>  	if (error)
> -		goto fail_gunlock2;
> +		goto fail_dealloc;
>  
>  	if (bh)
>  		brelse(bh);
> @@ -737,6 +737,8 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
>  	d_instantiate(dentry, inode);
>  	return 0;
>  
> +fail_dealloc:
> +	gfs2_dinode_dealloc(GFS2_I(inode));
>  fail_gunlock2:
>  	gfs2_glock_dq_uninit(ghs + 1);
>  fail_gunlock:
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 69eb568..0b00f2f 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1396,7 +1396,7 @@ static void gfs2_final_release_pages(struct gfs2_inode *ip)
>  	}
>  }
>  
> -static int gfs2_dinode_dealloc(struct gfs2_inode *ip)
> +int gfs2_dinode_dealloc(struct gfs2_inode *ip)
>  {
>  	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>  	struct gfs2_qadata *qa;
> diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
> index a046468..bb0029b 100644
> --- a/fs/gfs2/super.h
> +++ b/fs/gfs2/super.h
> @@ -48,6 +48,7 @@ extern int gfs2_statfs_sync(struct super_block *sb, int type);
>  
>  extern int gfs2_freeze_fs(struct gfs2_sbd *sdp);
>  extern void gfs2_unfreeze_fs(struct gfs2_sbd *sdp);
> +extern int gfs2_dinode_dealloc(struct gfs2_inode *ip);
>  
>  extern struct file_system_type gfs2_fs_type;
>  extern struct file_system_type gfs2meta_fs_type;

One further thought... since the struct inode exists by the stage we hit
those error cases, it should be deallocated when it gets iput() in the
error case. So I wonder why thats not happened... perhaps i_nlink is not
zero at that time?

Steve.