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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|