[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] [PATCH 4 of 5] Bz #248176: GFS2: invalid metadata block - REVISED



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]