All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 3/9] repair: ensure prefetched buffers have CRCs validated
Date: Thu, 24 Apr 2014 22:47:05 -0700	[thread overview]
Message-ID: <20140425054705.GA30118@infradead.org> (raw)
In-Reply-To: <1398315722-20870-4-git-send-email-david@fromorbit.com>

On Thu, Apr 24, 2014 at 03:01:56PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Prefetch currently does not do CRC validation when the IO completes
> due to the optimisation it performs and the fact that it does not
> know what the type of metadata into the buffer is supposed to be.
> Hence, mark all prefetched buffers as "suspect" so that when the
> end user tries to read it with a supplied validation function the
> validation is run even though the buffer was already in the cache.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks good, but a few minor nitpicks below:

> +		if (ops && (bp->b_flags & LIBXFS_B_UNCHECKED)) {
> +			bp->b_error = 0;
> +			bp->b_ops = ops;
> +			bp->b_ops->verify_read(bp);
> +			bp->b_flags &= ~LIBXFS_B_UNCHECKED;
> +		}

There's three copies of code in the previous and this patch, it probably
should go into a helper function.

> +	else if (bp->b_ops) {
>  		bp->b_ops->verify_read(bp);
> +		bp->b_flags &= ~LIBXFS_B_UNCHECKED;
> +	}

Same with this.

>  		bp->b_flags |= LIBXFS_B_UPTODATE;
>  		bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT);
> +		bp->b_flags &= ~LIBXFS_B_UNCHECKED;

Any reason not to clear all three flags in a single line?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-04-25  5:47 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-24  5:01 [PATCH 0/9 V2] xfs_db, xfs_repair: improve CRC error detection Dave Chinner
2014-04-24  5:01 ` [PATCH 1/9] db: don't claim unchecked CRCs are correct Dave Chinner
2014-04-24  5:01 ` [PATCH 2/9] db: verify buffer on type change Dave Chinner
2014-04-25  5:41   ` Christoph Hellwig
2014-04-24  5:01 ` [PATCH 3/9] repair: ensure prefetched buffers have CRCs validated Dave Chinner
2014-04-25  5:47   ` Christoph Hellwig [this message]
2014-04-24  5:01 ` [PATCH 4/9] repair: detect and correct CRC errors in directory blocks Dave Chinner
2014-04-25  5:47   ` Christoph Hellwig
2014-04-24  5:01 ` [PATCH 5/9] repair: detect CRC errors in AG headers Dave Chinner
2014-04-25  5:55   ` Christoph Hellwig
2014-04-24  5:01 ` [PATCH 6/9] repair: report AG btree verifier errors Dave Chinner
2014-04-25  5:55   ` Christoph Hellwig
2014-04-24  5:02 ` [PATCH 7/9] repair: remove more dirv1 leftovers Dave Chinner
2014-04-24  5:02 ` [PATCH 8/9] repair: handle remote symlink CRC errors Dave Chinner
2014-04-25  6:01   ` Christoph Hellwig
2014-04-24  5:02 ` [PATCH 9/9] repair: detect and handle attribute tree " Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2014-04-28 21:04 [PATCH 0/9 v3] xfs_db, xfs_repair: improve CRC error detection Dave Chinner
2014-04-28 21:04 ` [PATCH 3/9] repair: ensure prefetched buffers have CRCs validated Dave Chinner
2014-04-29 14:05   ` Brian Foster
2014-04-29 18:15   ` Christoph Hellwig
2014-04-15  8:24 [PATCH 0/9] xfs_db, xfs_repair: improve CRC error detection Dave Chinner
2014-04-15  8:24 ` [PATCH 3/9] repair: ensure prefetched buffers have CRCs validated Dave Chinner
2014-04-15 19:40   ` Brian Foster
2014-04-15 21:46     ` Dave Chinner
2014-04-15 22:06       ` Brian Foster
2014-04-16  0:41         ` Dave Chinner

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=20140425054705.GA30118@infradead.org \
    --to=hch@infradead.org \
    --cc=david@fromorbit.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.