[Top] [All Lists]

Re: [PATCH 5/9] repair: detect CRC errors in AG headers

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/9] repair: detect CRC errors in AG headers
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 22 Apr 2014 19:10:43 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140422064737.GA32026@xxxxxxxxxxxxx>
References: <1397550301-31883-1-git-send-email-david@xxxxxxxxxxxxx> <1397550301-31883-6-git-send-email-david@xxxxxxxxxxxxx> <20140421071106.GF20384@xxxxxxxxxxxxx> <20140421233512.GE18672@dastard> <20140422064737.GA32026@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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...


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>