* [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems [not found] <5d3809ab-68d1-429d-9c89-24467729eaf8@zmail16.collab.prod.int.phx2.redhat.com> @ 2012-01-04 14:06 ` Bob Peterson 2012-01-05 10:39 ` Steven Whitehouse 0 siblings, 1 reply; 5+ messages in thread From: Bob Peterson @ 2012-01-04 14:06 UTC (permalink / raw) To: cluster-devel.redhat.com 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 Signed-off-by: Bob Peterson <rpeterso@redhat.com> -- 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: ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems 2012-01-04 14:06 ` [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems Bob Peterson @ 2012-01-05 10:39 ` Steven Whitehouse 2012-01-05 14:57 ` Bob Peterson 0 siblings, 1 reply; 5+ messages in thread From: Steven Whitehouse @ 2012-01-05 10:39 UTC (permalink / raw) To: cluster-devel.redhat.com 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 > > Signed-off-by: Bob Peterson <rpeterso@redhat.com> > -- > 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: > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems 2012-01-05 10:39 ` Steven Whitehouse @ 2012-01-05 14:57 ` Bob Peterson 2012-01-05 15:32 ` Steven Whitehouse 0 siblings, 1 reply; 5+ messages in thread From: Bob Peterson @ 2012-01-05 14:57 UTC (permalink / raw) To: cluster-devel.redhat.com ----- 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; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems 2012-01-05 14:57 ` Bob Peterson @ 2012-01-05 15:32 ` Steven Whitehouse 2012-01-05 19:53 ` Bob Peterson 0 siblings, 1 reply; 5+ messages in thread From: Steven Whitehouse @ 2012-01-05 15:32 UTC (permalink / raw) To: cluster-devel.redhat.com 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems 2012-01-05 15:32 ` Steven Whitehouse @ 2012-01-05 19:53 ` Bob Peterson 0 siblings, 0 replies; 5+ messages in thread From: Bob Peterson @ 2012-01-05 19:53 UTC (permalink / raw) To: cluster-devel.redhat.com ----- Original Message ----- | 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. Hi, Good catch, Steve. I withdraw my previous patch because it's not needed upstream. The bug that I was trying to solve was already solved in upstream by another patch. So NACK to my patch; I'll use the upstream patch instead. Regards, Bob Peterson Red Hat File Systems ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-05 19:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5d3809ab-68d1-429d-9c89-24467729eaf8@zmail16.collab.prod.int.phx2.redhat.com>
2012-01-04 14:06 ` [Cluster-devel] [GFS2 Patch] GFS2: fsck.gfs2 finds unlinked inodes on full file systems Bob Peterson
2012-01-05 10:39 ` Steven Whitehouse
2012-01-05 14:57 ` Bob Peterson
2012-01-05 15:32 ` Steven Whitehouse
2012-01-05 19:53 ` Bob Peterson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.