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