linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 5/9] repair: detect CRC errors in AG headers
Date: Tue, 22 Apr 2014 19:10:43 +1000	[thread overview]
Message-ID: <20140422091043.GF15995@dastard> (raw)
In-Reply-To: <20140422064737.GA32026@infradead.org>

On Mon, Apr 21, 2014 at 11:47:37PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 22, 2014 at 09:35:12AM +1000, Dave Chinner wrote:
> > > Shouldn't we apply the same scheme as for directories here, that is if
> > > it fails with a verifier error re-read without the verifier and then
> > > still do the full check as well?
> > 
> > The directory code is the special case - it uses xfs_trans_read_buf*
> > interfaces, which return either a good buffer with no error or an
> > error with no buffer. Hence for the directory code, we have to
> > re-read the buffer without the verifier to grab the unchecked buffer
> > from the cache when the verifier detects an error.
> 
> How about just having a variant of xfs_da_read_buf that doesn't use
> xfs_trans_read_buf *?  xfs_da_read_buf is pretty simple, especially
> when removing the magic test that has been superceeded by the verifiers.

Right now I ijust want to keep the change as small as possible and
get a release out.

Yes, I agree there's much more cleanup that is needed in this code,
but at this point is seems unnecessary and doesn't matter for the
purpose of providing this functionality initially.  We have to
provide the same functionality for the kernel code for it to be able
to handle CRC errors sanely, and so we're going to need to
restructure xfs_trans_read_buf() to handle it. in doing this, we
solve the libxfs problem, too, because what we do to the kernel code
will flow on to userspace...

So really, I'd prefer to leave the userspace code doing this retry
for this one piece of infrastructure and do all the major
restructing of the directory buffer read code in the kernel code
first. That way we can pick up all the changes for userspace when
the libxfs code is updated.

Keep in mind that right now we've got a *lot* of updates from the
kernel to the libxfs code pending that are stalled waiting for a
release to be done...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2014-04-22  9:10 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15  8:24 [PATCH 0/9] xfs_db, xfs_repair: improve CRC error detection Dave Chinner
2014-04-15  8:24 ` [PATCH 1/9] db: don't claim unchecked CRCs are correct Dave Chinner
2014-04-21  7:00   ` Christoph Hellwig
2014-04-21 23:13     ` Dave Chinner
2014-04-15  8:24 ` [PATCH 2/9] db: verify buffer on type change Dave Chinner
2014-04-21  7:02   ` Christoph Hellwig
2014-04-21 23:14     ` 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
2014-04-15  8:24 ` [PATCH 4/9] repair: detect and correct CRC errors in directory blocks Dave Chinner
2014-04-21  7:08   ` Christoph Hellwig
2014-04-15  8:24 ` [PATCH 5/9] repair: detect CRC errors in AG headers Dave Chinner
2014-04-15 19:40   ` Brian Foster
2014-04-15 21:52     ` Dave Chinner
2014-04-21  7:11   ` Christoph Hellwig
2014-04-21 23:35     ` Dave Chinner
2014-04-22  6:47       ` Christoph Hellwig
2014-04-22  9:10         ` Dave Chinner [this message]
2014-04-22  9:41           ` Christoph Hellwig
2014-04-15  8:24 ` [PATCH 6/9] repair: report AG btree verifier errors Dave Chinner
2014-04-15 19:40   ` Brian Foster
2014-04-15 21:53     ` Dave Chinner
2014-04-15  8:24 ` [PATCH 7/9] repair: remove more dirv1 leftovers Dave Chinner
2014-04-16 13:23   ` Brian Foster
2014-04-21  7:14     ` Christoph Hellwig
2014-04-21  7:13   ` Christoph Hellwig
2014-04-15  8:25 ` [PATCH 8/9] repair: handle remote sylmlink CRC errors Dave Chinner
2014-04-16 13:23   ` Brian Foster
2014-04-15  8:25 ` [PATCH 9/9] repair: detect and handle attribute tree " Dave Chinner
2014-04-16 13:25   ` Brian Foster
2014-04-21 23:27     ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
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 5/9] repair: detect CRC errors in AG headers Dave Chinner
2014-04-25  5:55   ` Christoph Hellwig
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 5/9] repair: detect CRC errors in AG headers Dave Chinner
2014-04-29 14:06   ` Brian Foster
2014-05-01 23:27     ` Dave Chinner
2014-04-29 18:16   ` Christoph Hellwig

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=20140422091043.GF15995@dastard \
    --to=david@fromorbit.com \
    --cc=hch@infradead.org \
    --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;
as well as URLs for NNTP newsgroup(s).