public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH V5] xfs_db: add crc manipulation commands
Date: Mon, 22 Aug 2016 10:54:41 +1000	[thread overview]
Message-ID: <20160822005441.GO19025@dastard> (raw)
In-Reply-To: <0bb88897-3dfc-de4e-05fb-b737221f0d80@sandeen.net>

On Sun, Aug 14, 2016 at 07:30:51PM -0500, Eric Sandeen wrote:
> This adds a new "crc" command to xfs_db for CRC-enabled filesystems.
> 
> If a structure has a CRC field, we can validate it, invalidate/corrupt
> it, or revalidate/rewrite it:
> 
> xfs_db> sb 0
> xfs_db> crc -v
> crc = 0x796c814f (correct)
> xfs_db> crc -i
> Metadata CRC error detected at block 0x0/0x200
> crc = 0x796c8150 (bad)
> xfs_db> crc -r
> crc = 0x796c814f (correct)
> 
> (-i and -r require "expert" write-capable mode)
> 
> This requires temporarily replacing the write verifier with
> a dummy which won't recalculate the CRC on the way to disk.
> 
> It also required me to write a new flist function, which is
> totally foreign to me, so hopefully done right - but it seems
> to work here.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Actually, this need more help....

> @@ -473,12 +474,15 @@ xfs_verify_recalc_crc(
>  void
>  write_cur(void)
>  {
> +	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
> +

bool, and we can't use the iocur until we've checked if it's valid.
i.e. after this first check:

>  	if (iocur_sp < 0) {
>  		dbprintf(_("nothing to write\n"));
>  		return;
>  	}
>  
> -	if (xfs_sb_version_hascrc(&mp->m_sb) && iocur_top->ino_buf) {
> +	if (xfs_sb_version_hascrc(&mp->m_sb) && iocur_top->ino_buf &&
> +	    !skip_crc) {
>  		libxfs_dinode_calc_crc(mp, iocur_top->data);
>  		iocur_top->ino_crc_ok = 1;
>  	}

The line of code below this calculates CRCs for dquot buffers, so it
seems kind of broken to be skipping crc update for an inode but not
a dquot given they are updated in the same way....

Shouldn't this "skip_crc" condition also be applied to dquot
buffers, too?

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:[~2016-08-22  0:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-12 23:30 [PATCH V4] xfs_db: add crc manipulation commands Eric Sandeen
2016-08-14  3:15 ` Zorro Lang
2016-08-14 15:40   ` Eric Sandeen
2016-08-15  0:30 ` [PATCH V5] " Eric Sandeen
2016-08-22  0:38   ` Dave Chinner
2016-08-22  0:45     ` Eric Sandeen
2016-08-22  0:54   ` 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=20160822005441.GO19025@dastard \
    --to=david@fromorbit.com \
    --cc=sandeen@sandeen.net \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox