repair: ensure prefetched buffers have CRCs validated

Prefetch currently does not do CRC validation when the IO completes
due to the optimisation it performs and the fact that it does not
know what the type of metadata into the buffer is supposed to be.
Hence, mark all prefetched buffers as "suspect" so that when the
end user tries to read it with a supplied validation function the
validation is run even though the buffer was already in the cache.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>

diff --git a/include/libxfs.h b/include/libxfs.h
index 6b1e27628f8d2f6ddce7c11c14015aebe616946f..9c1095758220aabfda09c804aa8d621d9c2468b9 100644 (file)
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -436,6 +436,8 @@ extern void libxfs_putbuf (xfs_buf_t *);
 
 #endif
 
+extern void    libxfs_readbuf_verify(struct xfs_buf *bp,
+                       const struct xfs_buf_ops *ops);
 extern xfs_buf_t *libxfs_getsb(xfs_mount_t *, int);
 extern void    libxfs_bcache_purge(void);
 extern void    libxfs_bcache_flush(void);

diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 7208a2f0be86e672d4f7b5d7dd8e28c32863d4c5..ea4bdfd58f91f0a4ff5bfb9d2310f6db3b69e30d 100644 (file)
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -708,6 +708,17 @@ libxfs_readbufr(struct xfs_buftarg *btp, xfs_daddr_t blkno, xfs_buf_t *bp,
        return error;
 }
 
+void
+libxfs_readbuf_verify(struct xfs_buf *bp, const struct xfs_buf_ops *ops)
+{
+       if (!ops)
+               return;
+       bp->b_ops = ops;
+       bp->b_ops->verify_read(bp);
+       bp->b_flags &= ~LIBXFS_B_UNCHECKED;
+}
+
+
 xfs_buf_t *
 libxfs_readbuf(struct xfs_buftarg *btp, xfs_daddr_t blkno, int len, int flags,
                const struct xfs_buf_ops *ops)
@@ -718,23 +729,38 @@ libxfs_readbuf(struct xfs_buftarg *btp, xfs_daddr_t blkno, int len, int flags,
        bp = libxfs_getbuf(btp, blkno, len);
        if (!bp)
                return NULL;
-       if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY)))
+
+       /*
+        * if the buffer was prefetched, it is likely that it was not validated.
+        * Hence if we are supplied an ops function and the buffer is marked as
+        * unchecked, we need to validate it now.
+        *
+        * We do this verification even if the buffer is dirty - the
+        * verification is almost certainly going to fail the CRC check in this
+        * case as a dirty buffer has not had the CRC recalculated. However, we
+        * should not be dirtying unchecked buffers and therefore failing it
+        * here because it's dirty and unchecked indicates we've screwed up
+        * somewhere else.
+        */
+       bp->b_error = 0;
+       if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
+               if (bp->b_flags & LIBXFS_B_UNCHECKED)
+                       libxfs_readbuf_verify(bp, ops);
                return bp;
+       }
 
        /*
-        * only set the ops on a cache miss (i.e. first physical read) as the
-        * verifier may change the ops to match the typ eof buffer it contains.
+        * Set the ops on a cache miss (i.e. first physical read) as the
+        * verifier may change the ops to match the type of buffer it contains.
         * A cache hit might reset the verifier to the original type if we set
         * it again, but it won't get called again and set to match the buffer
         * contents. *cough* xfs_da_node_buf_ops *cough*.
         */
-       bp->b_error = 0;
-       bp->b_ops = ops;
        error = libxfs_readbufr(btp, blkno, bp, len, flags);
        if (error)
                bp->b_error = error;
-       else if (bp->b_ops)
-               bp->b_ops->verify_read(bp);
+       else
+               libxfs_readbuf_verify(bp, ops);
        return bp;
 }
 
@@ -786,16 +812,15 @@ libxfs_readbuf_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
                return NULL;
 
        bp->b_error = 0;
-       bp->b_ops = ops;
-       if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY)))
+       if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
+               if (bp->b_flags & LIBXFS_B_UNCHECKED)
+                       libxfs_readbuf_verify(bp, ops);
                return bp;
-
-       error = libxfs_readbufr_map(btp, bp, flags);
-       if (!error) {
-               bp->b_flags |= LIBXFS_B_UPTODATE;
-               if (bp->b_ops)
-                       bp->b_ops->verify_read(bp);
        }
+       error = libxfs_readbufr_map(btp, bp, flags);
+       if (!error)
+               libxfs_readbuf_verify(bp, ops);
+
 #ifdef IO_DEBUG
        printf("%lx: %s: read %lu bytes, error %d, blkno=%llu(%llu), %p\n",
                pthread_self(), __FUNCTION__, buf - (char *)bp->b_addr, error,
@@ -888,7 +913,8 @@ libxfs_writebufr(xfs_buf_t *bp)
 #endif
        if (!error) {
                bp->b_flags |= LIBXFS_B_UPTODATE;
-               bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT);
+               bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT |
+                                LIBXFS_B_UNCHECKED);
        }
        return error;
 }

diff --git a/repair/prefetch.c b/repair/prefetch.c
index 6d6d344c3ccec67418f33521a648dc445d4ae963..65fedf564d882495854af69a3a79c591d52effb1 100644 (file)
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -387,8 +387,7 @@ pf_read_inode_dirs(
        int                     hasdir = 0;
        int                     isadir;
 
-       bp->b_ops = &xfs_inode_buf_ops;
-       bp->b_ops->verify_read(bp);
+       libxfs_readbuf_verify(bp, &xfs_inode_buf_ops);
        if (bp->b_error)
                return;
 
@@ -460,6 +459,7 @@ pf_read_discontig(
 
        pthread_mutex_unlock(&args->lock);
        libxfs_readbufr_map(mp->m_ddev_targp, bp, 0);
+       bp->b_flags |= LIBXFS_B_UNCHECKED;
        libxfs_putbuf(bp);
        pthread_mutex_lock(&args->lock);
 }
@@ -582,7 +582,8 @@ pf_batch_read(
                                if (len < size)
                                        break;
                                memcpy(XFS_BUF_PTR(bplist[i]), pbuf, size);
-                               bplist[i]->b_flags |= LIBXFS_B_UPTODATE;
+                               bplist[i]->b_flags |= (LIBXFS_B_UPTODATE |
+                                                      LIBXFS_B_UNCHECKED);
                                len -= size;
                                if (B_IS_INODE(XFS_BUF_PRIORITY(bplist[i])))
                                        pf_read_inode_dirs(args, bplist[i]);