[Cluster-devel] [PATCH] GFS2: kernel changes to support new gfs2_grow command (Try 3)

[Steven Whitehouse]

Hi,

On Tue, 2007-05-08 at 09:17 -0500, Robert Peterson wrote:
> Robert Peterson wrote:
> > David Teigland wrote:
> >> On Wed, May 02, 2007 at 08:57:08PM -0500, Robert Peterson wrote:
> >>> @@ -978,18 +1038,25 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, 
> >>
> >>> -    error = gfs2_rindex_hold(sdp, &al->al_ri_gh);
> >>> +    /* 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_check_rindex_version(sdp);
> >>> +
> >>
> >> I don't see why we need this change, the original looks correct.
> > 
> > If we're updating through the meta_fs, the rindex file will already be
> > glocked and held through its inode.
> > 
> >> So there are the two distinct parts to the fs-grow procedure.  The first
> >> is writing to the rindex and statfs files -- this is what was missing in
> >> gfs2, and I'm ignoring that part for now.
> >>
> >> The second part is gfs detecting that a grow took place and updating its
> >> list of rg's.  I don't think this part needs any changing at all, it
> >> should work the same way it always has -- this is what I'm focussing on.
> > 
> > I'll re-test test the original code path to make sure that this part of
> > the fix is still really necessary.
> > 
> > Regards,
> > 
> > Bob Peterson
> > Red Hat Cluster Suite
> > 
> Hi Dave,
> 
> Well, I retested this code path and found that this code is still 
> necessary  (unless I should somehow be doing this a different way).
> Without the code, the gfs2_rindex_hold tries to add a holder,
> but there's already a holder due to the meta_fs.  Under the right 
> conditions, you get:
> 
> original: gfs2_prepare_write+0x49/0x237 [gfs2]
> new: gfs2_rindex_hold+0x2b/0x52a [gfs2]

I agree that this change is needed, its how we've solved the same
problem elsewhere and it looks ok to me,

Steve.