[PATCH 04/11] libxfs: clear buffer state flags in libxfs_getbuf and variants

[Darrick J. Wong]

On Wed, Aug 26, 2015 at 11:02:32AM +1000, Dave Chinner wrote:
> On Tue, Aug 25, 2015 at 05:32:46PM -0700, Darrick J. Wong wrote:
> > When we're running xfs_repair with prefetch enabled, it's possible
> > that repair will decide to clear an inode without examining all
> > metadata blocks owned by that inode.  This leaves the unreferenced
> > prefetched buffers marked UNCHECKED, which will cause a subsequent CRC
> > error if the block is reallocated to a different structure and read
> > more than once.  Typically this happens when a large directory is
> > corrupted and lost+found has to grow to accomodate all the
> > disconnected inodes.
> > 
> > In libxfs_getbuf*(), we're supposed to return an unused buffer which
> > has a clean state.  Unfortunately, things like UNCHECKED can hang
> > around to cause incorrect verifier errors later, so change those
> > functions to launder the state bits clean.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com>
> > ---
> >  libxfs/rdwr.c |   47 +++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 41 insertions(+), 6 deletions(-)
> > 
> > 
> > diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> > index 4f8212f..d28cea8 100644
> > --- a/libxfs/rdwr.c
> > +++ b/libxfs/rdwr.c
> > @@ -631,15 +631,39 @@ libxfs_getbuf_flags(struct xfs_buftarg *btp, xfs_daddr_t blkno, int len,
> >  	return __cache_lookup(&key, flags);
> >  }
> >  
> > +/*
> > + * Clean the buffer flags for libxfs_getbuf*(), which wants to return
> > + * an unused buffer with clean state.  This prevents CRC errors on a
> > + * re-read of a corrupt block that was prefetched and freed.  This
> > + * can happen with a massively corrupt directory that is discarded,
> > + * but whose blocks are then recycled into expanding lost+found.
> > + *
> > + * Note however that if the buffer's dirty (prefetch calls getbuf)
> > + * we'll leave the state alone because we don't want to discard blocks
> > + * that have been fixed.
> > + */
> > +static void
> > +try_clean_buf(
> 
> Only thing I don't like about this patch is the name of this
> function. It's really a "reset buffer state" function, so I think
> that calling it something like reset_buf_state() would be more
> appropriate.

Done.

--D

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david at fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs