From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
We don't quite handle buffer state properly in online repair's findroot
routine. If the buffer is already in-core we don't want to trash its
b_ops and state, so first we should try _get_buf to grab the buffer. If
the buffer is loaded, we only want to verify the structure of the buffer
since it could be dirty and the crc hasn't yet been recalculated.
Only if the buffer hasn't been read in should try _read_buf, and if we
were the ones who read the buffer then we must be careful to oneshot the
buffer so that a subsequent _read_buf won't find a buffer with no ops.
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
---
fs/xfs/scrub/repair.c | 67 +++++++++++++++++++++++++++++++++++++++----------
1 file changed, 54 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 97c3077fb005..fae50dced8bc 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -697,6 +697,7 @@ xrep_findroot_block(
struct xfs_mount *mp = ri->sc->mp;
struct xfs_buf *bp;
struct xfs_btree_block *btblock;
+ xfs_failaddr_t fa;
xfs_daddr_t daddr;
int block_level;
int error;
@@ -718,28 +719,68 @@ xrep_findroot_block(
return error;
}
- error = xfs_trans_read_buf(mp, ri->sc->tp, mp->m_ddev_targp, daddr,
- mp->m_bsize, 0, &bp, NULL);
- if (error)
- return error;
-
/*
- * Does this look like a block matching our fs and higher than any
- * other block we've found so far? If so, reattach buffer verifiers
- * so the AIL won't complain if the buffer is also dirty.
+ * Try to grab the buffer, on the off chance it's already in memory.
+ * If the buffer doesn't have the DONE flag set it hasn't been read
+ * into memory yet. Drop the buffer and read the buffer with NULL
+ * b_ops. (This could race with another read_buf.) If we get the
+ * buffer back with NULL b_ops then we know that there weren't any
+ * other readers. There's a risk we won't match the buffer with any
+ * of the findroot prototypes, so we want to encourage the buffer layer
+ * to drop the buffer as soon as possible.
*/
+ bp = xfs_trans_get_buf(ri->sc->tp, mp->m_ddev_targp, daddr,
+ mp->m_bsize, 0);
+ if (!bp)
+ return -ENOMEM;
+ if (!(bp->b_flags & XBF_DONE)) {
+ xfs_trans_brelse(ri->sc->tp, bp);
+
+ error = xfs_trans_read_buf(mp, ri->sc->tp, mp->m_ddev_targp,
+ daddr, mp->m_bsize, 0, &bp, NULL);
+ if (error)
+ return error;
+ if (!bp->b_ops)
+ xfs_buf_oneshot(bp);
+ }
+
+ /* Does this look like a block matching our fs? */
btblock = XFS_BUF_TO_BLOCK(bp);
if (be32_to_cpu(btblock->bb_magic) != fab->magic)
goto out;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!uuid_equal(&btblock->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
goto out;
- bp->b_ops = fab->buf_ops;
- /* Make sure we pass the verifiers. */
- bp->b_ops->verify_read(bp);
- if (bp->b_error)
- goto out;
+ /*
+ * We've matched this buffer by magic number to this findroot
+ * prototype. If there are no buffer ops attached, attach the one
+ * specified by the prototype. Otherwise, the buffer ops must match
+ * the prototype. We don't want to disturb existing b_ops.
+ */
+ if (bp->b_ops) {
+ if (bp->b_ops != fab->buf_ops)
+ goto out;
+ /*
+ * If the buffer was already incore (on a v5 fs) then it should
+ * already have had b_ops assigned. Call ->verify_struct to
+ * check the structure. Avoid checking the CRC because we
+ * don't calculate CRCs until the buffer is written by the log.
+ */
+ fa = bp->b_ops->verify_struct(bp);
+ if (fa)
+ goto out;
+ } else {
+ /*
+ * If we have to assign buffer ops, that means that nobody's
+ * checked the buffer structure or its CRC. Do both now by
+ * calling ->verify_read.
+ */
+ bp->b_ops = fab->buf_ops;
+ bp->b_ops->verify_read(bp);
+ if (bp->b_error)
+ goto out;
+ }
/* If we've recorded a root candidate... */
block_level = xfs_btree_get_level(btblock);
