Thread (25 messages) 25 messages, 2 authors, 2015-09-28

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help