Re: [RFC 00/12] xfs: more and better verifiers
From: Christoph Hellwig <hch@infradead.org>
Date: 2017-08-18 07:05:17
Subsystem:
filesystems (vfs and infrastructure), the rest, xfs filesystem · Maintainers:
Alexander Viro, Christian Brauner, Linus Torvalds, Carlos Maiolino
On Thu, Aug 17, 2017 at 04:31:29PM -0700, Darrick J. Wong wrote:
Hi all, This RFC combines all the random little fixes and improvements to the verifiers that we've been talking about for the past month or so into a single patch series! We start by refactoring the long format btree block header verifier into a single helper functionn and de-macroing dir block verifiers to make them less shouty. Next, we change verifier functions to return the approximate instruction pointer of the faulting test so that we can report more precise fault information to dmesg/tracepoints.
Just jumping here quickly because I don't have time for a detailed review: How good does this instruction pointer thing resolved to the actual issue? I'm currently watching a customer issue where a write verifier triggers, and I gave them a patch to add a debug print to every failing statement, including printing out the mismatch values if it's not simply a binary comparism. I though about preparing that patch as well as others for mainline. Here is the one I have at the moment: ---
From 6c5e2efc6f857228461d439feb3c98be58fb9744 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de> Date: Sat, 5 Aug 2017 16:34:15 +0200 Subject: xfs: print verbose information on dir leaf verifier failures Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_dir2_leaf.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index b887fb2a2bcf..4386c68f72c6 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c@@ -113,27 +113,37 @@ xfs_dir3_leaf_check_int( * Should factor in the size of the bests table as well. * We can deduce a value for that from di_size. */ - if (hdr->count > ops->leaf_max_ents(geo)) + if (hdr->count > ops->leaf_max_ents(geo)) { + xfs_warn(mp, "count (%d) above max (%d)\n", + hdr->count, ops->leaf_max_ents(geo)); return false; + } /* Leaves and bests don't overlap in leaf format. */ if ((hdr->magic == XFS_DIR2_LEAF1_MAGIC || hdr->magic == XFS_DIR3_LEAF1_MAGIC) && - (char *)&ents[hdr->count] > (char *)xfs_dir2_leaf_bests_p(ltp)) + (char *)&ents[hdr->count] > (char *)xfs_dir2_leaf_bests_p(ltp)) { + xfs_warn(mp, "ents overlappings bests\n"); return false; + } /* Check hash value order, count stale entries. */ for (i = stale = 0; i < hdr->count; i++) { if (i + 1 < hdr->count) { if (be32_to_cpu(ents[i].hashval) > - be32_to_cpu(ents[i + 1].hashval)) + be32_to_cpu(ents[i + 1].hashval)) { + xfs_warn(mp, "broken hash order\n"); return false; + } } if (ents[i].address == cpu_to_be32(XFS_DIR2_NULL_DATAPTR)) stale++; } - if (hdr->stale != stale) + if (hdr->stale != stale) { + xfs_warn(mp, "incorrect stalte count (%d, expected %d)\n", + hdr->stale, stale); return false; + } return true; }
@@ -159,12 +169,21 @@ xfs_dir3_leaf_verify( magic3 = (magic == XFS_DIR2_LEAF1_MAGIC) ? XFS_DIR3_LEAF1_MAGIC : XFS_DIR3_LEAFN_MAGIC; - if (leaf3->info.hdr.magic != cpu_to_be16(magic3)) + if (leaf3->info.hdr.magic != cpu_to_be16(magic3)) { + xfs_warn(mp, "incorrect magic number (0x%hx, expected 0x%hx)\n", + leaf3->info.hdr.magic, magic3); return false; - if (!uuid_equal(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid)) + } + if (!uuid_equal(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid)) { + xfs_warn(mp, "incorrect uuid, (%pUb, expected %pUb)\n", + &leaf3->info.uuid, &mp->m_sb.sb_meta_uuid); return false; - if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn) + } + if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn) { + xfs_warn(mp, "incorrect blkno, (%lld, expected %lld)\n", + be64_to_cpu(leaf3->info.blkno), bp->b_bn); return false; + } if (!xfs_log_check_lsn(mp, be64_to_cpu(leaf3->info.lsn))) return false; } else {
--
2.11.0