From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wendy Cheng Date: Thu, 09 Aug 2007 14:21:31 -0400 Subject: [Cluster-devel] [PATCH 4 of 5] Bz #248176: GFS2: invalid metadata block - REVISED In-Reply-To: <1186673842.25269.81.camel@technetium.msp.redhat.com> References: <1186609929.25269.46.camel@technetium.msp.redhat.com> <46BB1AC4.9030509@redhat.com> <1186673842.25269.81.camel@technetium.msp.redhat.com> Message-ID: <46BB5B2B.2040405@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Bob Peterson wrote: > 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. > Log flushing is an asynchronous event. I still don't see how this can *protect* the condition you just described (i.e., prevents the block being assigned to someone else before log flush occurs). Or do I understand your statement right (i.e., the log flushing must occur before the block is used by someone else) ? It may *reduce* the possibility (if log flushing happens at the same time as this assignment) but I don't see how it can *stop* the condition. You may "reduce" the (rare) possibility but the real issue is still hanging there ? If this is true, then I don't agree we have to pay the price of moving a journal flushing lock into resource handling code. -- Wendy