From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Thu, 09 Aug 2007 10:37:22 -0500 Subject: [Cluster-devel] [PATCH 4 of 5] Bz #248176: GFS2: invalid metadata block - REVISED In-Reply-To: <46BB1AC4.9030509@redhat.com> References: <1186609929.25269.46.camel@technetium.msp.redhat.com> <46BB1AC4.9030509@redhat.com> Message-ID: <1186673842.25269.81.camel@technetium.msp.redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Thu, 2007-08-09 at 09:46 -0400, Wendy Cheng wrote: > Set aside "after this patch, the problem goes away" thing ... > > I haven't checked previous three patches yet so I may not have the > overall picture ... but why adding the journal flush spin lock here > could prevent the new inode to get re-used before its associated buffer > are flushed to the logs ? Could you elaborate more ? > > > + down_write(&sdp->sd_log_flush_lock); > > block = rgblk_search(rgd, goal, GFS2_BLKST_UNLINKED, > > GFS2_BLKST_UNLINKED); > > + up_write(&sdp->sd_log_flush_lock); IIRC, if we don't protect rgblk_search from finding GFS2_BLKST_UNLINKED blocks, a "deleted" inode may be returned to function gfs2_inplace_reserve_i which will do an iput on the inode, which may reference buffers that are being flushed to disk. If almost all blocks in that bitmap are allocated, I think the deleted block may sometimes be reused and the buffer associated with the reused block may be changed before it's actually written out to disk. > My concern is that GFS2's usage of sd_log_flush_lock has been very > abused lately. The journal logic is gradually becoming difficult to > understand and maintain. With this change, we move a local spin lock > (that belongs to log.c) into another sub-component (rgrp). Intuitively, > this is not right. > > -- Wendy I agree that the journal logic is overly complex and difficult to understand. It's always been that way, but it is getting worse. I'd like to do some work on that section of code to simplify it, but only after we've stabilized the code for rhel5.1. Regards, Bob Peterson Red Hat Cluster Suite