All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 0/9] current series for verifier error differentiation
Date: Thu, 27 Feb 2014 20:12:11 +1100	[thread overview]
Message-ID: <20140227091211.GZ13647@dastard> (raw)
In-Reply-To: <1392767549-25574-1-git-send-email-sandeen@redhat.com>

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

      parent reply	other threads:[~2014-02-27  9:12 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Dave Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140227091211.GZ13647@dastard \
    --to=david@fromorbit.com \
    --cc=sandeen@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.