Re: [PATCH 04/11] libxfs: clear buffer state flags in libxfs_getbuf and variants
From: Darrick J. Wong <hidden>
Date: 2015-08-26 04:06:00
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:quoted
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 <redacted> --- 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@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs