* [PATCH 0/9] current series for verifier error differentiation
@ 2014-02-18 23:52 Eric Sandeen
2014-02-18 23:52 ` [PATCH 1/9] xfs: skip verification on initial "guess" superblock read Eric Sandeen
` (9 more replies)
0 siblings, 10 replies; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
This is the current series I have leading up to verifier error differentiation,
just resending them all.
As I send this, I remember that EILSEQ is giving us an unusual perror()
output, so the last patch which uses it may need to pick some (?)
other error code...
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/9] xfs: skip verification on initial "guess" superblock read
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 3:36 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption Eric Sandeen
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
When xfs_readsb() does the very first read of the superblock,
it makes a guess at the length of the buffer, based on the
sector size of the underlying storage. This may or may
not match the filesystem sector size in sb_sectsize, so
we can't i.e. do a CRC check on it; it might be too short.
In fact, mounting a filesystem with sb_sectsize larger
than the device sector size will cause a mount failure
if CRCs are enabled, because we are checksumming a length
which exceeds the buffer passed to it.
So always read twice; the first time we read with NULL
buffer ops to skip verification; then set the proper
read length, hook up the proper verifier, and give it
another go.
Once we are sure that we've got the right buffer length,
we can also use bp->b_length in the xfs_sb_read_verify,
rather than the less-trusted on-disk sectorsize for
secondary superblocks. Before this we ran the risk of
passing junk to the crc32c routines, which didn't always
handle extreme values.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_mount.c | 24 ++++++++++++++++--------
fs/xfs/xfs_sb.c | 3 +--
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02df7b4..f96c056 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -282,22 +282,29 @@ xfs_readsb(
struct xfs_sb *sbp = &mp->m_sb;
int error;
int loud = !(flags & XFS_MFSI_QUIET);
+ const struct xfs_buf_ops *buf_ops;
ASSERT(mp->m_sb_bp == NULL);
ASSERT(mp->m_ddev_targp != NULL);
/*
+ * For the initial read, we must guess at the sector
+ * size based on the block device. It's enough to
+ * get the sb_sectsize out of the superblock and
+ * then reread with the proper length.
+ * We don't verify it yet, because it may not be complete.
+ */
+ sector_size = xfs_getsize_buftarg(mp->m_ddev_targp);
+ buf_ops = NULL;
+
+ /*
* Allocate a (locked) buffer to hold the superblock.
* This will be kept around at all times to optimize
* access to the superblock.
*/
- sector_size = xfs_getsize_buftarg(mp->m_ddev_targp);
-
reread:
bp = xfs_buf_read_uncached(mp->m_ddev_targp, XFS_SB_DADDR,
- BTOBB(sector_size), 0,
- loud ? &xfs_sb_buf_ops
- : &xfs_sb_quiet_buf_ops);
+ BTOBB(sector_size), 0, buf_ops);
if (!bp) {
if (loud)
xfs_warn(mp, "SB buffer read failed");
@@ -328,12 +335,13 @@ reread:
}
/*
- * If device sector size is smaller than the superblock size,
- * re-read the superblock so the buffer is correctly sized.
+ * Re-read the superblock so the buffer is correctly sized,
+ * and properly verified.
*/
- if (sector_size < sbp->sb_sectsize) {
+ if (buf_ops == NULL) {
xfs_buf_relse(bp);
sector_size = sbp->sb_sectsize;
+ buf_ops = loud ? &xfs_sb_buf_ops : &xfs_sb_quiet_buf_ops;
goto reread;
}
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 5071ccb..359b19a 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -611,7 +611,7 @@ xfs_sb_read_verify(
XFS_SB_VERSION_5) ||
dsb->sb_crc != 0)) {
- if (!xfs_verify_cksum(bp->b_addr, be16_to_cpu(dsb->sb_sectsize),
+ if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
offsetof(struct xfs_sb, sb_crc))) {
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
@@ -644,7 +644,6 @@ xfs_sb_quiet_read_verify(
{
struct xfs_dsb *dsb = XFS_BUF_TO_SBP(bp);
-
if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
/* XFS filesystem, verify noisily! */
xfs_sb_read_verify(bp);
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
2014-02-18 23:52 ` [PATCH 1/9] xfs: skip verification on initial "guess" superblock read Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 3:37 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures Eric Sandeen
` (7 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Today, if
xfs_sb_read_verify
xfs_sb_verify
xfs_mount_validate_sb
detects superblock corruption, it'll be extremely noisy, dumping
2 stacks, 2 hexdumps, etc.
This is because we call XFS_CORRUPTION_ERROR in xfs_mount_validate_sb
as well as in xfs_sb_read_verify.
Also, *any* errors in xfs_mount_validate_sb which are not corruption
per se; things like too-big-blocksize, bad version, bad magic, v1 dirs,
rw-incompat etc - things which do not return EFSCORRUPTED - will
still do the whole XFS_CORRUPTION_ERROR spew when xfs_sb_read_verify
sees any error at all. And it suggests to the user that they
should run xfs_repair, even if the root cause of the mount failure
is a simple incompatibility.
I'll submit that the probably-not-corrupted errors don't warrant
this much noise, so this patch removes the warning for anything
other than EFSCORRUPTED returns, and replaces the lower-level
XFS_CORRUPTION_ERROR with an xfs_notice().
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_sb.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 359b19a..1e11679 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -295,8 +295,7 @@ xfs_mount_validate_sb(
sbp->sb_dblocks == 0 ||
sbp->sb_dblocks > XFS_MAX_DBLOCKS(sbp) ||
sbp->sb_dblocks < XFS_MIN_DBLOCKS(sbp))) {
- XFS_CORRUPTION_ERROR("SB sanity check failed",
- XFS_ERRLEVEL_LOW, mp, sbp);
+ xfs_notice(mp, "SB sanity check failed");
return XFS_ERROR(EFSCORRUPTED);
}
@@ -625,7 +624,7 @@ xfs_sb_read_verify(
out_error:
if (error) {
- if (error != EWRONGFS)
+ if (error == EFSCORRUPTED)
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
mp, bp->b_addr);
xfs_buf_ioerror(bp, error);
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
2014-02-18 23:52 ` [PATCH 1/9] xfs: skip verification on initial "guess" superblock read Eric Sandeen
2014-02-18 23:52 ` [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 6:35 ` Jeff Liu
2014-02-18 23:52 ` [PATCH 4/9] xfs: Use defines for CRC offsets in all cases Eric Sandeen
` (6 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Most write verifiers don't update CRCs after the verifier
has failed and the buffer has been marked in error. These
two didn't, but should.
Add returns to the verifier failure block,
since the buffer won't be written anyway.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_alloc_btree.c | 1 +
fs/xfs/xfs_ialloc_btree.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index 1308542..144d3b0 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -373,6 +373,7 @@ xfs_allocbt_write_verify(
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
index c8fa5bb..0028c50 100644
--- a/fs/xfs/xfs_ialloc_btree.c
+++ b/fs/xfs/xfs_ialloc_btree.c
@@ -261,6 +261,7 @@ xfs_inobt_write_verify(
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ return;
}
xfs_btree_sblock_calc_crc(bp);
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (2 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 7:56 ` Jeff Liu
2014-02-18 23:52 ` [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs Eric Sandeen
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Some calls to crc functions used useful #defines,
others used awkward offsetof() constructs.
Switch them all to #define to make things a bit
cleaner.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_ag.h | 6 ++++++
fs/xfs/xfs_alloc.c | 10 ++++------
fs/xfs/xfs_dinode.h | 2 ++
fs/xfs/xfs_format.h | 2 ++
fs/xfs/xfs_ialloc.c | 5 ++---
fs/xfs/xfs_inode_buf.c | 4 ++--
fs/xfs/xfs_sb.c | 5 ++---
fs/xfs/xfs_sb.h | 2 ++
fs/xfs/xfs_symlink_remote.c | 5 ++---
9 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
index 3fc1098..0fdd410 100644
--- a/fs/xfs/xfs_ag.h
+++ b/fs/xfs/xfs_ag.h
@@ -89,6 +89,8 @@ typedef struct xfs_agf {
/* structure must be padded to 64 bit alignment */
} xfs_agf_t;
+#define XFS_AGF_CRC_OFF offsetof(struct xfs_agf, agf_crc)
+
#define XFS_AGF_MAGICNUM 0x00000001
#define XFS_AGF_VERSIONNUM 0x00000002
#define XFS_AGF_SEQNO 0x00000004
@@ -167,6 +169,8 @@ typedef struct xfs_agi {
/* structure must be padded to 64 bit alignment */
} xfs_agi_t;
+#define XFS_AGI_CRC_OFF offsetof(struct xfs_agi, agi_crc)
+
#define XFS_AGI_MAGICNUM 0x00000001
#define XFS_AGI_VERSIONNUM 0x00000002
#define XFS_AGI_SEQNO 0x00000004
@@ -222,6 +226,8 @@ typedef struct xfs_agfl {
__be32 agfl_bno[]; /* actually XFS_AGFL_SIZE(mp) */
} xfs_agfl_t;
+#define XFS_AGFL_CRC_OFF offsetof(struct xfs_agfl, agfl_crc)
+
/*
* tags for inode radix tree
*/
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 9eab2df..72ea855 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -486,7 +486,7 @@ xfs_agfl_read_verify(
return;
agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agfl, agfl_crc));
+ XFS_AGFL_CRC_OFF);
agfl_ok = agfl_ok && xfs_agfl_verify(bp);
@@ -516,8 +516,7 @@ xfs_agfl_write_verify(
if (bip)
XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agfl, agfl_crc));
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGFL_CRC_OFF);
}
const struct xfs_buf_ops xfs_agfl_buf_ops = {
@@ -2242,7 +2241,7 @@ xfs_agf_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb))
agf_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agf, agf_crc));
+ XFS_AGF_CRC_OFF);
agf_ok = agf_ok && xfs_agf_verify(mp, bp);
@@ -2272,8 +2271,7 @@ xfs_agf_write_verify(
if (bip)
XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agf, agf_crc));
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGF_CRC_OFF);
}
const struct xfs_buf_ops xfs_agf_buf_ops = {
diff --git a/fs/xfs/xfs_dinode.h b/fs/xfs/xfs_dinode.h
index e5869b5..623bbe8 100644
--- a/fs/xfs/xfs_dinode.h
+++ b/fs/xfs/xfs_dinode.h
@@ -89,6 +89,8 @@ typedef struct xfs_dinode {
/* structure must be padded to 64 bit alignment */
} xfs_dinode_t;
+#define XFS_DINODE_CRC_OFF offsetof(struct xfs_dinode, di_crc)
+
#define DI_MAX_FLUSH 0xffff
/*
diff --git a/fs/xfs/xfs_format.h b/fs/xfs/xfs_format.h
index b6ab5a3..9898f31 100644
--- a/fs/xfs/xfs_format.h
+++ b/fs/xfs/xfs_format.h
@@ -145,6 +145,8 @@ struct xfs_dsymlink_hdr {
__be64 sl_lsn;
};
+#define XFS_SYMLINK_CRC_OFF offsetof(struct xfs_dsymlink_hdr, sl_crc)
+
/*
* The maximum pathlen is 1024 bytes. Since the minimum file system
* blocksize is 512 bytes, we can get a max of 3 extents back from
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 5d7f105..d79210b 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1572,7 +1572,7 @@ xfs_agi_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb))
agi_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agi, agi_crc));
+ XFS_AGI_CRC_OFF);
agi_ok = agi_ok && xfs_agi_verify(bp);
if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
@@ -1600,8 +1600,7 @@ xfs_agi_write_verify(
if (bip)
XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_agi, agi_crc));
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGI_CRC_OFF);
}
const struct xfs_buf_ops xfs_agi_buf_ops = {
diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
index 4fc9f39..606b43a 100644
--- a/fs/xfs/xfs_inode_buf.c
+++ b/fs/xfs/xfs_inode_buf.c
@@ -306,7 +306,7 @@ xfs_dinode_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return false;
if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
- offsetof(struct xfs_dinode, di_crc)))
+ XFS_DINODE_CRC_OFF))
return false;
if (be64_to_cpu(dip->di_ino) != ip->i_ino)
return false;
@@ -327,7 +327,7 @@ xfs_dinode_calc_crc(
ASSERT(xfs_sb_version_hascrc(&mp->m_sb));
crc = xfs_start_cksum((char *)dip, mp->m_sb.sb_inodesize,
- offsetof(struct xfs_dinode, di_crc));
+ XFS_DINODE_CRC_OFF);
dip->di_crc = xfs_end_cksum(crc);
}
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 1e11679..1ea7c86 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -611,7 +611,7 @@ xfs_sb_read_verify(
dsb->sb_crc != 0)) {
if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_sb, sb_crc))) {
+ XFS_SB_CRC_OFF)) {
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
xfs_sb_version_hascrc(&mp->m_sb)) {
@@ -674,8 +674,7 @@ xfs_sb_write_verify(
if (bip)
XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_sb, sb_crc));
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SB_CRC_OFF);
}
const struct xfs_buf_ops xfs_sb_buf_ops = {
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index 35061d4..f7b2fe7 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -182,6 +182,8 @@ typedef struct xfs_sb {
/* must be padded to 64 bit alignment */
} xfs_sb_t;
+#define XFS_SB_CRC_OFF offsetof(struct xfs_sb, sb_crc)
+
/*
* Superblock - on disk version. Must match the in core version above.
* Must be padded to 64 bit alignment.
diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
index bf59a2b..7a705a4 100644
--- a/fs/xfs/xfs_symlink_remote.c
+++ b/fs/xfs/xfs_symlink_remote.c
@@ -134,7 +134,7 @@ xfs_symlink_read_verify(
return;
if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_dsymlink_hdr, sl_crc)) ||
+ XFS_SYMLINK_CRC_OFF) ||
!xfs_symlink_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
@@ -162,8 +162,7 @@ xfs_symlink_write_verify(
struct xfs_dsymlink_hdr *dsl = bp->b_addr;
dsl->sl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
}
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- offsetof(struct xfs_dsymlink_hdr, sl_crc));
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SYMLINK_CRC_OFF);
}
const struct xfs_buf_ops xfs_symlink_buf_ops = {
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (3 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 4/9] xfs: Use defines for CRC offsets in all cases Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-27 4:17 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 6/9] xfs: add helper for updating " Eric Sandeen
` (4 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Many/most callers of xfs_verify_cksum() pass bp->b_addr and
BBTOB(bp->b_length) as the first 2 args. Add a helper
which can just accept the bp and the crc offset, and work
it out on its own, for brevity.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_alloc.c | 6 ++----
fs/xfs/xfs_attr_leaf.c | 3 +--
fs/xfs/xfs_btree.c | 8 ++++----
fs/xfs/xfs_buf.h | 7 +++++++
fs/xfs/xfs_da_btree.c | 3 +--
fs/xfs/xfs_dir2_block.c | 3 +--
fs/xfs/xfs_dir2_data.c | 3 +--
fs/xfs/xfs_dir2_leaf.c | 3 +--
fs/xfs/xfs_dir2_node.c | 3 +--
fs/xfs/xfs_ialloc.c | 4 ++--
fs/xfs/xfs_linux.h | 1 +
fs/xfs/xfs_sb.c | 3 +--
fs/xfs/xfs_symlink_remote.c | 3 +--
13 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 72ea855..5050c9a 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -485,8 +485,7 @@ xfs_agfl_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_AGFL_CRC_OFF);
+ agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
agfl_ok = agfl_ok && xfs_agfl_verify(bp);
@@ -2240,8 +2239,7 @@ xfs_agf_read_verify(
int agf_ok = 1;
if (xfs_sb_version_hascrc(&mp->m_sb))
- agf_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_AGF_CRC_OFF);
+ agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
agf_ok = agf_ok && xfs_agf_verify(mp, bp);
diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
index 7b126f4..a19a023 100644
--- a/fs/xfs/xfs_attr_leaf.c
+++ b/fs/xfs/xfs_attr_leaf.c
@@ -240,8 +240,7 @@ xfs_attr3_leaf_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_ATTR3_LEAF_CRC_OFF)) ||
+ !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
!xfs_attr3_leaf_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
diff --git a/fs/xfs/xfs_btree.c b/fs/xfs/xfs_btree.c
index 9adaae4..4e8524d 100644
--- a/fs/xfs/xfs_btree.c
+++ b/fs/xfs/xfs_btree.c
@@ -243,8 +243,8 @@ xfs_btree_lblock_verify_crc(
struct xfs_buf *bp)
{
if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb))
- return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_BTREE_LBLOCK_CRC_OFF);
+ return xfs_buf_verify_cksum(bp, XFS_BTREE_LBLOCK_CRC_OFF);
+
return true;
}
@@ -276,8 +276,8 @@ xfs_btree_sblock_verify_crc(
struct xfs_buf *bp)
{
if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb))
- return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_BTREE_SBLOCK_CRC_OFF);
+ return xfs_buf_verify_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);
+
return true;
}
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 9953395..5edcfba 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -369,6 +369,13 @@ static inline void xfs_buf_relse(xfs_buf_t *bp)
xfs_buf_rele(bp);
}
+static inline int
+xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
+{
+ return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
+ cksum_offset);
+}
+
/*
* Handling of buftargs.
*/
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 796272a..6cece55 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -214,8 +214,7 @@ xfs_da3_node_read_verify(
switch (be16_to_cpu(info->magic)) {
case XFS_DA3_NODE_MAGIC:
- if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_DA3_NODE_CRC_OFF))
+ if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
break;
/* fall through */
case XFS_DA_NODE_MAGIC:
diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
index 90cdbf4..948dc39 100644
--- a/fs/xfs/xfs_dir2_block.c
+++ b/fs/xfs/xfs_dir2_block.c
@@ -90,8 +90,7 @@ xfs_dir3_block_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_DIR3_DATA_CRC_OFF)) ||
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
!xfs_dir3_block_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
index 70acff4..1952f00 100644
--- a/fs/xfs/xfs_dir2_data.c
+++ b/fs/xfs/xfs_dir2_data.c
@@ -268,8 +268,7 @@ xfs_dir3_data_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_DIR3_DATA_CRC_OFF)) ||
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
!xfs_dir3_data_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
index ae47ec6..1a412eb 100644
--- a/fs/xfs/xfs_dir2_leaf.c
+++ b/fs/xfs/xfs_dir2_leaf.c
@@ -180,8 +180,7 @@ __read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_DIR3_LEAF_CRC_OFF)) ||
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
!xfs_dir3_leaf_verify(bp, magic)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 48c7d18..875e7c0 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -116,8 +116,7 @@ xfs_dir3_free_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_DIR3_FREE_CRC_OFF)) ||
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
!xfs_dir3_free_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index d79210b..d6a879d 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1571,8 +1571,8 @@ xfs_agi_read_verify(
int agi_ok = 1;
if (xfs_sb_version_hascrc(&mp->m_sb))
- agi_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_AGI_CRC_OFF);
+ agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
+
agi_ok = agi_ok && xfs_agi_verify(bp);
if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index f9bb590..e8fed74 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -119,6 +119,7 @@ typedef __uint64_t __psunsigned_t;
#include "xfs_iops.h"
#include "xfs_aops.h"
#include "xfs_super.h"
+#include "xfs_cksum.h"
#include "xfs_buf.h"
#include "xfs_message.h"
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 1ea7c86..36f287f 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -610,8 +610,7 @@ xfs_sb_read_verify(
XFS_SB_VERSION_5) ||
dsb->sb_crc != 0)) {
- if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_SB_CRC_OFF)) {
+ if (!xfs_buf_verify_cksum(bp, XFS_SB_CRC_OFF)) {
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
xfs_sb_version_hascrc(&mp->m_sb)) {
diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
index 7a705a4..b172955 100644
--- a/fs/xfs/xfs_symlink_remote.c
+++ b/fs/xfs/xfs_symlink_remote.c
@@ -133,8 +133,7 @@ xfs_symlink_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_SYMLINK_CRC_OFF) ||
+ if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
!xfs_symlink_verify(bp)) {
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 6/9] xfs: add helper for updating checksums on xfs_bufs
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (4 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-18 23:52 ` [PATCH 7/9] xfs: add xfs_verifier_error() Eric Sandeen
` (3 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Many/most callers of xfs_update_cksum() pass bp->b_addr and
BBTOB(bp->b_length) as the first 2 args. Add a helper
which can just accept the bp and the crc offset, and work
it out on its own, for brevity.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_alloc.c | 4 ++--
fs/xfs/xfs_attr_leaf.c | 2 +-
fs/xfs/xfs_btree.c | 6 ++----
fs/xfs/xfs_buf.h | 7 +++++++
fs/xfs/xfs_da_btree.c | 2 +-
fs/xfs/xfs_dir2_block.c | 2 +-
fs/xfs/xfs_dir2_data.c | 2 +-
fs/xfs/xfs_dir2_leaf.c | 2 +-
fs/xfs/xfs_dir2_node.c | 2 +-
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_sb.c | 2 +-
fs/xfs/xfs_symlink_remote.c | 2 +-
12 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 5050c9a..9c7cf3d 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -515,7 +515,7 @@ xfs_agfl_write_verify(
if (bip)
XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGFL_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_AGFL_CRC_OFF);
}
const struct xfs_buf_ops xfs_agfl_buf_ops = {
@@ -2269,7 +2269,7 @@ xfs_agf_write_verify(
if (bip)
XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGF_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_AGF_CRC_OFF);
}
const struct xfs_buf_ops xfs_agf_buf_ops = {
diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
index a19a023..b552378 100644
--- a/fs/xfs/xfs_attr_leaf.c
+++ b/fs/xfs/xfs_attr_leaf.c
@@ -224,7 +224,7 @@ xfs_attr3_leaf_write_verify(
if (bip)
hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_ATTR3_LEAF_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF);
}
/*
diff --git a/fs/xfs/xfs_btree.c b/fs/xfs/xfs_btree.c
index 4e8524d..e80d59f 100644
--- a/fs/xfs/xfs_btree.c
+++ b/fs/xfs/xfs_btree.c
@@ -234,8 +234,7 @@ xfs_btree_lblock_calc_crc(
return;
if (bip)
block->bb_u.l.bb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_BTREE_LBLOCK_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_BTREE_LBLOCK_CRC_OFF);
}
bool
@@ -267,8 +266,7 @@ xfs_btree_sblock_calc_crc(
return;
if (bip)
block->bb_u.s.bb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
- XFS_BTREE_SBLOCK_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);
}
bool
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 5edcfba..b8a3abf 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -376,6 +376,13 @@ xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
cksum_offset);
}
+static inline void
+xfs_buf_update_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
+{
+ xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
+ cksum_offset);
+}
+
/*
* Handling of buftargs.
*/
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 6cece55..75ef990 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -196,7 +196,7 @@ xfs_da3_node_write_verify(
if (bip)
hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DA3_NODE_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_DA3_NODE_CRC_OFF);
}
/*
diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
index 948dc39..724377e 100644
--- a/fs/xfs/xfs_dir2_block.c
+++ b/fs/xfs/xfs_dir2_block.c
@@ -117,7 +117,7 @@ xfs_dir3_block_write_verify(
if (bip)
hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_DATA_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_DIR3_DATA_CRC_OFF);
}
const struct xfs_buf_ops xfs_dir3_block_buf_ops = {
diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
index 1952f00..74ae85e 100644
--- a/fs/xfs/xfs_dir2_data.c
+++ b/fs/xfs/xfs_dir2_data.c
@@ -295,7 +295,7 @@ xfs_dir3_data_write_verify(
if (bip)
hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_DATA_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_DIR3_DATA_CRC_OFF);
}
const struct xfs_buf_ops xfs_dir3_data_buf_ops = {
diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
index 1a412eb..dffb61b 100644
--- a/fs/xfs/xfs_dir2_leaf.c
+++ b/fs/xfs/xfs_dir2_leaf.c
@@ -208,7 +208,7 @@ __write_verify(
if (bip)
hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_LEAF_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_DIR3_LEAF_CRC_OFF);
}
static void
diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 875e7c0..0904b20 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -143,7 +143,7 @@ xfs_dir3_free_write_verify(
if (bip)
hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_FREE_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_DIR3_FREE_CRC_OFF);
}
const struct xfs_buf_ops xfs_dir3_free_buf_ops = {
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index d6a879d..4657586 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1600,7 +1600,7 @@ xfs_agi_write_verify(
if (bip)
XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGI_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_AGI_CRC_OFF);
}
const struct xfs_buf_ops xfs_agi_buf_ops = {
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 36f287f..818359f 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -673,7 +673,7 @@ xfs_sb_write_verify(
if (bip)
XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SB_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
}
const struct xfs_buf_ops xfs_sb_buf_ops = {
diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
index b172955..defa09f 100644
--- a/fs/xfs/xfs_symlink_remote.c
+++ b/fs/xfs/xfs_symlink_remote.c
@@ -161,7 +161,7 @@ xfs_symlink_write_verify(
struct xfs_dsymlink_hdr *dsl = bp->b_addr;
dsl->sl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
}
- xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SYMLINK_CRC_OFF);
+ xfs_buf_update_cksum(bp, XFS_SYMLINK_CRC_OFF);
}
const struct xfs_buf_ops xfs_symlink_buf_ops = {
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 7/9] xfs: add xfs_verifier_error()
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (5 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 6/9] xfs: add helper for updating " Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 6:30 ` Dave Chinner
2014-02-20 2:58 ` [PATCH 7/9 V2] " Eric Sandeen
2014-02-18 23:52 ` [PATCH 8/9] xfs: print useful caller information in xfs_error_report Eric Sandeen
` (2 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
We want to distinguish between corruption, CRC errors,
etc. In addition, the full stack trace on verifier errors
seems less than helpful; it looks more like an oops than
corruption.
Create a new function to specifically alert the user to
verifier errors, which can differentiate between
EFSCORRUPTED and CRC mismatches. It doesn't dump stack
unless the xfs error level is turned up high.
Define a new error message (EFSBADCRC) to clearly identify
CRC errors. (Defined to EILSEQ, bad byte sequence)
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_error.c | 26 ++++++++++++++++++++++++++
fs/xfs/xfs_error.h | 1 +
fs/xfs/xfs_linux.h | 1 +
3 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 9995b80..8733c59 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -178,3 +178,29 @@ xfs_corruption_error(
xfs_error_report(tag, level, mp, filename, linenum, ra);
xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
}
+
+/*
+ * Warnings specifically for verifier errors. Differentiate CRC vs. invalid
+ * values, and omit the stack trace unless the error level is tuned high.
+ */
+void
+xfs_verifier_error(
+ struct xfs_buf *bp)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+
+ xfs_alert(mp, "Metadata %s detected at %pF, block 0x%llx",
+ bp->b_error == EFSBADCRC ? "CRC error" : "corruption",
+ __return_address, bp->b_bn);
+
+ xfs_alert(mp, "Unmount and run xfs_repair");
+
+ /* XXX handle page-mapped buffers too? */
+ if (xfs_error_level >= XFS_ERRLEVEL_LOW && bp->b_addr) {
+ xfs_alert(mp, "First 64 bytes of corrupted metadata buffer:");
+ xfs_hex_dump(bp->b_addr, 64);
+ }
+
+ if (xfs_error_level >= XFS_ERRLEVEL_HIGH)
+ xfs_stack_trace();
+}
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 079a367..c1c57d4 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -34,6 +34,7 @@ extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
extern void xfs_corruption_error(const char *tag, int level,
struct xfs_mount *mp, void *p, const char *filename,
int linenum, inst_t *ra);
+extern void xfs_verifier_error(struct xfs_buf *bp);
#define XFS_ERROR_REPORT(e, lvl, mp) \
xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index e8fed74..016ea8d 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -179,6 +179,7 @@ typedef __uint64_t __psunsigned_t;
#define ENOATTR ENODATA /* Attribute not found */
#define EWRONGFS EINVAL /* Mount with wrong filesystem type */
#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EILSEQ /* Bad CRC detected */
#define SYNCHRONIZE() barrier()
#define __return_address __builtin_return_address(0)
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 8/9] xfs: print useful caller information in xfs_error_report
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (6 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 7/9] xfs: add xfs_verifier_error() Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 12:42 ` Jeff Liu
2014-02-18 23:52 ` [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors Eric Sandeen
2014-02-27 9:12 ` [PATCH 0/9] current series for verifier error differentiation Dave Chinner
9 siblings, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
xfs_error_report used to just print the hex address of the caller;
%pF will give us something more human-readable.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_error.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 8733c59..04d8e65 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -156,7 +156,7 @@ xfs_error_report(
{
if (level <= xfs_error_level) {
xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
- "Internal error %s at line %d of file %s. Caller 0x%p",
+ "Internal error %s at line %d of file %s. Caller %pF",
tag, linenum, filename, ra);
xfs_stack_trace();
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (7 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 8/9] xfs: print useful caller information in xfs_error_report Eric Sandeen
@ 2014-02-18 23:52 ` Eric Sandeen
2014-02-19 14:01 ` Brian Foster
2014-02-20 3:10 ` [PATCH 9/9 V2] " Eric Sandeen
2014-02-27 9:12 ` [PATCH 0/9] current series for verifier error differentiation Dave Chinner
9 siblings, 2 replies; 28+ messages in thread
From: Eric Sandeen @ 2014-02-18 23:52 UTC (permalink / raw)
To: xfs
Modify all read & write verifiers to differentiate
between CRC errors and other inconsistencies.
This sets the appropriate error number on bp->b_error,
and then calls xfs_verifier_error() if something went
wrong. That function will issue the appropriate message
to the user.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_alloc.c | 37 +++++++++++++++++--------------------
fs/xfs/xfs_alloc_btree.c | 15 ++++++++-------
fs/xfs/xfs_attr_leaf.c | 14 ++++++++------
fs/xfs/xfs_attr_remote.c | 15 ++++++---------
fs/xfs/xfs_bmap_btree.c | 16 ++++++++--------
fs/xfs/xfs_da_btree.c | 14 ++++++++------
fs/xfs/xfs_dir2_block.c | 14 ++++++++------
fs/xfs/xfs_dir2_data.c | 17 +++++++++--------
fs/xfs/xfs_dir2_leaf.c | 14 ++++++++------
fs/xfs/xfs_dir2_node.c | 14 ++++++++------
fs/xfs/xfs_dquot_buf.c | 11 +++++++----
fs/xfs/xfs_ialloc.c | 12 ++++++++----
fs/xfs/xfs_ialloc_btree.c | 15 ++++++++-------
fs/xfs/xfs_inode_buf.c | 3 +--
fs/xfs/xfs_sb.c | 10 ++++------
fs/xfs/xfs_symlink_remote.c | 12 +++++++-----
16 files changed, 123 insertions(+), 110 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 9c7cf3d..9a93601 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -474,7 +474,6 @@ xfs_agfl_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- int agfl_ok = 1;
/*
* There is no verification of non-crc AGFLs because mkfs does not
@@ -485,14 +484,13 @@ xfs_agfl_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
-
- agfl_ok = agfl_ok && xfs_agfl_verify(bp);
-
- if (!agfl_ok) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_buf_verify_cksum(bp, offsetof(struct xfs_agfl, agfl_crc)))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_agfl_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -507,8 +505,8 @@ xfs_agfl_write_verify(
return;
if (!xfs_agfl_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -2236,18 +2234,17 @@ xfs_agf_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- int agf_ok = 1;
-
- if (xfs_sb_version_hascrc(&mp->m_sb))
- agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
- agf_ok = agf_ok && xfs_agf_verify(mp, bp);
-
- if (unlikely(XFS_TEST_ERROR(!agf_ok, mp, XFS_ERRTAG_ALLOC_READ_AGF,
- XFS_RANDOM_ALLOC_READ_AGF))) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, offsetof(struct xfs_agf, agf_crc)))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
+ XFS_ERRTAG_ALLOC_READ_AGF,
+ XFS_RANDOM_ALLOC_READ_AGF))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -2258,8 +2255,8 @@ xfs_agf_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
if (!xfs_agf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index 144d3b0..cc1eadc 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -355,12 +355,14 @@ static void
xfs_allocbt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_sblock_verify_crc(bp) &&
- xfs_allocbt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_allocbt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -370,9 +372,8 @@ xfs_allocbt_write_verify(
{
if (!xfs_allocbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
index b552378..fe9587f 100644
--- a/fs/xfs/xfs_attr_leaf.c
+++ b/fs/xfs/xfs_attr_leaf.c
@@ -213,8 +213,8 @@ xfs_attr3_leaf_write_verify(
struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
if (!xfs_attr3_leaf_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -239,12 +239,14 @@ xfs_attr3_leaf_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
- !xfs_attr3_leaf_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_attr3_leaf_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
diff --git a/fs/xfs/xfs_attr_remote.c b/fs/xfs/xfs_attr_remote.c
index 5549d69..6e37823 100644
--- a/fs/xfs/xfs_attr_remote.c
+++ b/fs/xfs/xfs_attr_remote.c
@@ -125,7 +125,6 @@ xfs_attr3_rmt_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
char *ptr;
int len;
- bool corrupt = false;
xfs_daddr_t bno;
/* no verification of non-crc buffers */
@@ -140,11 +139,11 @@ xfs_attr3_rmt_read_verify(
while (len > 0) {
if (!xfs_verify_cksum(ptr, XFS_LBSIZE(mp),
XFS_ATTR3_RMT_CRC_OFF)) {
- corrupt = true;
+ xfs_buf_ioerror(bp, EFSBADCRC);
break;
}
if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
- corrupt = true;
+ xfs_buf_ioerror(bp, EFSCORRUPTED);
break;
}
len -= XFS_LBSIZE(mp);
@@ -152,10 +151,9 @@ xfs_attr3_rmt_read_verify(
bno += mp->m_bsize;
}
- if (corrupt) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
- xfs_buf_ioerror(bp, EFSCORRUPTED);
- } else
+ if (bp->b_error)
+ xfs_verifier_error(bp);
+ else
ASSERT(len == 0);
}
@@ -180,9 +178,8 @@ xfs_attr3_rmt_write_verify(
while (len > 0) {
if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
- XFS_CORRUPTION_ERROR(__func__,
- XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
if (bip) {
diff --git a/fs/xfs/xfs_bmap_btree.c b/fs/xfs/xfs_bmap_btree.c
index 706bc3f..818d546 100644
--- a/fs/xfs/xfs_bmap_btree.c
+++ b/fs/xfs/xfs_bmap_btree.c
@@ -780,12 +780,14 @@ static void
xfs_bmbt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_lblock_verify_crc(bp) &&
- xfs_bmbt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_lblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_bmbt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -794,11 +796,9 @@ xfs_bmbt_write_verify(
struct xfs_buf *bp)
{
if (!xfs_bmbt_verify(bp)) {
- xfs_warn(bp->b_target->bt_mount, "bmbt daddr 0x%llx failed", bp->b_bn);
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_lblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 75ef990..1f5af79 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -185,8 +185,8 @@ xfs_da3_node_write_verify(
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
if (!xfs_da3_node_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -209,17 +209,20 @@ static void
xfs_da3_node_read_verify(
struct xfs_buf *bp)
{
- struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_da_blkinfo *info = bp->b_addr;
switch (be16_to_cpu(info->magic)) {
case XFS_DA3_NODE_MAGIC:
- if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
+ if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF)) {
+ xfs_buf_ioerror(bp, EFSBADCRC);
break;
+ }
/* fall through */
case XFS_DA_NODE_MAGIC:
- if (!xfs_da3_node_verify(bp))
+ if (!xfs_da3_node_verify(bp)) {
+ xfs_buf_ioerror(bp, EFSCORRUPTED);
break;
+ }
return;
case XFS_ATTR_LEAF_MAGIC:
case XFS_ATTR3_LEAF_MAGIC:
@@ -236,8 +239,7 @@ xfs_da3_node_read_verify(
}
/* corrupt block */
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
- xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
}
const struct xfs_buf_ops xfs_da3_node_buf_ops = {
diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
index 724377e..4f6a38c 100644
--- a/fs/xfs/xfs_dir2_block.c
+++ b/fs/xfs/xfs_dir2_block.c
@@ -89,12 +89,14 @@ xfs_dir3_block_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
- !xfs_dir3_block_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_block_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -106,8 +108,8 @@ xfs_dir3_block_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_block_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
index 74ae85e..afa4ad5 100644
--- a/fs/xfs/xfs_dir2_data.c
+++ b/fs/xfs/xfs_dir2_data.c
@@ -241,7 +241,6 @@ static void
xfs_dir3_data_reada_verify(
struct xfs_buf *bp)
{
- struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_dir2_data_hdr *hdr = bp->b_addr;
switch (hdr->magic) {
@@ -255,8 +254,8 @@ xfs_dir3_data_reada_verify(
xfs_dir3_data_verify(bp);
return;
default:
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, hdr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
break;
}
}
@@ -267,12 +266,14 @@ xfs_dir3_data_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
- !xfs_dir3_data_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_data_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -284,8 +285,8 @@ xfs_dir3_data_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_data_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
index dffb61b..d36e97d 100644
--- a/fs/xfs/xfs_dir2_leaf.c
+++ b/fs/xfs/xfs_dir2_leaf.c
@@ -179,12 +179,14 @@ __read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
- !xfs_dir3_leaf_verify(bp, magic)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_leaf_verify(bp, magic))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -197,8 +199,8 @@ __write_verify(
struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_leaf_verify(bp, magic)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 0904b20..cb434d7 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -115,12 +115,14 @@ xfs_dir3_free_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
- !xfs_dir3_free_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_free_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -132,8 +134,8 @@ xfs_dir3_free_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_free_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dquot_buf.c b/fs/xfs/xfs_dquot_buf.c
index d401457..610da81 100644
--- a/fs/xfs/xfs_dquot_buf.c
+++ b/fs/xfs/xfs_dquot_buf.c
@@ -257,10 +257,13 @@ xfs_dquot_buf_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if (!xfs_dquot_buf_verify_crc(mp, bp) || !xfs_dquot_buf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_dquot_buf_verify_crc(mp, bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dquot_buf_verify(mp, bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
/*
@@ -275,8 +278,8 @@ xfs_dquot_buf_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if (!xfs_dquot_buf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
}
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 4657586..8aa720d 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1573,13 +1573,17 @@ xfs_agi_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb))
agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
+ if (!agi_ok)
+ xfs_buf_ioerror(bp, EFSBADCRC);
+
agi_ok = agi_ok && xfs_agi_verify(bp);
if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
- XFS_RANDOM_IALLOC_READ_AGI))) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ XFS_RANDOM_IALLOC_READ_AGI)))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -1590,8 +1594,8 @@ xfs_agi_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
if (!xfs_agi_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
index 0028c50..7e309b1 100644
--- a/fs/xfs/xfs_ialloc_btree.c
+++ b/fs/xfs/xfs_ialloc_btree.c
@@ -243,12 +243,14 @@ static void
xfs_inobt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_sblock_verify_crc(bp) &&
- xfs_inobt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_inobt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -258,9 +260,8 @@ xfs_inobt_write_verify(
{
if (!xfs_inobt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
index 606b43a..24e9939 100644
--- a/fs/xfs/xfs_inode_buf.c
+++ b/fs/xfs/xfs_inode_buf.c
@@ -102,8 +102,7 @@ xfs_inode_buf_verify(
}
xfs_buf_ioerror(bp, EFSCORRUPTED);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_HIGH,
- mp, dip);
+ xfs_verifier_error(bp);
#ifdef DEBUG
xfs_alert(mp,
"bad inode magic/vsn daddr %lld #%d (magic=%x)",
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 818359f..b134aa8 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -614,7 +614,7 @@ xfs_sb_read_verify(
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
xfs_sb_version_hascrc(&mp->m_sb)) {
- error = EFSCORRUPTED;
+ error = EFSBADCRC;
goto out_error;
}
}
@@ -623,10 +623,9 @@ xfs_sb_read_verify(
out_error:
if (error) {
- if (error == EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- mp, bp->b_addr);
xfs_buf_ioerror(bp, error);
+ if (error == EFSCORRUPTED || error == EFSBADCRC)
+ xfs_verifier_error(bp);
}
}
@@ -661,9 +660,8 @@ xfs_sb_write_verify(
error = xfs_sb_verify(bp, false);
if (error) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- mp, bp->b_addr);
xfs_buf_ioerror(bp, error);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
index defa09f..9b32052 100644
--- a/fs/xfs/xfs_symlink_remote.c
+++ b/fs/xfs/xfs_symlink_remote.c
@@ -133,11 +133,13 @@ xfs_symlink_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
- !xfs_symlink_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_symlink_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -152,8 +154,8 @@ xfs_symlink_write_verify(
return;
if (!xfs_symlink_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] xfs: skip verification on initial "guess" superblock read
2014-02-18 23:52 ` [PATCH 1/9] xfs: skip verification on initial "guess" superblock read Eric Sandeen
@ 2014-02-19 3:36 ` Dave Chinner
0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-19 3:36 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Tue, Feb 18, 2014 at 05:52:21PM -0600, Eric Sandeen wrote:
> When xfs_readsb() does the very first read of the superblock,
> it makes a guess at the length of the buffer, based on the
> sector size of the underlying storage. This may or may
> not match the filesystem sector size in sb_sectsize, so
> we can't i.e. do a CRC check on it; it might be too short.
>
> In fact, mounting a filesystem with sb_sectsize larger
> than the device sector size will cause a mount failure
> if CRCs are enabled, because we are checksumming a length
> which exceeds the buffer passed to it.
>
> So always read twice; the first time we read with NULL
> buffer ops to skip verification; then set the proper
> read length, hook up the proper verifier, and give it
> another go.
>
> Once we are sure that we've got the right buffer length,
> we can also use bp->b_length in the xfs_sb_read_verify,
> rather than the less-trusted on-disk sectorsize for
> secondary superblocks. Before this we ran the risk of
> passing junk to the crc32c routines, which didn't always
> handle extreme values.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption
2014-02-18 23:52 ` [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption Eric Sandeen
@ 2014-02-19 3:37 ` Dave Chinner
0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-19 3:37 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Tue, Feb 18, 2014 at 05:52:22PM -0600, Eric Sandeen wrote:
> Today, if
>
> xfs_sb_read_verify
> xfs_sb_verify
> xfs_mount_validate_sb
>
> detects superblock corruption, it'll be extremely noisy, dumping
> 2 stacks, 2 hexdumps, etc.
>
> This is because we call XFS_CORRUPTION_ERROR in xfs_mount_validate_sb
> as well as in xfs_sb_read_verify.
>
> Also, *any* errors in xfs_mount_validate_sb which are not corruption
> per se; things like too-big-blocksize, bad version, bad magic, v1 dirs,
> rw-incompat etc - things which do not return EFSCORRUPTED - will
> still do the whole XFS_CORRUPTION_ERROR spew when xfs_sb_read_verify
> sees any error at all. And it suggests to the user that they
> should run xfs_repair, even if the root cause of the mount failure
> is a simple incompatibility.
>
> I'll submit that the probably-not-corrupted errors don't warrant
> this much noise, so this patch removes the warning for anything
> other than EFSCORRUPTED returns, and replaces the lower-level
> XFS_CORRUPTION_ERROR with an xfs_notice().
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Agreed. Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 7/9] xfs: add xfs_verifier_error()
2014-02-18 23:52 ` [PATCH 7/9] xfs: add xfs_verifier_error() Eric Sandeen
@ 2014-02-19 6:30 ` Dave Chinner
2014-02-20 2:58 ` [PATCH 7/9 V2] " Eric Sandeen
1 sibling, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-19 6:30 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Tue, Feb 18, 2014 at 05:52:27PM -0600, Eric Sandeen wrote:
> We want to distinguish between corruption, CRC errors,
> etc. In addition, the full stack trace on verifier errors
> seems less than helpful; it looks more like an oops than
> corruption.
>
> Create a new function to specifically alert the user to
> verifier errors, which can differentiate between
> EFSCORRUPTED and CRC mismatches. It doesn't dump stack
> unless the xfs error level is turned up high.
>
> Define a new error message (EFSBADCRC) to clearly identify
> CRC errors. (Defined to EILSEQ, bad byte sequence)
I think we decided the other best candidates were:
EBADE: Invalid exchange
EPROTO: Protocol error
EBADMSG: Bad message
EKEYREJECTED: Key was rejected by service
because of the silly glibc translation of EILSEQ:
EILSEQ: Invalid or incomplete multibyte or wide character
I'm leaning towards EBADMSG as the one to use here.
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_error.c | 26 ++++++++++++++++++++++++++
> fs/xfs/xfs_error.h | 1 +
> fs/xfs/xfs_linux.h | 1 +
> 3 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 9995b80..8733c59 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -178,3 +178,29 @@ xfs_corruption_error(
> xfs_error_report(tag, level, mp, filename, linenum, ra);
> xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
> }
> +
> +/*
> + * Warnings specifically for verifier errors. Differentiate CRC vs. invalid
> + * values, and omit the stack trace unless the error level is tuned high.
> + */
> +void
> +xfs_verifier_error(
> + struct xfs_buf *bp)
> +{
> + struct xfs_mount *mp = bp->b_target->bt_mount;
> +
> + xfs_alert(mp, "Metadata %s detected at %pF, block 0x%llx",
> + bp->b_error == EFSBADCRC ? "CRC error" : "corruption",
> + __return_address, bp->b_bn);
> +
> + xfs_alert(mp, "Unmount and run xfs_repair");
> +
> + /* XXX handle page-mapped buffers too? */
> + if (xfs_error_level >= XFS_ERRLEVEL_LOW && bp->b_addr) {
> + xfs_alert(mp, "First 64 bytes of corrupted metadata buffer:");
> + xfs_hex_dump(bp->b_addr, 64);
> + }
Just use xfs_buf_offset(bp, 0) here, and you don't have to care
about how the buffer is mapped or allocated.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures
2014-02-18 23:52 ` [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures Eric Sandeen
@ 2014-02-19 6:35 ` Jeff Liu
0 siblings, 0 replies; 28+ messages in thread
From: Jeff Liu @ 2014-02-19 6:35 UTC (permalink / raw)
To: Eric Sandeen, xfs
On 02/19 2014 07:52 AM, Eric Sandeen wrote:
> Most write verifiers don't update CRCs after the verifier
> has failed and the buffer has been marked in error. These
> two didn't, but should.
>
> Add returns to the verifier failure block,
> since the buffer won't be written anyway.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_alloc_btree.c | 1 +
> fs/xfs/xfs_ialloc_btree.c | 1 +
> 2 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
> index 1308542..144d3b0 100644
> --- a/fs/xfs/xfs_alloc_btree.c
> +++ b/fs/xfs/xfs_alloc_btree.c
> @@ -373,6 +373,7 @@ xfs_allocbt_write_verify(
> XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + return;
> }
> xfs_btree_sblock_calc_crc(bp);
>
> diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
> index c8fa5bb..0028c50 100644
> --- a/fs/xfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/xfs_ialloc_btree.c
> @@ -261,6 +261,7 @@ xfs_inobt_write_verify(
> XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + return;
> }
> xfs_btree_sblock_calc_crc(bp);
Looks good to me.
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-18 23:52 ` [PATCH 4/9] xfs: Use defines for CRC offsets in all cases Eric Sandeen
@ 2014-02-19 7:56 ` Jeff Liu
2014-02-20 0:27 ` Dave Chinner
0 siblings, 1 reply; 28+ messages in thread
From: Jeff Liu @ 2014-02-19 7:56 UTC (permalink / raw)
To: Eric Sandeen, xfs
Hi Eric,
I read the previous comments from Dave about using defines for CRC offsets,
and with a grep search after applying this patch, looks there have another
two places maybe we should switch them to the macros as well:
fs/xfs/xfs_log.c:
Do we need a log record crc offset macros for offsetof(struct xlog_rec_header, h_crc))?
xfs_dinode.h:
we added the XFS_DINODE_CRC_OFF, just use it at below routine?
static inline uint xfs_dinode_size(int version)
{
if (version == 3)
return sizeof(struct xfs_dinode);
return offsetof(struct xfs_dinode, di_crc);
}
Thanks,
-Jeff
On 02/19 2014 07:52 AM, Eric Sandeen wrote:
> Some calls to crc functions used useful #defines,
> others used awkward offsetof() constructs.
>
> Switch them all to #define to make things a bit
> cleaner.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_ag.h | 6 ++++++
> fs/xfs/xfs_alloc.c | 10 ++++------
> fs/xfs/xfs_dinode.h | 2 ++
> fs/xfs/xfs_format.h | 2 ++
> fs/xfs/xfs_ialloc.c | 5 ++---
> fs/xfs/xfs_inode_buf.c | 4 ++--
> fs/xfs/xfs_sb.c | 5 ++---
> fs/xfs/xfs_sb.h | 2 ++
> fs/xfs/xfs_symlink_remote.c | 5 ++---
> 9 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
> index 3fc1098..0fdd410 100644
> --- a/fs/xfs/xfs_ag.h
> +++ b/fs/xfs/xfs_ag.h
> @@ -89,6 +89,8 @@ typedef struct xfs_agf {
> /* structure must be padded to 64 bit alignment */
> } xfs_agf_t;
>
> +#define XFS_AGF_CRC_OFF offsetof(struct xfs_agf, agf_crc)
> +
> #define XFS_AGF_MAGICNUM 0x00000001
> #define XFS_AGF_VERSIONNUM 0x00000002
> #define XFS_AGF_SEQNO 0x00000004
> @@ -167,6 +169,8 @@ typedef struct xfs_agi {
> /* structure must be padded to 64 bit alignment */
> } xfs_agi_t;
>
> +#define XFS_AGI_CRC_OFF offsetof(struct xfs_agi, agi_crc)
> +
> #define XFS_AGI_MAGICNUM 0x00000001
> #define XFS_AGI_VERSIONNUM 0x00000002
> #define XFS_AGI_SEQNO 0x00000004
> @@ -222,6 +226,8 @@ typedef struct xfs_agfl {
> __be32 agfl_bno[]; /* actually XFS_AGFL_SIZE(mp) */
> } xfs_agfl_t;
>
> +#define XFS_AGFL_CRC_OFF offsetof(struct xfs_agfl, agfl_crc)
> +
> /*
> * tags for inode radix tree
> */
> diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
> index 9eab2df..72ea855 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -486,7 +486,7 @@ xfs_agfl_read_verify(
> return;
>
> agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agfl, agfl_crc));
> + XFS_AGFL_CRC_OFF);
>
> agfl_ok = agfl_ok && xfs_agfl_verify(bp);
>
> @@ -516,8 +516,7 @@ xfs_agfl_write_verify(
> if (bip)
> XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
>
> - xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agfl, agfl_crc));
> + xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGFL_CRC_OFF);
> }
>
> const struct xfs_buf_ops xfs_agfl_buf_ops = {
> @@ -2242,7 +2241,7 @@ xfs_agf_read_verify(
>
> if (xfs_sb_version_hascrc(&mp->m_sb))
> agf_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agf, agf_crc));
> + XFS_AGF_CRC_OFF);
>
> agf_ok = agf_ok && xfs_agf_verify(mp, bp);
>
> @@ -2272,8 +2271,7 @@ xfs_agf_write_verify(
> if (bip)
> XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
>
> - xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agf, agf_crc));
> + xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGF_CRC_OFF);
> }
>
> const struct xfs_buf_ops xfs_agf_buf_ops = {
> diff --git a/fs/xfs/xfs_dinode.h b/fs/xfs/xfs_dinode.h
> index e5869b5..623bbe8 100644
> --- a/fs/xfs/xfs_dinode.h
> +++ b/fs/xfs/xfs_dinode.h
> @@ -89,6 +89,8 @@ typedef struct xfs_dinode {
> /* structure must be padded to 64 bit alignment */
> } xfs_dinode_t;
>
> +#define XFS_DINODE_CRC_OFF offsetof(struct xfs_dinode, di_crc)
> +
> #define DI_MAX_FLUSH 0xffff
>
> /*
> diff --git a/fs/xfs/xfs_format.h b/fs/xfs/xfs_format.h
> index b6ab5a3..9898f31 100644
> --- a/fs/xfs/xfs_format.h
> +++ b/fs/xfs/xfs_format.h
> @@ -145,6 +145,8 @@ struct xfs_dsymlink_hdr {
> __be64 sl_lsn;
> };
>
> +#define XFS_SYMLINK_CRC_OFF offsetof(struct xfs_dsymlink_hdr, sl_crc)
> +
> /*
> * The maximum pathlen is 1024 bytes. Since the minimum file system
> * blocksize is 512 bytes, we can get a max of 3 extents back from
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 5d7f105..d79210b 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -1572,7 +1572,7 @@ xfs_agi_read_verify(
>
> if (xfs_sb_version_hascrc(&mp->m_sb))
> agi_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agi, agi_crc));
> + XFS_AGI_CRC_OFF);
> agi_ok = agi_ok && xfs_agi_verify(bp);
>
> if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
> @@ -1600,8 +1600,7 @@ xfs_agi_write_verify(
>
> if (bip)
> XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
> - xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agi, agi_crc));
> + xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGI_CRC_OFF);
> }
>
> const struct xfs_buf_ops xfs_agi_buf_ops = {
> diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
> index 4fc9f39..606b43a 100644
> --- a/fs/xfs/xfs_inode_buf.c
> +++ b/fs/xfs/xfs_inode_buf.c
> @@ -306,7 +306,7 @@ xfs_dinode_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return false;
> if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
> - offsetof(struct xfs_dinode, di_crc)))
> + XFS_DINODE_CRC_OFF))
> return false;
> if (be64_to_cpu(dip->di_ino) != ip->i_ino)
> return false;
> @@ -327,7 +327,7 @@ xfs_dinode_calc_crc(
>
> ASSERT(xfs_sb_version_hascrc(&mp->m_sb));
> crc = xfs_start_cksum((char *)dip, mp->m_sb.sb_inodesize,
> - offsetof(struct xfs_dinode, di_crc));
> + XFS_DINODE_CRC_OFF);
> dip->di_crc = xfs_end_cksum(crc);
> }
>
> diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
> index 1e11679..1ea7c86 100644
> --- a/fs/xfs/xfs_sb.c
> +++ b/fs/xfs/xfs_sb.c
> @@ -611,7 +611,7 @@ xfs_sb_read_verify(
> dsb->sb_crc != 0)) {
>
> if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_sb, sb_crc))) {
> + XFS_SB_CRC_OFF)) {
> /* Only fail bad secondaries on a known V5 filesystem */
> if (bp->b_bn == XFS_SB_DADDR ||
> xfs_sb_version_hascrc(&mp->m_sb)) {
> @@ -674,8 +674,7 @@ xfs_sb_write_verify(
> if (bip)
> XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
>
> - xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_sb, sb_crc));
> + xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SB_CRC_OFF);
> }
>
> const struct xfs_buf_ops xfs_sb_buf_ops = {
> diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> index 35061d4..f7b2fe7 100644
> --- a/fs/xfs/xfs_sb.h
> +++ b/fs/xfs/xfs_sb.h
> @@ -182,6 +182,8 @@ typedef struct xfs_sb {
> /* must be padded to 64 bit alignment */
> } xfs_sb_t;
>
> +#define XFS_SB_CRC_OFF offsetof(struct xfs_sb, sb_crc)
> +
> /*
> * Superblock - on disk version. Must match the in core version above.
> * Must be padded to 64 bit alignment.
> diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
> index bf59a2b..7a705a4 100644
> --- a/fs/xfs/xfs_symlink_remote.c
> +++ b/fs/xfs/xfs_symlink_remote.c
> @@ -134,7 +134,7 @@ xfs_symlink_read_verify(
> return;
>
> if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_dsymlink_hdr, sl_crc)) ||
> + XFS_SYMLINK_CRC_OFF) ||
> !xfs_symlink_verify(bp)) {
> XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> @@ -162,8 +162,7 @@ xfs_symlink_write_verify(
> struct xfs_dsymlink_hdr *dsl = bp->b_addr;
> dsl->sl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
> }
> - xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_dsymlink_hdr, sl_crc));
> + xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SYMLINK_CRC_OFF);
> }
>
> const struct xfs_buf_ops xfs_symlink_buf_ops = {
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 8/9] xfs: print useful caller information in xfs_error_report
2014-02-18 23:52 ` [PATCH 8/9] xfs: print useful caller information in xfs_error_report Eric Sandeen
@ 2014-02-19 12:42 ` Jeff Liu
0 siblings, 0 replies; 28+ messages in thread
From: Jeff Liu @ 2014-02-19 12:42 UTC (permalink / raw)
To: Eric Sandeen, xfs
On 02/19 2014 07:52 AM, Eric Sandeen wrote:
> xfs_error_report used to just print the hex address of the caller;
> %pF will give us something more human-readable.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_error.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 8733c59..04d8e65 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -156,7 +156,7 @@ xfs_error_report(
> {
> if (level <= xfs_error_level) {
> xfs_alert_tag(mp, XFS_PTAG_ERROR_REPORT,
> - "Internal error %s at line %d of file %s. Caller 0x%p",
> + "Internal error %s at line %d of file %s. Caller %pF",
> tag, linenum, filename, ra);
>
> xfs_stack_trace();
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
In comparison with the hex code, this is obviously more readable as below.
Patched:
-------
XFS (sda7): Internal error xfs_sb_read_verify at line 630 of file fs/xfs/xfs_sb.c. Caller xfs_buf_iodone_work+0xa5/0xd0 [xfs]
<snip>
Call Trace:
[<ffffffff81741f15>] dump_stack+0x45/0x56
[<ffffffffa07a5a0b>] xfs_error_report+0x3b/0x40 [xfs]
[<ffffffffa07a26b5>] ? xfs_buf_iodone_work+0xa5/0xd0 [xfs]
[<ffffffffa07a5a65>] xfs_corruption_error+0x55/0x80 [xfs]
[<ffffffffa080bf23>] xfs_sb_read_verify+0x143/0x150 [xfs]
[<ffffffffa07a26b5>] ? xfs_buf_iodone_work+0xa5/0xd0 [xfs]
[<ffffffff8108744f>] ? process_one_work+0x1af/0x540
[<ffffffffa07a26b5>] xfs_buf_iodone_work+0xa5/0xd0 [xfs]
<snip>
Old:
----
[ 235.506754] XFS (sda7): Internal error xfs_sb_read_verify at line 630 of file fs/xfs/xfs_sb.c. Caller 0xffffffffa08aa6b5
<snip>
[ 235.506796] Call Trace:
[ 235.506802] [<ffffffff81741f15>] dump_stack+0x45/0x56
[ 235.506818] [<ffffffffa08ada0b>] xfs_error_report+0x3b/0x40 [xfs]
[ 235.506832] [<ffffffffa08aa6b5>] ? xfs_buf_iodone_work+0xa5/0xd0 [xfs]
[ 235.506845] [<ffffffffa08ada65>] xfs_corruption_error+0x55/0x80 [xfs]
[ 235.506871] [<ffffffffa0913f23>] xfs_sb_read_verify+0x143/0x150 [xfs]
[ 235.506885] [<ffffffffa08aa6b5>] ? xfs_buf_iodone_work+0xa5/0xd0 [xfs]
[ 235.506890] [<ffffffff8108744f>] ? process_one_work+0x1af/0x540
[ 235.506903] [<ffffffffa08aa6b5>] xfs_buf_iodone_work+0xa5/0xd0 [xfs]
<snip>
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors
2014-02-18 23:52 ` [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors Eric Sandeen
@ 2014-02-19 14:01 ` Brian Foster
2014-02-19 16:12 ` Eric Sandeen
2014-02-20 3:10 ` [PATCH 9/9 V2] " Eric Sandeen
1 sibling, 1 reply; 28+ messages in thread
From: Brian Foster @ 2014-02-19 14:01 UTC (permalink / raw)
To: Eric Sandeen, xfs
On 02/18/2014 06:52 PM, Eric Sandeen wrote:
> Modify all read & write verifiers to differentiate
> between CRC errors and other inconsistencies.
>
> This sets the appropriate error number on bp->b_error,
> and then calls xfs_verifier_error() if something went
> wrong. That function will issue the appropriate message
> to the user.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/xfs_alloc.c | 37 +++++++++++++++++--------------------
> fs/xfs/xfs_alloc_btree.c | 15 ++++++++-------
> fs/xfs/xfs_attr_leaf.c | 14 ++++++++------
> fs/xfs/xfs_attr_remote.c | 15 ++++++---------
> fs/xfs/xfs_bmap_btree.c | 16 ++++++++--------
> fs/xfs/xfs_da_btree.c | 14 ++++++++------
> fs/xfs/xfs_dir2_block.c | 14 ++++++++------
> fs/xfs/xfs_dir2_data.c | 17 +++++++++--------
> fs/xfs/xfs_dir2_leaf.c | 14 ++++++++------
> fs/xfs/xfs_dir2_node.c | 14 ++++++++------
> fs/xfs/xfs_dquot_buf.c | 11 +++++++----
> fs/xfs/xfs_ialloc.c | 12 ++++++++----
> fs/xfs/xfs_ialloc_btree.c | 15 ++++++++-------
> fs/xfs/xfs_inode_buf.c | 3 +--
> fs/xfs/xfs_sb.c | 10 ++++------
> fs/xfs/xfs_symlink_remote.c | 12 +++++++-----
> 16 files changed, 123 insertions(+), 110 deletions(-)
>
> diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
> index 9c7cf3d..9a93601 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -474,7 +474,6 @@ xfs_agfl_read_verify(
> struct xfs_buf *bp)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
> - int agfl_ok = 1;
>
> /*
> * There is no verification of non-crc AGFLs because mkfs does not
> @@ -485,14 +484,13 @@ xfs_agfl_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
> -
> - agfl_ok = agfl_ok && xfs_agfl_verify(bp);
> -
> - if (!agfl_ok) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (!xfs_buf_verify_cksum(bp, offsetof(struct xfs_agfl, agfl_crc)))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_agfl_verify(bp))
Obviously you added the CRC_OFF directives earlier in the set. It looks
like this patch squashed a couple of them (XFS_AGF_CRC_OFF as well).
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
...
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 4657586..8aa720d 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -1573,13 +1573,17 @@ xfs_agi_read_verify(
> if (xfs_sb_version_hascrc(&mp->m_sb))
> agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
>
> + if (!agi_ok)
> + xfs_buf_ioerror(bp, EFSBADCRC);
> +
> agi_ok = agi_ok && xfs_agi_verify(bp);
>
> if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
> - XFS_RANDOM_IALLOC_READ_AGI))) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + XFS_RANDOM_IALLOC_READ_AGI)))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
Any reason not to use the same if/else pattern here that the others are
now using (i.e., similar to xfs_agf_read_verify(), removing the need for
agi_ok)?
Brian
>
> static void
> @@ -1590,8 +1594,8 @@ xfs_agi_write_verify(
> struct xfs_buf_log_item *bip = bp->b_fspriv;
>
> if (!xfs_agi_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
> index 0028c50..7e309b1 100644
> --- a/fs/xfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/xfs_ialloc_btree.c
> @@ -243,12 +243,14 @@ static void
> xfs_inobt_read_verify(
> struct xfs_buf *bp)
> {
> - if (!(xfs_btree_sblock_verify_crc(bp) &&
> - xfs_inobt_verify(bp))) {
> - trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> + if (!xfs_btree_sblock_verify_crc(bp))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_inobt_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> +
> + if (bp->b_error) {
> + trace_xfs_btree_corrupt(bp, _RET_IP_);
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -258,9 +260,8 @@ xfs_inobt_write_verify(
> {
> if (!xfs_inobt_verify(bp)) {
> trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> xfs_btree_sblock_calc_crc(bp);
> diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
> index 606b43a..24e9939 100644
> --- a/fs/xfs/xfs_inode_buf.c
> +++ b/fs/xfs/xfs_inode_buf.c
> @@ -102,8 +102,7 @@ xfs_inode_buf_verify(
> }
>
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_HIGH,
> - mp, dip);
> + xfs_verifier_error(bp);
> #ifdef DEBUG
> xfs_alert(mp,
> "bad inode magic/vsn daddr %lld #%d (magic=%x)",
> diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
> index 818359f..b134aa8 100644
> --- a/fs/xfs/xfs_sb.c
> +++ b/fs/xfs/xfs_sb.c
> @@ -614,7 +614,7 @@ xfs_sb_read_verify(
> /* Only fail bad secondaries on a known V5 filesystem */
> if (bp->b_bn == XFS_SB_DADDR ||
> xfs_sb_version_hascrc(&mp->m_sb)) {
> - error = EFSCORRUPTED;
> + error = EFSBADCRC;
> goto out_error;
> }
> }
> @@ -623,10 +623,9 @@ xfs_sb_read_verify(
>
> out_error:
> if (error) {
> - if (error == EFSCORRUPTED)
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - mp, bp->b_addr);
> xfs_buf_ioerror(bp, error);
> + if (error == EFSCORRUPTED || error == EFSBADCRC)
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -661,9 +660,8 @@ xfs_sb_write_verify(
>
> error = xfs_sb_verify(bp, false);
> if (error) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - mp, bp->b_addr);
> xfs_buf_ioerror(bp, error);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
> index defa09f..9b32052 100644
> --- a/fs/xfs/xfs_symlink_remote.c
> +++ b/fs/xfs/xfs_symlink_remote.c
> @@ -133,11 +133,13 @@ xfs_symlink_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
> - !xfs_symlink_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_symlink_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -152,8 +154,8 @@ xfs_symlink_write_verify(
> return;
>
> if (!xfs_symlink_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors
2014-02-19 14:01 ` Brian Foster
@ 2014-02-19 16:12 ` Eric Sandeen
0 siblings, 0 replies; 28+ messages in thread
From: Eric Sandeen @ 2014-02-19 16:12 UTC (permalink / raw)
To: Brian Foster, Eric Sandeen, xfs
On 2/19/14, 8:01 AM, Brian Foster wrote:
> On 02/18/2014 06:52 PM, Eric Sandeen wrote:
>> Modify all read & write verifiers to differentiate
>> between CRC errors and other inconsistencies.
>>
>> This sets the appropriate error number on bp->b_error,
>> and then calls xfs_verifier_error() if something went
>> wrong. That function will issue the appropriate message
>> to the user.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
...
>> @@ -485,14 +484,13 @@ xfs_agfl_read_verify(
>> if (!xfs_sb_version_hascrc(&mp->m_sb))
>> return;
>>
>> - agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
>> -
>> - agfl_ok = agfl_ok && xfs_agfl_verify(bp);
>> -
>> - if (!agfl_ok) {
>> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
>> + if (!xfs_buf_verify_cksum(bp, offsetof(struct xfs_agfl, agfl_crc)))
>> + xfs_buf_ioerror(bp, EFSBADCRC);
>> + else if (!xfs_agfl_verify(bp))
>
> Obviously you added the CRC_OFF directives earlier in the set. It looks
> like this patch squashed a couple of them (XFS_AGF_CRC_OFF as well).
Whoops, no idea how that happened :/ Thanks.
>> xfs_buf_ioerror(bp, EFSCORRUPTED);
>> - }
>> +
>> + if (bp->b_error)
>> + xfs_verifier_error(bp);
>> }
>>
> ...
>> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
>> index 4657586..8aa720d 100644
>> --- a/fs/xfs/xfs_ialloc.c
>> +++ b/fs/xfs/xfs_ialloc.c
>> @@ -1573,13 +1573,17 @@ xfs_agi_read_verify(
>> if (xfs_sb_version_hascrc(&mp->m_sb))
>> agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
>>
>> + if (!agi_ok)
>> + xfs_buf_ioerror(bp, EFSBADCRC);
>> +
>> agi_ok = agi_ok && xfs_agi_verify(bp);
>>
>> if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
>> - XFS_RANDOM_IALLOC_READ_AGI))) {
>> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
>> + XFS_RANDOM_IALLOC_READ_AGI)))
>> xfs_buf_ioerror(bp, EFSCORRUPTED);
>> - }
>> +
>> + if (bp->b_error)
>> + xfs_verifier_error(bp);
>> }
>
> Any reason not to use the same if/else pattern here that the others are
> now using (i.e., similar to xfs_agf_read_verify(), removing the need for
> agi_ok)?
Hm I was thinking it was the weird XFS_TEST_ERROR construction but
xfs_agf_read_verify has that too. I'll take another look, thanks.
(TBH all these verifiers are so similar, I wish there were a way
to not do so much of what is essentially cut and paste with different
error tags & offsets...)
Thanks for the careful review,
-Eric
> Brian
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-19 7:56 ` Jeff Liu
@ 2014-02-20 0:27 ` Dave Chinner
2014-02-20 9:33 ` Jeff Liu
2014-02-20 9:41 ` Jeff Liu
0 siblings, 2 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-20 0:27 UTC (permalink / raw)
To: Jeff Liu; +Cc: Eric Sandeen, xfs
On Wed, Feb 19, 2014 at 03:56:54PM +0800, Jeff Liu wrote:
> Hi Eric,
>
> I read the previous comments from Dave about using defines for CRC offsets,
> and with a grep search after applying this patch, looks there have another
> two places maybe we should switch them to the macros as well:
>
> fs/xfs/xfs_log.c:
> Do we need a log record crc offset macros for offsetof(struct xlog_rec_header, h_crc))?
>
> xfs_dinode.h:
> we added the XFS_DINODE_CRC_OFF, just use it at below routine?
>
> static inline uint xfs_dinode_size(int version)
> {
> if (version == 3)
> return sizeof(struct xfs_dinode);
> return offsetof(struct xfs_dinode, di_crc);
> }
No, that's a different case - it's not being used for determining
the offset of a CRC varaible - it's being used to calculate the size
of the version 2 inode core. Hence it should remain open coded like
because it has a different purpose in life....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 7/9 V2] xfs: add xfs_verifier_error()
2014-02-18 23:52 ` [PATCH 7/9] xfs: add xfs_verifier_error() Eric Sandeen
2014-02-19 6:30 ` Dave Chinner
@ 2014-02-20 2:58 ` Eric Sandeen
2014-02-27 4:20 ` Dave Chinner
1 sibling, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-20 2:58 UTC (permalink / raw)
To: Eric Sandeen, xfs
We want to distinguish between corruption, CRC errors,
etc. In addition, the full stack trace on verifier errors
seems less than helpful; it looks more like an oops than
corruption.
Create a new function to specifically alert the user to
verifier errors, which can differentiate between
EFSCORRUPTED and CRC mismatches. It doesn't dump stack
unless the xfs error level is turned up high.
Define a new error message (EFSBADCRC) to clearly identify
CRC errors. (Defined to EILSEQ, bad byte sequence)
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: Switch to EBADMSG, use xfs_buf_offset()
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 9995b80..a8b2ecb 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -178,3 +178,28 @@ xfs_corruption_error(
xfs_error_report(tag, level, mp, filename, linenum, ra);
xfs_alert(mp, "Corruption detected. Unmount and run xfs_repair");
}
+
+/*
+ * Warnings specifically for verifier errors. Differentiate CRC vs. invalid
+ * values, and omit the stack trace unless the error level is tuned high.
+ */
+void
+xfs_verifier_error(
+ struct xfs_buf *bp)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+
+ xfs_alert(mp, "Metadata %s detected at %pF, block 0x%llx",
+ bp->b_error == EFSBADCRC ? "CRC error" : "corruption",
+ __return_address, bp->b_bn);
+
+ xfs_alert(mp, "Unmount and run xfs_repair");
+
+ if (xfs_error_level >= XFS_ERRLEVEL_LOW) {
+ xfs_alert(mp, "First 64 bytes of corrupted metadata buffer:");
+ xfs_hex_dump(xfs_buf_offset(bp, 0), 64);
+ }
+
+ if (xfs_error_level >= XFS_ERRLEVEL_HIGH)
+ xfs_stack_trace();
+}
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 079a367..c1c57d4 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -34,6 +34,7 @@ extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
extern void xfs_corruption_error(const char *tag, int level,
struct xfs_mount *mp, void *p, const char *filename,
int linenum, inst_t *ra);
+extern void xfs_verifier_error(struct xfs_buf *bp);
#define XFS_ERROR_REPORT(e, lvl, mp) \
xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index e8fed74..825249d 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -179,6 +179,7 @@ typedef __uint64_t __psunsigned_t;
#define ENOATTR ENODATA /* Attribute not found */
#define EWRONGFS EINVAL /* Mount with wrong filesystem type */
#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#define EFSBADCRC EBADMSG /* Bad CRC detected */
#define SYNCHRONIZE() barrier()
#define __return_address __builtin_return_address(0)
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 9/9 V2] xfs: modify verifiers to differentiate CRC from other errors
2014-02-18 23:52 ` [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors Eric Sandeen
2014-02-19 14:01 ` Brian Foster
@ 2014-02-20 3:10 ` Eric Sandeen
2014-02-20 13:10 ` Brian Foster
1 sibling, 1 reply; 28+ messages in thread
From: Eric Sandeen @ 2014-02-20 3:10 UTC (permalink / raw)
To: Eric Sandeen, xfs
Modify all read & write verifiers to differentiate
between CRC errors and other inconsistencies.
This sets the appropriate error number on bp->b_error,
and then calls xfs_verifier_error() if something went
wrong. That function will issue the appropriate message
to the user.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: Fix inexplicably lost XFS_*_CRC_OFF macros, and
rework xfs_agi_read_verify() to be similar to other
verifiers, per Brian's review
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 9c7cf3d..c1cf6a3 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -474,7 +474,6 @@ xfs_agfl_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- int agfl_ok = 1;
/*
* There is no verification of non-crc AGFLs because mkfs does not
@@ -485,14 +484,13 @@ xfs_agfl_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
-
- agfl_ok = agfl_ok && xfs_agfl_verify(bp);
-
- if (!agfl_ok) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_agfl_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -507,8 +505,8 @@ xfs_agfl_write_verify(
return;
if (!xfs_agfl_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -2236,18 +2234,17 @@ xfs_agf_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- int agf_ok = 1;
-
- if (xfs_sb_version_hascrc(&mp->m_sb))
- agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
- agf_ok = agf_ok && xfs_agf_verify(mp, bp);
-
- if (unlikely(XFS_TEST_ERROR(!agf_ok, mp, XFS_ERRTAG_ALLOC_READ_AGF,
- XFS_RANDOM_ALLOC_READ_AGF))) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
+ XFS_ERRTAG_ALLOC_READ_AGF,
+ XFS_RANDOM_ALLOC_READ_AGF))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -2258,8 +2255,8 @@ xfs_agf_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
if (!xfs_agf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index 144d3b0..cc1eadc 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -355,12 +355,14 @@ static void
xfs_allocbt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_sblock_verify_crc(bp) &&
- xfs_allocbt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_allocbt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -370,9 +372,8 @@ xfs_allocbt_write_verify(
{
if (!xfs_allocbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
index b552378..fe9587f 100644
--- a/fs/xfs/xfs_attr_leaf.c
+++ b/fs/xfs/xfs_attr_leaf.c
@@ -213,8 +213,8 @@ xfs_attr3_leaf_write_verify(
struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
if (!xfs_attr3_leaf_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -239,12 +239,14 @@ xfs_attr3_leaf_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
- !xfs_attr3_leaf_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_attr3_leaf_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
diff --git a/fs/xfs/xfs_attr_remote.c b/fs/xfs/xfs_attr_remote.c
index 5549d69..6e37823 100644
--- a/fs/xfs/xfs_attr_remote.c
+++ b/fs/xfs/xfs_attr_remote.c
@@ -125,7 +125,6 @@ xfs_attr3_rmt_read_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
char *ptr;
int len;
- bool corrupt = false;
xfs_daddr_t bno;
/* no verification of non-crc buffers */
@@ -140,11 +139,11 @@ xfs_attr3_rmt_read_verify(
while (len > 0) {
if (!xfs_verify_cksum(ptr, XFS_LBSIZE(mp),
XFS_ATTR3_RMT_CRC_OFF)) {
- corrupt = true;
+ xfs_buf_ioerror(bp, EFSBADCRC);
break;
}
if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
- corrupt = true;
+ xfs_buf_ioerror(bp, EFSCORRUPTED);
break;
}
len -= XFS_LBSIZE(mp);
@@ -152,10 +151,9 @@ xfs_attr3_rmt_read_verify(
bno += mp->m_bsize;
}
- if (corrupt) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
- xfs_buf_ioerror(bp, EFSCORRUPTED);
- } else
+ if (bp->b_error)
+ xfs_verifier_error(bp);
+ else
ASSERT(len == 0);
}
@@ -180,9 +178,8 @@ xfs_attr3_rmt_write_verify(
while (len > 0) {
if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
- XFS_CORRUPTION_ERROR(__func__,
- XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
if (bip) {
diff --git a/fs/xfs/xfs_bmap_btree.c b/fs/xfs/xfs_bmap_btree.c
index 706bc3f..818d546 100644
--- a/fs/xfs/xfs_bmap_btree.c
+++ b/fs/xfs/xfs_bmap_btree.c
@@ -780,12 +780,14 @@ static void
xfs_bmbt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_lblock_verify_crc(bp) &&
- xfs_bmbt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_lblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_bmbt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -794,11 +796,9 @@ xfs_bmbt_write_verify(
struct xfs_buf *bp)
{
if (!xfs_bmbt_verify(bp)) {
- xfs_warn(bp->b_target->bt_mount, "bmbt daddr 0x%llx failed", bp->b_bn);
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_lblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 75ef990..1f5af79 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -185,8 +185,8 @@ xfs_da3_node_write_verify(
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
if (!xfs_da3_node_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
@@ -209,17 +209,20 @@ static void
xfs_da3_node_read_verify(
struct xfs_buf *bp)
{
- struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_da_blkinfo *info = bp->b_addr;
switch (be16_to_cpu(info->magic)) {
case XFS_DA3_NODE_MAGIC:
- if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
+ if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF)) {
+ xfs_buf_ioerror(bp, EFSBADCRC);
break;
+ }
/* fall through */
case XFS_DA_NODE_MAGIC:
- if (!xfs_da3_node_verify(bp))
+ if (!xfs_da3_node_verify(bp)) {
+ xfs_buf_ioerror(bp, EFSCORRUPTED);
break;
+ }
return;
case XFS_ATTR_LEAF_MAGIC:
case XFS_ATTR3_LEAF_MAGIC:
@@ -236,8 +239,7 @@ xfs_da3_node_read_verify(
}
/* corrupt block */
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
- xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
}
const struct xfs_buf_ops xfs_da3_node_buf_ops = {
diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
index 724377e..4f6a38c 100644
--- a/fs/xfs/xfs_dir2_block.c
+++ b/fs/xfs/xfs_dir2_block.c
@@ -89,12 +89,14 @@ xfs_dir3_block_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
- !xfs_dir3_block_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_block_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -106,8 +108,8 @@ xfs_dir3_block_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_block_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
index 74ae85e..afa4ad5 100644
--- a/fs/xfs/xfs_dir2_data.c
+++ b/fs/xfs/xfs_dir2_data.c
@@ -241,7 +241,6 @@ static void
xfs_dir3_data_reada_verify(
struct xfs_buf *bp)
{
- struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_dir2_data_hdr *hdr = bp->b_addr;
switch (hdr->magic) {
@@ -255,8 +254,8 @@ xfs_dir3_data_reada_verify(
xfs_dir3_data_verify(bp);
return;
default:
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, hdr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
break;
}
}
@@ -267,12 +266,14 @@ xfs_dir3_data_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
- !xfs_dir3_data_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_data_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -284,8 +285,8 @@ xfs_dir3_data_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_data_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
index dffb61b..d36e97d 100644
--- a/fs/xfs/xfs_dir2_leaf.c
+++ b/fs/xfs/xfs_dir2_leaf.c
@@ -179,12 +179,14 @@ __read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
- !xfs_dir3_leaf_verify(bp, magic)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_leaf_verify(bp, magic))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -197,8 +199,8 @@ __write_verify(
struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_leaf_verify(bp, magic)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 0904b20..cb434d7 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -115,12 +115,14 @@ xfs_dir3_free_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if ((xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
- !xfs_dir3_free_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dir3_free_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -132,8 +134,8 @@ xfs_dir3_free_write_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (!xfs_dir3_free_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_dquot_buf.c b/fs/xfs/xfs_dquot_buf.c
index d401457..610da81 100644
--- a/fs/xfs/xfs_dquot_buf.c
+++ b/fs/xfs/xfs_dquot_buf.c
@@ -257,10 +257,13 @@ xfs_dquot_buf_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- if (!xfs_dquot_buf_verify_crc(mp, bp) || !xfs_dquot_buf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_dquot_buf_verify_crc(mp, bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_dquot_buf_verify(mp, bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
/*
@@ -275,8 +278,8 @@ xfs_dquot_buf_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
if (!xfs_dquot_buf_verify(mp, bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
}
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 4657586..5959b3b 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1568,18 +1568,17 @@ xfs_agi_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
- int agi_ok = 1;
- if (xfs_sb_version_hascrc(&mp->m_sb))
- agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
-
- agi_ok = agi_ok && xfs_agi_verify(bp);
-
- if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
- XFS_RANDOM_IALLOC_READ_AGI))) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (xfs_sb_version_hascrc(&mp->m_sb) &&
+ !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
+ XFS_ERRTAG_IALLOC_READ_AGI,
+ XFS_RANDOM_IALLOC_READ_AGI))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -1590,8 +1589,8 @@ xfs_agi_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
if (!xfs_agi_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
index 0028c50..7e309b1 100644
--- a/fs/xfs/xfs_ialloc_btree.c
+++ b/fs/xfs/xfs_ialloc_btree.c
@@ -243,12 +243,14 @@ static void
xfs_inobt_read_verify(
struct xfs_buf *bp)
{
- if (!(xfs_btree_sblock_verify_crc(bp) &&
- xfs_inobt_verify(bp))) {
- trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
+ if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_inobt_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+ if (bp->b_error) {
+ trace_xfs_btree_corrupt(bp, _RET_IP_);
+ xfs_verifier_error(bp);
}
}
@@ -258,9 +260,8 @@ xfs_inobt_write_verify(
{
if (!xfs_inobt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- bp->b_target->bt_mount, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
index 606b43a..24e9939 100644
--- a/fs/xfs/xfs_inode_buf.c
+++ b/fs/xfs/xfs_inode_buf.c
@@ -102,8 +102,7 @@ xfs_inode_buf_verify(
}
xfs_buf_ioerror(bp, EFSCORRUPTED);
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_HIGH,
- mp, dip);
+ xfs_verifier_error(bp);
#ifdef DEBUG
xfs_alert(mp,
"bad inode magic/vsn daddr %lld #%d (magic=%x)",
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 818359f..b134aa8 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -614,7 +614,7 @@ xfs_sb_read_verify(
/* Only fail bad secondaries on a known V5 filesystem */
if (bp->b_bn == XFS_SB_DADDR ||
xfs_sb_version_hascrc(&mp->m_sb)) {
- error = EFSCORRUPTED;
+ error = EFSBADCRC;
goto out_error;
}
}
@@ -623,10 +623,9 @@ xfs_sb_read_verify(
out_error:
if (error) {
- if (error == EFSCORRUPTED)
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- mp, bp->b_addr);
xfs_buf_ioerror(bp, error);
+ if (error == EFSCORRUPTED || error == EFSBADCRC)
+ xfs_verifier_error(bp);
}
}
@@ -661,9 +660,8 @@ xfs_sb_write_verify(
error = xfs_sb_verify(bp, false);
if (error) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
- mp, bp->b_addr);
xfs_buf_ioerror(bp, error);
+ xfs_verifier_error(bp);
return;
}
diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
index defa09f..9b32052 100644
--- a/fs/xfs/xfs_symlink_remote.c
+++ b/fs/xfs/xfs_symlink_remote.c
@@ -133,11 +133,13 @@ xfs_symlink_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
- !xfs_symlink_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+ if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
+ xfs_buf_ioerror(bp, EFSBADCRC);
+ else if (!xfs_symlink_verify(bp))
xfs_buf_ioerror(bp, EFSCORRUPTED);
- }
+
+ if (bp->b_error)
+ xfs_verifier_error(bp);
}
static void
@@ -152,8 +154,8 @@ xfs_symlink_write_verify(
return;
if (!xfs_symlink_verify(bp)) {
- XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
xfs_buf_ioerror(bp, EFSCORRUPTED);
+ xfs_verifier_error(bp);
return;
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-20 0:27 ` Dave Chinner
@ 2014-02-20 9:33 ` Jeff Liu
2014-02-20 9:41 ` Jeff Liu
1 sibling, 0 replies; 28+ messages in thread
From: Jeff Liu @ 2014-02-20 9:33 UTC (permalink / raw)
To: Dave Chinner; +Cc: Eric Sandeen, xfs
On 02/20 2014 08:27 AM, Dave Chinner wrote:
> On Wed, Feb 19, 2014 at 03:56:54PM +0800, Jeff Liu wrote:
>> Hi Eric,
>>
>> I read the previous comments from Dave about using defines for CRC offsets,
>> and with a grep search after applying this patch, looks there have another
>> two places maybe we should switch them to the macros as well:
>>
>> fs/xfs/xfs_log.c:
>> Do we need a log record crc offset macros for offsetof(struct xlog_rec_header, h_crc))?
>>
>> xfs_dinode.h:
>> we added the XFS_DINODE_CRC_OFF, just use it at below routine?
>>
>> static inline uint xfs_dinode_size(int version)
>> {
>> if (version == 3)
>> return sizeof(struct xfs_dinode);
>> return offsetof(struct xfs_dinode, di_crc);
>> }
>
> No, that's a different case - it's not being used for determining
> the offset of a CRC varaible - it's being used to calculate the size
> of the version 2 inode core. Hence it should remain open coded like
> because it has a different purpose in life....
Thanks for the clarification, so we don't need that for the second inode
case, but the first case is used to determine the log record crc offset
to generate the crc for record header, shouldn't we make it consistent
with others?
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-20 0:27 ` Dave Chinner
2014-02-20 9:33 ` Jeff Liu
@ 2014-02-20 9:41 ` Jeff Liu
2014-02-27 2:15 ` Dave Chinner
1 sibling, 1 reply; 28+ messages in thread
From: Jeff Liu @ 2014-02-20 9:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: Eric Sandeen, xfs
On 02/20 2014 08:27 AM, Dave Chinner wrote:
> On Wed, Feb 19, 2014 at 03:56:54PM +0800, Jeff Liu wrote:
>> Hi Eric,
>>
>> I read the previous comments from Dave about using defines for CRC offsets,
>> and with a grep search after applying this patch, looks there have another
>> two places maybe we should switch them to the macros as well:
>>
>> fs/xfs/xfs_log.c:
>> Do we need a log record crc offset macros for offsetof(struct xlog_rec_header, h_crc))?
>>
>> xfs_dinode.h:
>> we added the XFS_DINODE_CRC_OFF, just use it at below routine?
>>
>> static inline uint xfs_dinode_size(int version)
>> {
>> if (version == 3)
>> return sizeof(struct xfs_dinode);
>> return offsetof(struct xfs_dinode, di_crc);
>> }
>
> No, that's a different case - it's not being used for determining
> the offset of a CRC varaible - it's being used to calculate the size
> of the version 2 inode core. Hence it should remain open coded like
> because it has a different purpose in life....
Thanks for the clarification, so we don't need that for the second inode
case, but the first case is used to determine the log record crc offset
to generate the crc for record header, shouldn't we make it consistent
with others?
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 9/9 V2] xfs: modify verifiers to differentiate CRC from other errors
2014-02-20 3:10 ` [PATCH 9/9 V2] " Eric Sandeen
@ 2014-02-20 13:10 ` Brian Foster
0 siblings, 0 replies; 28+ messages in thread
From: Brian Foster @ 2014-02-20 13:10 UTC (permalink / raw)
To: Eric Sandeen, Eric Sandeen, xfs
On 02/19/2014 10:10 PM, Eric Sandeen wrote:
> Modify all read & write verifiers to differentiate
> between CRC errors and other inconsistencies.
>
> This sets the appropriate error number on bp->b_error,
> and then calls xfs_verifier_error() if something went
> wrong. That function will issue the appropriate message
> to the user.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
Looks good to me, thanks Eric.
Reviewed-by: Brian Foster <bfoster@redhat.com>
> V2: Fix inexplicably lost XFS_*_CRC_OFF macros, and
> rework xfs_agi_read_verify() to be similar to other
> verifiers, per Brian's review
>
> diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
> index 9c7cf3d..c1cf6a3 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -474,7 +474,6 @@ xfs_agfl_read_verify(
> struct xfs_buf *bp)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
> - int agfl_ok = 1;
>
> /*
> * There is no verification of non-crc AGFLs because mkfs does not
> @@ -485,14 +484,13 @@ xfs_agfl_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
> -
> - agfl_ok = agfl_ok && xfs_agfl_verify(bp);
> -
> - if (!agfl_ok) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_agfl_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -507,8 +505,8 @@ xfs_agfl_write_verify(
> return;
>
> if (!xfs_agfl_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> @@ -2236,18 +2234,17 @@ xfs_agf_read_verify(
> struct xfs_buf *bp)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
> - int agf_ok = 1;
> -
> - if (xfs_sb_version_hascrc(&mp->m_sb))
> - agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
>
> - agf_ok = agf_ok && xfs_agf_verify(mp, bp);
> -
> - if (unlikely(XFS_TEST_ERROR(!agf_ok, mp, XFS_ERRTAG_ALLOC_READ_AGF,
> - XFS_RANDOM_ALLOC_READ_AGF))) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
> + XFS_ERRTAG_ALLOC_READ_AGF,
> + XFS_RANDOM_ALLOC_READ_AGF))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -2258,8 +2255,8 @@ xfs_agf_write_verify(
> struct xfs_buf_log_item *bip = bp->b_fspriv;
>
> if (!xfs_agf_verify(mp, bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
> index 144d3b0..cc1eadc 100644
> --- a/fs/xfs/xfs_alloc_btree.c
> +++ b/fs/xfs/xfs_alloc_btree.c
> @@ -355,12 +355,14 @@ static void
> xfs_allocbt_read_verify(
> struct xfs_buf *bp)
> {
> - if (!(xfs_btree_sblock_verify_crc(bp) &&
> - xfs_allocbt_verify(bp))) {
> - trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> + if (!xfs_btree_sblock_verify_crc(bp))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_allocbt_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> +
> + if (bp->b_error) {
> + trace_xfs_btree_corrupt(bp, _RET_IP_);
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -370,9 +372,8 @@ xfs_allocbt_write_verify(
> {
> if (!xfs_allocbt_verify(bp)) {
> trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> xfs_btree_sblock_calc_crc(bp);
> diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
> index b552378..fe9587f 100644
> --- a/fs/xfs/xfs_attr_leaf.c
> +++ b/fs/xfs/xfs_attr_leaf.c
> @@ -213,8 +213,8 @@ xfs_attr3_leaf_write_verify(
> struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_attr3_leaf_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> @@ -239,12 +239,14 @@ xfs_attr3_leaf_read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if ((xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
> - !xfs_attr3_leaf_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_attr3_leaf_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
> diff --git a/fs/xfs/xfs_attr_remote.c b/fs/xfs/xfs_attr_remote.c
> index 5549d69..6e37823 100644
> --- a/fs/xfs/xfs_attr_remote.c
> +++ b/fs/xfs/xfs_attr_remote.c
> @@ -125,7 +125,6 @@ xfs_attr3_rmt_read_verify(
> struct xfs_mount *mp = bp->b_target->bt_mount;
> char *ptr;
> int len;
> - bool corrupt = false;
> xfs_daddr_t bno;
>
> /* no verification of non-crc buffers */
> @@ -140,11 +139,11 @@ xfs_attr3_rmt_read_verify(
> while (len > 0) {
> if (!xfs_verify_cksum(ptr, XFS_LBSIZE(mp),
> XFS_ATTR3_RMT_CRC_OFF)) {
> - corrupt = true;
> + xfs_buf_ioerror(bp, EFSBADCRC);
> break;
> }
> if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
> - corrupt = true;
> + xfs_buf_ioerror(bp, EFSCORRUPTED);
> break;
> }
> len -= XFS_LBSIZE(mp);
> @@ -152,10 +151,9 @@ xfs_attr3_rmt_read_verify(
> bno += mp->m_bsize;
> }
>
> - if (corrupt) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> - xfs_buf_ioerror(bp, EFSCORRUPTED);
> - } else
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> + else
> ASSERT(len == 0);
> }
>
> @@ -180,9 +178,8 @@ xfs_attr3_rmt_write_verify(
>
> while (len > 0) {
> if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
> - XFS_CORRUPTION_ERROR(__func__,
> - XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> if (bip) {
> diff --git a/fs/xfs/xfs_bmap_btree.c b/fs/xfs/xfs_bmap_btree.c
> index 706bc3f..818d546 100644
> --- a/fs/xfs/xfs_bmap_btree.c
> +++ b/fs/xfs/xfs_bmap_btree.c
> @@ -780,12 +780,14 @@ static void
> xfs_bmbt_read_verify(
> struct xfs_buf *bp)
> {
> - if (!(xfs_btree_lblock_verify_crc(bp) &&
> - xfs_bmbt_verify(bp))) {
> - trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> + if (!xfs_btree_lblock_verify_crc(bp))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_bmbt_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> +
> + if (bp->b_error) {
> + trace_xfs_btree_corrupt(bp, _RET_IP_);
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -794,11 +796,9 @@ xfs_bmbt_write_verify(
> struct xfs_buf *bp)
> {
> if (!xfs_bmbt_verify(bp)) {
> - xfs_warn(bp->b_target->bt_mount, "bmbt daddr 0x%llx failed", bp->b_bn);
> trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> xfs_btree_lblock_calc_crc(bp);
> diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
> index 75ef990..1f5af79 100644
> --- a/fs/xfs/xfs_da_btree.c
> +++ b/fs/xfs/xfs_da_btree.c
> @@ -185,8 +185,8 @@ xfs_da3_node_write_verify(
> struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_da3_node_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> @@ -209,17 +209,20 @@ static void
> xfs_da3_node_read_verify(
> struct xfs_buf *bp)
> {
> - struct xfs_mount *mp = bp->b_target->bt_mount;
> struct xfs_da_blkinfo *info = bp->b_addr;
>
> switch (be16_to_cpu(info->magic)) {
> case XFS_DA3_NODE_MAGIC:
> - if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
> + if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF)) {
> + xfs_buf_ioerror(bp, EFSBADCRC);
> break;
> + }
> /* fall through */
> case XFS_DA_NODE_MAGIC:
> - if (!xfs_da3_node_verify(bp))
> + if (!xfs_da3_node_verify(bp)) {
> + xfs_buf_ioerror(bp, EFSCORRUPTED);
> break;
> + }
> return;
> case XFS_ATTR_LEAF_MAGIC:
> case XFS_ATTR3_LEAF_MAGIC:
> @@ -236,8 +239,7 @@ xfs_da3_node_read_verify(
> }
>
> /* corrupt block */
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> - xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> }
>
> const struct xfs_buf_ops xfs_da3_node_buf_ops = {
> diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
> index 724377e..4f6a38c 100644
> --- a/fs/xfs/xfs_dir2_block.c
> +++ b/fs/xfs/xfs_dir2_block.c
> @@ -89,12 +89,14 @@ xfs_dir3_block_read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if ((xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
> - !xfs_dir3_block_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_dir3_block_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -106,8 +108,8 @@ xfs_dir3_block_write_verify(
> struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_dir3_block_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
> index 74ae85e..afa4ad5 100644
> --- a/fs/xfs/xfs_dir2_data.c
> +++ b/fs/xfs/xfs_dir2_data.c
> @@ -241,7 +241,6 @@ static void
> xfs_dir3_data_reada_verify(
> struct xfs_buf *bp)
> {
> - struct xfs_mount *mp = bp->b_target->bt_mount;
> struct xfs_dir2_data_hdr *hdr = bp->b_addr;
>
> switch (hdr->magic) {
> @@ -255,8 +254,8 @@ xfs_dir3_data_reada_verify(
> xfs_dir3_data_verify(bp);
> return;
> default:
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, hdr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> break;
> }
> }
> @@ -267,12 +266,14 @@ xfs_dir3_data_read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if ((xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
> - !xfs_dir3_data_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_dir3_data_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -284,8 +285,8 @@ xfs_dir3_data_write_verify(
> struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_dir3_data_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
> index dffb61b..d36e97d 100644
> --- a/fs/xfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/xfs_dir2_leaf.c
> @@ -179,12 +179,14 @@ __read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if ((xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
> - !xfs_dir3_leaf_verify(bp, magic)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_dir3_leaf_verify(bp, magic))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -197,8 +199,8 @@ __write_verify(
> struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_dir3_leaf_verify(bp, magic)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
> index 0904b20..cb434d7 100644
> --- a/fs/xfs/xfs_dir2_node.c
> +++ b/fs/xfs/xfs_dir2_node.c
> @@ -115,12 +115,14 @@ xfs_dir3_free_read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if ((xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
> - !xfs_dir3_free_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_dir3_free_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -132,8 +134,8 @@ xfs_dir3_free_write_verify(
> struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
>
> if (!xfs_dir3_free_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_dquot_buf.c b/fs/xfs/xfs_dquot_buf.c
> index d401457..610da81 100644
> --- a/fs/xfs/xfs_dquot_buf.c
> +++ b/fs/xfs/xfs_dquot_buf.c
> @@ -257,10 +257,13 @@ xfs_dquot_buf_read_verify(
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> - if (!xfs_dquot_buf_verify_crc(mp, bp) || !xfs_dquot_buf_verify(mp, bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (!xfs_dquot_buf_verify_crc(mp, bp))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_dquot_buf_verify(mp, bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> /*
> @@ -275,8 +278,8 @@ xfs_dquot_buf_write_verify(
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> if (!xfs_dquot_buf_verify(mp, bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> }
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 4657586..5959b3b 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -1568,18 +1568,17 @@ xfs_agi_read_verify(
> struct xfs_buf *bp)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
> - int agi_ok = 1;
>
> - if (xfs_sb_version_hascrc(&mp->m_sb))
> - agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
> -
> - agi_ok = agi_ok && xfs_agi_verify(bp);
> -
> - if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
> - XFS_RANDOM_IALLOC_READ_AGI))) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (xfs_sb_version_hascrc(&mp->m_sb) &&
> + !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
> + XFS_ERRTAG_IALLOC_READ_AGI,
> + XFS_RANDOM_IALLOC_READ_AGI))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -1590,8 +1589,8 @@ xfs_agi_write_verify(
> struct xfs_buf_log_item *bip = bp->b_fspriv;
>
> if (!xfs_agi_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
> index 0028c50..7e309b1 100644
> --- a/fs/xfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/xfs_ialloc_btree.c
> @@ -243,12 +243,14 @@ static void
> xfs_inobt_read_verify(
> struct xfs_buf *bp)
> {
> - if (!(xfs_btree_sblock_verify_crc(bp) &&
> - xfs_inobt_verify(bp))) {
> - trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> + if (!xfs_btree_sblock_verify_crc(bp))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_inobt_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> +
> + if (bp->b_error) {
> + trace_xfs_btree_corrupt(bp, _RET_IP_);
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -258,9 +260,8 @@ xfs_inobt_write_verify(
> {
> if (!xfs_inobt_verify(bp)) {
> trace_xfs_btree_corrupt(bp, _RET_IP_);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - bp->b_target->bt_mount, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
> xfs_btree_sblock_calc_crc(bp);
> diff --git a/fs/xfs/xfs_inode_buf.c b/fs/xfs/xfs_inode_buf.c
> index 606b43a..24e9939 100644
> --- a/fs/xfs/xfs_inode_buf.c
> +++ b/fs/xfs/xfs_inode_buf.c
> @@ -102,8 +102,7 @@ xfs_inode_buf_verify(
> }
>
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_HIGH,
> - mp, dip);
> + xfs_verifier_error(bp);
> #ifdef DEBUG
> xfs_alert(mp,
> "bad inode magic/vsn daddr %lld #%d (magic=%x)",
> diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
> index 818359f..b134aa8 100644
> --- a/fs/xfs/xfs_sb.c
> +++ b/fs/xfs/xfs_sb.c
> @@ -614,7 +614,7 @@ xfs_sb_read_verify(
> /* Only fail bad secondaries on a known V5 filesystem */
> if (bp->b_bn == XFS_SB_DADDR ||
> xfs_sb_version_hascrc(&mp->m_sb)) {
> - error = EFSCORRUPTED;
> + error = EFSBADCRC;
> goto out_error;
> }
> }
> @@ -623,10 +623,9 @@ xfs_sb_read_verify(
>
> out_error:
> if (error) {
> - if (error == EFSCORRUPTED)
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - mp, bp->b_addr);
> xfs_buf_ioerror(bp, error);
> + if (error == EFSCORRUPTED || error == EFSBADCRC)
> + xfs_verifier_error(bp);
> }
> }
>
> @@ -661,9 +660,8 @@ xfs_sb_write_verify(
>
> error = xfs_sb_verify(bp, false);
> if (error) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
> - mp, bp->b_addr);
> xfs_buf_ioerror(bp, error);
> + xfs_verifier_error(bp);
> return;
> }
>
> diff --git a/fs/xfs/xfs_symlink_remote.c b/fs/xfs/xfs_symlink_remote.c
> index defa09f..9b32052 100644
> --- a/fs/xfs/xfs_symlink_remote.c
> +++ b/fs/xfs/xfs_symlink_remote.c
> @@ -133,11 +133,13 @@ xfs_symlink_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
> - !xfs_symlink_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> + if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
> + xfs_buf_ioerror(bp, EFSBADCRC);
> + else if (!xfs_symlink_verify(bp))
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> - }
> +
> + if (bp->b_error)
> + xfs_verifier_error(bp);
> }
>
> static void
> @@ -152,8 +154,8 @@ xfs_symlink_write_verify(
> return;
>
> if (!xfs_symlink_verify(bp)) {
> - XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
> xfs_buf_ioerror(bp, EFSCORRUPTED);
> + xfs_verifier_error(bp);
> return;
> }
>
>
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/9] xfs: Use defines for CRC offsets in all cases
2014-02-20 9:41 ` Jeff Liu
@ 2014-02-27 2:15 ` Dave Chinner
0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-27 2:15 UTC (permalink / raw)
To: Jeff Liu; +Cc: Eric Sandeen, xfs
On Thu, Feb 20, 2014 at 05:41:02PM +0800, Jeff Liu wrote:
>
> On 02/20 2014 08:27 AM, Dave Chinner wrote:
> > On Wed, Feb 19, 2014 at 03:56:54PM +0800, Jeff Liu wrote:
> >> Hi Eric,
> >>
> >> I read the previous comments from Dave about using defines for CRC offsets,
> >> and with a grep search after applying this patch, looks there have another
> >> two places maybe we should switch them to the macros as well:
> >>
> >> fs/xfs/xfs_log.c:
> >> Do we need a log record crc offset macros for offsetof(struct xlog_rec_header, h_crc))?
> >>
> >> xfs_dinode.h:
> >> we added the XFS_DINODE_CRC_OFF, just use it at below routine?
> >>
> >> static inline uint xfs_dinode_size(int version)
> >> {
> >> if (version == 3)
> >> return sizeof(struct xfs_dinode);
> >> return offsetof(struct xfs_dinode, di_crc);
> >> }
> >
> > No, that's a different case - it's not being used for determining
> > the offset of a CRC varaible - it's being used to calculate the size
> > of the version 2 inode core. Hence it should remain open coded like
> > because it has a different purpose in life....
>
> Thanks for the clarification, so we don't need that for the second inode
> case, but the first case is used to determine the log record crc offset
> to generate the crc for record header, shouldn't we make it consistent
> with others?
Probably should. Eric, can you send a separate patch that converts
the log crc fileds to use the same convention?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs
2014-02-18 23:52 ` [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs Eric Sandeen
@ 2014-02-27 4:17 ` Dave Chinner
0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-27 4:17 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Tue, Feb 18, 2014 at 05:52:25PM -0600, Eric Sandeen wrote:
> Many/most callers of xfs_verify_cksum() pass bp->b_addr and
> BBTOB(bp->b_length) as the first 2 args. Add a helper
> which can just accept the bp and the crc offset, and work
> it out on its own, for brevity.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 7/9 V2] xfs: add xfs_verifier_error()
2014-02-20 2:58 ` [PATCH 7/9 V2] " Eric Sandeen
@ 2014-02-27 4:20 ` Dave Chinner
0 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-27 4:20 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs
On Wed, Feb 19, 2014 at 08:58:11PM -0600, Eric Sandeen wrote:
> We want to distinguish between corruption, CRC errors,
> etc. In addition, the full stack trace on verifier errors
> seems less than helpful; it looks more like an oops than
> corruption.
>
> Create a new function to specifically alert the user to
> verifier errors, which can differentiate between
> EFSCORRUPTED and CRC mismatches. It doesn't dump stack
> unless the xfs error level is turned up high.
>
> Define a new error message (EFSBADCRC) to clearly identify
> CRC errors. (Defined to EILSEQ, bad byte sequence)
I've updated this to be EBADMSG, to match the code.
Otherwise, looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] current series for verifier error differentiation
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
` (8 preceding siblings ...)
2014-02-18 23:52 ` [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors Eric Sandeen
@ 2014-02-27 9:12 ` Dave Chinner
9 siblings, 0 replies; 28+ messages in thread
From: Dave Chinner @ 2014-02-27 9:12 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Tue, Feb 18, 2014 at 05:52:20PM -0600, Eric Sandeen wrote:
> This is the current series I have leading up to verifier error differentiation,
> just resending them all.
>
> As I send this, I remember that EILSEQ is giving us an unusual perror()
> output, so the last patch which uses it may need to pick some (?)
> other error code...
Eric, this leaks EBADMSG to userspace in xfs/005 with a corrupted
superblock:
$ diff -u tests/xfs/005.out /home/dave/src/xfstests-dev/results//xfs/005.out.bad
--- tests/xfs/005.out 2014-02-24 09:58:09.000000000 +1100
+++ /home/dave/src/xfstests-dev/results//xfs/005.out.bad
2014-02-27 19:40:56.000000000 +1100
@@ -1,4 +1,4 @@
QA output created by 005
wrote 4/4 bytes at offset 224
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-mount: Structure needs cleaning
+mount: Bad message
Dmesg contains this:
[ 8790.946956] XFS (vdb): Metadata CRC error detected at xfs_sb_read_verify+0x10c/0x140, block 0x0
[ 8790.949984] XFS (vdb): Unmount and run xfs_repair
[ 8790.951725] XFS (vdb): First 64 bytes of corrupted metadata buffer:
[ 8790.953883] ffff880003eea000: 58 46 53 42 00 00 10 00 00 00 00 00 00 2c 00 00 XFSB.........,..
[ 8790.956452] ffff880003eea010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
[ 8790.958518] ffff880003eea020: 0b 21 13 2a 5b 00 40 59 bd 7c 2b d4 5b 6c a4 0a .!.*[.@Y.|+.[l..
[ 8790.960487] ffff880003eea030: 00 00 00 00 00 20 00 04 00 00 00 00 00 00 00 40 ..... .........@
So that part is good. However, I suspect we should still be
returning EFSCORRUPTED to userspace. Most metadata buffer reads go
through xfs_trans_read_buf_map(), so there only a couple of places
we need to catch and convert the error being returned. The other two
I notice that are need conversion are xfs_buf_read() in
xfs_readlink_bmap(), and the cause of this regression which is
xfs_buf_read_uncached() in xfs_readsb()....
Can you look into writing up a followup patch that does this
catch-and-convert? It will also mean all the higher layers will
treat a CRC error as corrupt metadata, which is exactly what we want
it to do. :)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2014-02-27 9:12 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-18 23:52 [PATCH 0/9] current series for verifier error differentiation Eric Sandeen
2014-02-18 23:52 ` [PATCH 1/9] xfs: skip verification on initial "guess" superblock read Eric Sandeen
2014-02-19 3:36 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 2/9] xfs: limit superblock corruption errors to actual corruption Eric Sandeen
2014-02-19 3:37 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 3/9] xfs: skip pointless CRC updates after verifier failures Eric Sandeen
2014-02-19 6:35 ` Jeff Liu
2014-02-18 23:52 ` [PATCH 4/9] xfs: Use defines for CRC offsets in all cases Eric Sandeen
2014-02-19 7:56 ` Jeff Liu
2014-02-20 0:27 ` Dave Chinner
2014-02-20 9:33 ` Jeff Liu
2014-02-20 9:41 ` Jeff Liu
2014-02-27 2:15 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 5/9] xfs: add helper for verifying checksums on xfs_bufs Eric Sandeen
2014-02-27 4:17 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 6/9] xfs: add helper for updating " Eric Sandeen
2014-02-18 23:52 ` [PATCH 7/9] xfs: add xfs_verifier_error() Eric Sandeen
2014-02-19 6:30 ` Dave Chinner
2014-02-20 2:58 ` [PATCH 7/9 V2] " Eric Sandeen
2014-02-27 4:20 ` Dave Chinner
2014-02-18 23:52 ` [PATCH 8/9] xfs: print useful caller information in xfs_error_report Eric Sandeen
2014-02-19 12:42 ` Jeff Liu
2014-02-18 23:52 ` [PATCH 9/9] xfs: modify verifiers to differentiate CRC from other errors Eric Sandeen
2014-02-19 14:01 ` Brian Foster
2014-02-19 16:12 ` Eric Sandeen
2014-02-20 3:10 ` [PATCH 9/9 V2] " Eric Sandeen
2014-02-20 13:10 ` Brian Foster
2014-02-27 9:12 ` [PATCH 0/9] current series for verifier error differentiation Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.