[Top] [All Lists]

Re: [PATCH 04/11] xfs: remote attribute tail zeroing does too much

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH 04/11] xfs: remote attribute tail zeroing does too much
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 22 May 2013 08:55:16 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130521223109.GN20028@xxxxxxx>
References: <1369123330-9579-1-git-send-email-david@xxxxxxxxxxxxx> <1369123330-9579-5-git-send-email-david@xxxxxxxxxxxxx> <20130521223109.GN20028@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, May 21, 2013 at 05:31:09PM -0500, Ben Myers wrote:
> On Tue, May 21, 2013 at 06:02:03PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > When an attribute data does not fill then entire remote block, we
> > zero the remaining part of the buffer. This, however, needs to take
> > into account that the buffer has a header, and so the offset where
> > zeroing starts and the length of zeroing need to take this into
> > account. Otherwise we end up with zeros over the end of the
> > attribute value when CRCs are enabled.
> > 
> > While there, make sure we only ask to map an extent that covers the
> > remaining range of the attribute, rather than asking every time for
> > the full length of remote data. If the remote attribute blocks are
> > contiguous with other parts of the attribute tree, it will map those
> > blocks as well and we can potentially zero them incorrectly. We can
> > also get buffer size mistmatches when trying to read or remove the
> > remote attribute, and this can lead to not finding the correct
> > buffer when looking it up in cache.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> You fixed the bug I mentioned in the last review.  Beaten.
> This looks ok.  From the looks of things we might consider cleanup of who is
> setting rmtblkcnt in the future.  It is getting to be confusing.

Yes, it is. This exact problem is one of the things my rework of the
attr CRC code later in the series fixes.


Dave Chinner

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