diff options
author | Dave Chinner <dchinner@redhat.com> | 2013-08-27 00:10:53 +0200 |
---|---|---|
committer | Ben Myers <bpm@sgi.com> | 2013-08-29 17:37:06 +0200 |
commit | 84a5b7300c724f4000f689c410aeae3242b4f034 (patch) | |
tree | 9982c376b488c62d75ff832e27ca1da69e55a86b /fs | |
parent | xfs: check for underflow in xfs_iformat_fork() (diff) | |
download | linux-84a5b7300c724f4000f689c410aeae3242b4f034.tar.xz linux-84a5b7300c724f4000f689c410aeae3242b4f034.zip |
xfs: don't account buffer cancellation during log recovery readahead
When doing readhaead in log recovery, we check to see if buffers are
cancelled before doing readahead. If we find a cancelled buffer,
however, we always decrement the reference count we have on it, and
that means that readahead is causing a double decrement of the
cancelled buffer reference count.
This results in log recovery *replaying cancelled buffers* as the
actual recovery pass does not find the cancelled buffer entry in the
commit phase of the second pass across a transaction. On debug
kernels, this results in an ASSERT failure like so:
XFS: Assertion failed: !(flags & XFS_BLF_CANCEL), file: fs/xfs/xfs_log_recover.c, line: 1815
xfstests generic/311 reproduces this ASSERT failure with 100%
reproducability.
Fix it by making readahead only peek at the buffer cancelled state
rather than the full accounting that xlog_check_buffer_cancelled()
does.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Ben Myers <bpm@sgi.com>
Signed-off-by: Ben Myers <bpm@sgi.com>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/xfs/xfs_log_recover.c | 61 |
1 files changed, 35 insertions, 26 deletions
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 64e530e67053..90b756f7117c 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -1769,19 +1769,11 @@ xlog_recover_buffer_pass1( /* * Check to see whether the buffer being recovered has a corresponding - * entry in the buffer cancel record table. If it does then return 1 - * so that it will be cancelled, otherwise return 0. If the buffer is - * actually a buffer cancel item (XFS_BLF_CANCEL is set), then decrement - * the refcount on the entry in the table and remove it from the table - * if this is the last reference. - * - * We remove the cancel record from the table when we encounter its - * last occurrence in the log so that if the same buffer is re-used - * again after its last cancellation we actually replay the changes - * made at that point. + * entry in the buffer cancel record table. If it is, return the cancel + * buffer structure to the caller. */ -STATIC int -xlog_check_buffer_cancelled( +STATIC struct xfs_buf_cancel * +xlog_peek_buffer_cancelled( struct xlog *log, xfs_daddr_t blkno, uint len, @@ -1790,22 +1782,16 @@ xlog_check_buffer_cancelled( struct list_head *bucket; struct xfs_buf_cancel *bcp; - if (log->l_buf_cancel_table == NULL) { - /* - * There is nothing in the table built in pass one, - * so this buffer must not be cancelled. - */ + if (!log->l_buf_cancel_table) { + /* empty table means no cancelled buffers in the log */ ASSERT(!(flags & XFS_BLF_CANCEL)); - return 0; + return NULL; } - /* - * Search for an entry in the cancel table that matches our buffer. - */ bucket = XLOG_BUF_CANCEL_BUCKET(log, blkno); list_for_each_entry(bcp, bucket, bc_list) { if (bcp->bc_blkno == blkno && bcp->bc_len == len) - goto found; + return bcp; } /* @@ -1813,9 +1799,32 @@ xlog_check_buffer_cancelled( * that the buffer is NOT cancelled. */ ASSERT(!(flags & XFS_BLF_CANCEL)); - return 0; + return NULL; +} + +/* + * If the buffer is being cancelled then return 1 so that it will be cancelled, + * otherwise return 0. If the buffer is actually a buffer cancel item + * (XFS_BLF_CANCEL is set), then decrement the refcount on the entry in the + * table and remove it from the table if this is the last reference. + * + * We remove the cancel record from the table when we encounter its last + * occurrence in the log so that if the same buffer is re-used again after its + * last cancellation we actually replay the changes made at that point. + */ +STATIC int +xlog_check_buffer_cancelled( + struct xlog *log, + xfs_daddr_t blkno, + uint len, + ushort flags) +{ + struct xfs_buf_cancel *bcp; + + bcp = xlog_peek_buffer_cancelled(log, blkno, len, flags); + if (!bcp) + return 0; -found: /* * We've go a match, so return 1 so that the recovery of this buffer * is cancelled. If this buffer is actually a buffer cancel log @@ -3127,7 +3136,7 @@ xlog_recover_buffer_ra_pass2( struct xfs_buf_log_format *buf_f = item->ri_buf[0].i_addr; struct xfs_mount *mp = log->l_mp; - if (xlog_check_buffer_cancelled(log, buf_f->blf_blkno, + if (xlog_peek_buffer_cancelled(log, buf_f->blf_blkno, buf_f->blf_len, buf_f->blf_flags)) { return; } @@ -3156,7 +3165,7 @@ xlog_recover_inode_ra_pass2( return; } - if (xlog_check_buffer_cancelled(log, ilfp->ilf_blkno, ilfp->ilf_len, 0)) + if (xlog_peek_buffer_cancelled(log, ilfp->ilf_blkno, ilfp->ilf_len, 0)) return; xfs_buf_readahead(mp->m_ddev_targp, ilfp->ilf_blkno, |