Commit 84a5b730 authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers

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: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBen Myers <bpm@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 0d0ab120
...@@ -1769,19 +1769,11 @@ xlog_recover_buffer_pass1( ...@@ -1769,19 +1769,11 @@ xlog_recover_buffer_pass1(
/* /*
* Check to see whether the buffer being recovered has a corresponding * Check to see whether the buffer being recovered has a corresponding
* entry in the buffer cancel record table. If it does then return 1 * entry in the buffer cancel record table. If it is, return the cancel
* so that it will be cancelled, otherwise return 0. If the buffer is * buffer structure to the caller.
* 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 STATIC struct xfs_buf_cancel *
xlog_check_buffer_cancelled( xlog_peek_buffer_cancelled(
struct xlog *log, struct xlog *log,
xfs_daddr_t blkno, xfs_daddr_t blkno,
uint len, uint len,
...@@ -1790,22 +1782,16 @@ xlog_check_buffer_cancelled( ...@@ -1790,22 +1782,16 @@ xlog_check_buffer_cancelled(
struct list_head *bucket; struct list_head *bucket;
struct xfs_buf_cancel *bcp; struct xfs_buf_cancel *bcp;
if (log->l_buf_cancel_table == NULL) { if (!log->l_buf_cancel_table) {
/* /* empty table means no cancelled buffers in the log */
* There is nothing in the table built in pass one,
* so this buffer must not be cancelled.
*/
ASSERT(!(flags & XFS_BLF_CANCEL)); 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); bucket = XLOG_BUF_CANCEL_BUCKET(log, blkno);
list_for_each_entry(bcp, bucket, bc_list) { list_for_each_entry(bcp, bucket, bc_list) {
if (bcp->bc_blkno == blkno && bcp->bc_len == len) if (bcp->bc_blkno == blkno && bcp->bc_len == len)
goto found; return bcp;
} }
/* /*
...@@ -1813,9 +1799,32 @@ xlog_check_buffer_cancelled( ...@@ -1813,9 +1799,32 @@ xlog_check_buffer_cancelled(
* that the buffer is NOT cancelled. * that the buffer is NOT cancelled.
*/ */
ASSERT(!(flags & XFS_BLF_CANCEL)); 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 * 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 * is cancelled. If this buffer is actually a buffer cancel log
...@@ -3127,7 +3136,7 @@ xlog_recover_buffer_ra_pass2( ...@@ -3127,7 +3136,7 @@ xlog_recover_buffer_ra_pass2(
struct xfs_buf_log_format *buf_f = item->ri_buf[0].i_addr; struct xfs_buf_log_format *buf_f = item->ri_buf[0].i_addr;
struct xfs_mount *mp = log->l_mp; 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)) { buf_f->blf_len, buf_f->blf_flags)) {
return; return;
} }
...@@ -3156,7 +3165,7 @@ xlog_recover_inode_ra_pass2( ...@@ -3156,7 +3165,7 @@ xlog_recover_inode_ra_pass2(
return; 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; return;
xfs_buf_readahead(mp->m_ddev_targp, ilfp->ilf_blkno, xfs_buf_readahead(mp->m_ddev_targp, ilfp->ilf_blkno,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment