Skip to content

Commit

Permalink
xfs: don't account buffer cancellation during log recovery readahead
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Dave Chinner authored and Ben Myers committed Aug 29, 2013
1 parent 0d0ab12 commit 84a5b73
Showing 1 changed file with 35 additions and 26 deletions.
61 changes: 35 additions & 26 deletions fs/xfs/xfs_log_recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1790,32 +1782,49 @@ 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;
}

/*
* We didn't find a corresponding entry in the table, so return 0 so
* 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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 84a5b73

Please sign in to comment.