From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 08 May 2007 15:24:41 +0100 Subject: [Cluster-devel] [PATCH] GFS2: kernel changes to support new gfs2_grow command (Try 3) In-Reply-To: <46408688.7060507@redhat.com> References: <46394174.2030709@redhat.com> <20070504202317.GA16877@redhat.com> <463BAA28.7000407@redhat.com> <46408688.7060507@redhat.com> Message-ID: <1178634281.7476.13.camel@quoit> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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.