From fc5bc4c85c45f0bf854404e5736aa8b65720a18d Mon Sep 17 00:00:00 2001 From: Andy Poling Date: Tue, 3 Nov 2009 17:26:47 +0000 Subject: xfs: Wrapped journal record corruption on read at recovery Summary of problem: If a journal record wraps at the physical end of the journal, it has to be read in two parts in xlog_do_recovery_pass(): a read at the physical end and a read at the physical beginning. If xlog_bread() has to re-align the first read, the second read request does not take that re-alignment into account. If the first read was re-aligned, the second read over-writes the end of the data from the first read, effectively corrupting it. This can happen either when reading the record header or reading the record data. The first sanity check in xlog_recover_process_data() is to check for a valid clientid, so that is the error reported. Summary of fix: If there was a first read at the physical end, XFS_BUF_PTR() returns where the data was requested to begin. Conversely, because it is the result of xlog_align(), offset indicates where the requested data for the first read actually begins - whether or not xlog_bread() has re-aligned it. Using offset as the base for the calculation of where to place the second read data ensures that it will be correctly placed immediately following the data from the first read instead of sometimes over-writing the end of it. The attached patch has resolved the reported problem of occasional inability to recover the journal (reporting "bad clientid"). Signed-off-by: Andy Poling Reviewed-by: Alex Elder Signed-off-by: Alex Elder --- fs/xfs/xfs_log_recover.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) (limited to 'fs/xfs') diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index fb17f8226b0..b5b0d805591 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -3517,7 +3517,7 @@ xlog_do_recovery_pass( { xlog_rec_header_t *rhead; xfs_daddr_t blk_no; - xfs_caddr_t bufaddr, offset; + xfs_caddr_t offset; xfs_buf_t *hbp, *dbp; int error = 0, h_size; int bblks, split_bblks; @@ -3610,7 +3610,7 @@ xlog_do_recovery_pass( /* * Check for header wrapping around physical end-of-log */ - offset = NULL; + offset = XFS_BUF_PTR(hbp); split_hblks = 0; wrapped_hblks = 0; if (blk_no + hblks <= log->l_logBBsize) { @@ -3646,9 +3646,8 @@ xlog_do_recovery_pass( * - order is important. */ wrapped_hblks = hblks - split_hblks; - bufaddr = XFS_BUF_PTR(hbp); error = XFS_BUF_SET_PTR(hbp, - bufaddr + BBTOB(split_hblks), + offset + BBTOB(split_hblks), BBTOB(hblks - split_hblks)); if (error) goto bread_err2; @@ -3658,14 +3657,10 @@ xlog_do_recovery_pass( if (error) goto bread_err2; - error = XFS_BUF_SET_PTR(hbp, bufaddr, + error = XFS_BUF_SET_PTR(hbp, offset, BBTOB(hblks)); if (error) goto bread_err2; - - if (!offset) - offset = xlog_align(log, 0, - wrapped_hblks, hbp); } rhead = (xlog_rec_header_t *)offset; error = xlog_valid_rec_header(log, rhead, @@ -3685,7 +3680,7 @@ xlog_do_recovery_pass( } else { /* This log record is split across the * physical end of log */ - offset = NULL; + offset = XFS_BUF_PTR(dbp); split_bblks = 0; if (blk_no != log->l_logBBsize) { /* some data is before the physical @@ -3714,9 +3709,8 @@ xlog_do_recovery_pass( * _first_, then the log start (LR header end) * - order is important. */ - bufaddr = XFS_BUF_PTR(dbp); error = XFS_BUF_SET_PTR(dbp, - bufaddr + BBTOB(split_bblks), + offset + BBTOB(split_bblks), BBTOB(bblks - split_bblks)); if (error) goto bread_err2; @@ -3727,13 +3721,9 @@ xlog_do_recovery_pass( if (error) goto bread_err2; - error = XFS_BUF_SET_PTR(dbp, bufaddr, h_size); + error = XFS_BUF_SET_PTR(dbp, offset, h_size); if (error) goto bread_err2; - - if (!offset) - offset = xlog_align(log, wrapped_hblks, - bblks - split_bblks, dbp); } xlog_unpack_data(rhead, offset, log); if ((error = xlog_recover_process_data(log, rhash, -- cgit v1.2.3-18-g5258