* [Cluster-devel] [PATCH GFS2] GFS2 fatal: filesystem consistency error on rename [not found] <1792171916.105961285856840027.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com> @ 2010-09-30 14:34 ` Bob Peterson 2010-10-01 13:31 ` Steven Whitehouse 0 siblings, 1 reply; 2+ messages in thread From: Bob Peterson @ 2010-09-30 14:34 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, This patch fixes a GFS2 problem whereby the first rename after a mount can result in a file system consistency error being flagged improperly and cause the file system to withdraw. The problem is that the rename code tries to run the rgrp list with function gfs2_blk2rgrpd before the rgrp list is guaranteed to be read in from disk. The patch makes the rename function hold the rindex glock (as the gfs2_unlink code does today) which reads in the rgrp list if need be. There were a total of three places in the rename code that improperly referenced the rgrp list without the rindex glock and this patch fixes all three. Regards, Bob Peterson Red Hat GFS Signed-off-by: Bob Peterson <rpeterso@redhat.com> -- fs/gfs2/ops_inode.c | 8 ++++++-- fs/gfs2/rgrp.c | 22 +++++++++++++--------- fs/gfs2/rgrp.h | 8 +++++--- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/fs/gfs2/ops_inode.c b/fs/gfs2/ops_inode.c index fba0017..0534510 100644 --- a/fs/gfs2/ops_inode.c +++ b/fs/gfs2/ops_inode.c @@ -736,7 +736,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, struct gfs2_inode *ip = GFS2_I(odentry->d_inode); struct gfs2_inode *nip = NULL; struct gfs2_sbd *sdp = GFS2_SB(odir); - struct gfs2_holder ghs[5], r_gh = { .gh_gl = NULL, }; + struct gfs2_holder ghs[5], r_gh = { .gh_gl = NULL, }, ri_gh; struct gfs2_rgrpd *nrgd; unsigned int num_gh; int dir_rename = 0; @@ -750,6 +750,9 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, return 0; } + error = gfs2_rindex_hold(sdp, &ri_gh); + if (error) + return error; if (odip != ndip) { error = gfs2_glock_nq_init(sdp->sd_rename_gl, LM_ST_EXCLUSIVE, @@ -879,7 +882,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, al->al_requested = sdp->sd_max_dirres; - error = gfs2_inplace_reserve(ndip); + error = gfs2_inplace_reserve_ri(ndip); if (error) goto out_gunlock_q; @@ -961,6 +964,7 @@ out_gunlock_r: if (r_gh.gh_gl) gfs2_glock_dq_uninit(&r_gh); out: + gfs2_glock_dq_uninit(&ri_gh); return error; } diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index f9ddcf4..fb67f59 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1200,7 +1200,8 @@ out: * Returns: errno */ -int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line) +int gfs2_inplace_reserve_i(struct gfs2_inode *ip, int hold_rindex, + char *file, unsigned int line) { struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode); struct gfs2_alloc *al = ip->i_alloc; @@ -1211,12 +1212,15 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line) return -EINVAL; try_again: - /* We need to hold the rindex unless the inode we're using is - the rindex itself, in which case it's already held. */ - if (ip != GFS2_I(sdp->sd_rindex)) - error = gfs2_rindex_hold(sdp, &al->al_ri_gh); - else if (!sdp->sd_rgrps) /* We may not have the rindex read in, so: */ - error = gfs2_ri_update_special(ip); + if (hold_rindex) { + /* We need to hold the rindex unless the inode we're using is + the rindex itself, in which case it's already held. */ + if (ip != GFS2_I(sdp->sd_rindex)) + error = gfs2_rindex_hold(sdp, &al->al_ri_gh); + else if (!sdp->sd_rgrps) /* We may not have the rindex read + in, so: */ + error = gfs2_ri_update_special(ip); + } if (error) return error; @@ -1227,7 +1231,7 @@ try_again: try to free it, and try the allocation again. */ error = get_local_rgrp(ip, &unlinked, &last_unlinked); if (error) { - if (ip != GFS2_I(sdp->sd_rindex)) + if (hold_rindex && ip != GFS2_I(sdp->sd_rindex)) gfs2_glock_dq_uninit(&al->al_ri_gh); if (error != -EAGAIN) return error; @@ -1269,7 +1273,7 @@ void gfs2_inplace_release(struct gfs2_inode *ip) al->al_rgd = NULL; if (al->al_rgd_gh.gh_gl) gfs2_glock_dq_uninit(&al->al_rgd_gh); - if (ip != GFS2_I(sdp->sd_rindex)) + if (ip != GFS2_I(sdp->sd_rindex) && al->al_ri_gh.gh_gl) gfs2_glock_dq_uninit(&al->al_ri_gh); } diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h index f07119d..0e35c04 100644 --- a/fs/gfs2/rgrp.h +++ b/fs/gfs2/rgrp.h @@ -39,10 +39,12 @@ static inline void gfs2_alloc_put(struct gfs2_inode *ip) ip->i_alloc = NULL; } -extern int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, - unsigned int line); +extern int gfs2_inplace_reserve_i(struct gfs2_inode *ip, int hold_rindex, + char *file, unsigned int line); #define gfs2_inplace_reserve(ip) \ -gfs2_inplace_reserve_i((ip), __FILE__, __LINE__) + gfs2_inplace_reserve_i((ip), 1, __FILE__, __LINE__) +#define gfs2_inplace_reserve_ri(ip) \ + gfs2_inplace_reserve_i((ip), 0, __FILE__, __LINE__) extern void gfs2_inplace_release(struct gfs2_inode *ip); ^ permalink raw reply related [flat|nested] 2+ messages in thread
* [Cluster-devel] [PATCH GFS2] GFS2 fatal: filesystem consistency error on rename 2010-09-30 14:34 ` [Cluster-devel] [PATCH GFS2] GFS2 fatal: filesystem consistency error on rename Bob Peterson @ 2010-10-01 13:31 ` Steven Whitehouse 0 siblings, 0 replies; 2+ messages in thread From: Steven Whitehouse @ 2010-10-01 13:31 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, Now in the -nmw git tree. Thanks, Steve. On Thu, 2010-09-30 at 10:34 -0400, Bob Peterson wrote: > Hi, > > This patch fixes a GFS2 problem whereby the first rename after a > mount can result in a file system consistency error being flagged > improperly and cause the file system to withdraw. The problem is > that the rename code tries to run the rgrp list with function > gfs2_blk2rgrpd before the rgrp list is guaranteed to be read in > from disk. The patch makes the rename function hold the rindex > glock (as the gfs2_unlink code does today) which reads in the rgrp > list if need be. There were a total of three places in the rename > code that improperly referenced the rgrp list without the rindex > glock and this patch fixes all three. > > Regards, > > Bob Peterson > Red Hat GFS > > Signed-off-by: Bob Peterson <rpeterso@redhat.com> > -- > fs/gfs2/ops_inode.c | 8 ++++++-- > fs/gfs2/rgrp.c | 22 +++++++++++++--------- > fs/gfs2/rgrp.h | 8 +++++--- > 3 files changed, 24 insertions(+), 14 deletions(-) > > diff --git a/fs/gfs2/ops_inode.c b/fs/gfs2/ops_inode.c > index fba0017..0534510 100644 > --- a/fs/gfs2/ops_inode.c > +++ b/fs/gfs2/ops_inode.c > @@ -736,7 +736,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, > struct gfs2_inode *ip = GFS2_I(odentry->d_inode); > struct gfs2_inode *nip = NULL; > struct gfs2_sbd *sdp = GFS2_SB(odir); > - struct gfs2_holder ghs[5], r_gh = { .gh_gl = NULL, }; > + struct gfs2_holder ghs[5], r_gh = { .gh_gl = NULL, }, ri_gh; > struct gfs2_rgrpd *nrgd; > unsigned int num_gh; > int dir_rename = 0; > @@ -750,6 +750,9 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, > return 0; > } > > + error = gfs2_rindex_hold(sdp, &ri_gh); > + if (error) > + return error; > > if (odip != ndip) { > error = gfs2_glock_nq_init(sdp->sd_rename_gl, LM_ST_EXCLUSIVE, > @@ -879,7 +882,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry, > > al->al_requested = sdp->sd_max_dirres; > > - error = gfs2_inplace_reserve(ndip); > + error = gfs2_inplace_reserve_ri(ndip); > if (error) > goto out_gunlock_q; > > @@ -961,6 +964,7 @@ out_gunlock_r: > if (r_gh.gh_gl) > gfs2_glock_dq_uninit(&r_gh); > out: > + gfs2_glock_dq_uninit(&ri_gh); > return error; > } > > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c > index f9ddcf4..fb67f59 100644 > --- a/fs/gfs2/rgrp.c > +++ b/fs/gfs2/rgrp.c > @@ -1200,7 +1200,8 @@ out: > * Returns: errno > */ > > -int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line) > +int gfs2_inplace_reserve_i(struct gfs2_inode *ip, int hold_rindex, > + char *file, unsigned int line) > { > struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode); > struct gfs2_alloc *al = ip->i_alloc; > @@ -1211,12 +1212,15 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line) > return -EINVAL; > > try_again: > - /* We need to hold the rindex unless the inode we're using is > - the rindex itself, in which case it's already held. */ > - if (ip != GFS2_I(sdp->sd_rindex)) > - error = gfs2_rindex_hold(sdp, &al->al_ri_gh); > - else if (!sdp->sd_rgrps) /* We may not have the rindex read in, so: */ > - error = gfs2_ri_update_special(ip); > + if (hold_rindex) { > + /* We need to hold the rindex unless the inode we're using is > + the rindex itself, in which case it's already held. */ > + if (ip != GFS2_I(sdp->sd_rindex)) > + error = gfs2_rindex_hold(sdp, &al->al_ri_gh); > + else if (!sdp->sd_rgrps) /* We may not have the rindex read > + in, so: */ > + error = gfs2_ri_update_special(ip); > + } > > if (error) > return error; > @@ -1227,7 +1231,7 @@ try_again: > try to free it, and try the allocation again. */ > error = get_local_rgrp(ip, &unlinked, &last_unlinked); > if (error) { > - if (ip != GFS2_I(sdp->sd_rindex)) > + if (hold_rindex && ip != GFS2_I(sdp->sd_rindex)) > gfs2_glock_dq_uninit(&al->al_ri_gh); > if (error != -EAGAIN) > return error; > @@ -1269,7 +1273,7 @@ void gfs2_inplace_release(struct gfs2_inode *ip) > al->al_rgd = NULL; > if (al->al_rgd_gh.gh_gl) > gfs2_glock_dq_uninit(&al->al_rgd_gh); > - if (ip != GFS2_I(sdp->sd_rindex)) > + if (ip != GFS2_I(sdp->sd_rindex) && al->al_ri_gh.gh_gl) > gfs2_glock_dq_uninit(&al->al_ri_gh); > } > > diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h > index f07119d..0e35c04 100644 > --- a/fs/gfs2/rgrp.h > +++ b/fs/gfs2/rgrp.h > @@ -39,10 +39,12 @@ static inline void gfs2_alloc_put(struct gfs2_inode *ip) > ip->i_alloc = NULL; > } > > -extern int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, > - unsigned int line); > +extern int gfs2_inplace_reserve_i(struct gfs2_inode *ip, int hold_rindex, > + char *file, unsigned int line); > #define gfs2_inplace_reserve(ip) \ > -gfs2_inplace_reserve_i((ip), __FILE__, __LINE__) > + gfs2_inplace_reserve_i((ip), 1, __FILE__, __LINE__) > +#define gfs2_inplace_reserve_ri(ip) \ > + gfs2_inplace_reserve_i((ip), 0, __FILE__, __LINE__) > > extern void gfs2_inplace_release(struct gfs2_inode *ip); > > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-10-01 13:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1792171916.105961285856840027.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com>
2010-09-30 14:34 ` [Cluster-devel] [PATCH GFS2] GFS2 fatal: filesystem consistency error on rename Bob Peterson
2010-10-01 13:31 ` Steven Whitehouse
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.