On Fri, Jun 22, 2018 at 05:26:02PM +0200, Hans van Kranenburg wrote: > On 06/22/2018 01:48 PM, Nikolay Borisov wrote: > > > > > > On 22.06.2018 04:52, Su Yue wrote: > >> For easier debug, print eb->start if level is invalid. > >> Also make print clear if bytenr found is not expected. > >> > >> Signed-off-by: Su Yue <suy.f...@cn.fujitsu.com> > >> --- > >> fs/btrfs/disk-io.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > >> index c3504b4d281b..a90dab84f41b 100644 > >> --- a/fs/btrfs/disk-io.c > >> +++ b/fs/btrfs/disk-io.c > >> @@ -615,8 +615,8 @@ static int btree_readpage_end_io_hook(struct > >> btrfs_io_bio *io_bio, > >> > >> found_start = btrfs_header_bytenr(eb); > >> if (found_start != eb->start) { > >> - btrfs_err_rl(fs_info, "bad tree block start %llu %llu", > >> - found_start, eb->start); > >> + btrfs_err_rl(fs_info, "bad tree block start want %llu have > >> %llu", > > > > nit: I'd rather have the want/have in brackets (want %llu have% llu) > > From a user support point of view, this text should really be improved. > There are a few places where 'want' and 'have' are reported in error > strings, and it's totally unclear what they mean. > > Intuitively I'd say when checking a csum, the "want" would be what's on > disk now, since you want that to be correct, and the "have" would be > what you have calculated, but it's actually the other way round, or > wasn't it? Or was it? > > Every time someone pastes such a message when we help on IRC for > example, there's confusion, and I have to look up the source again, > because I always forget. > > What about (%llu stored on disk, %llu calculated now) or something similar?
Yes, definitely this. I experience the same confusion as Hans, and I think a lot of other people do, too. I usually read "want" and "have" the wrong way round, so more clarity would be really helpful. Hugo. > >> + eb->start, found_start); > >> ret = -EIO; > >> goto err; > >> } > >> @@ -628,8 +628,8 @@ static int btree_readpage_end_io_hook(struct > >> btrfs_io_bio *io_bio, > >> } > >> found_level = btrfs_header_level(eb); > >> if (found_level >= BTRFS_MAX_LEVEL) { > >> - btrfs_err(fs_info, "bad tree block level %d", > >> - (int)btrfs_header_level(eb)); > >> + btrfs_err(fs_info, "bad tree block level %d on %llu", > >> + (int)btrfs_header_level(eb), eb->start); > >> ret = -EIO; > >> goto err; > >> } > >> > -- Hugo Mills | "There's a Martian war machine outside -- they want hugo@... carfax.org.uk | to talk to you about a cure for the common cold." http://carfax.org.uk/ | PGP: E2AB1DE4 | Stephen Franklin, Babylon 5
signature.asc
Description: Digital signature