Btrfs: don't clear uptodate if the eb is under IO

So I have an awful exercise script that will run snapshot, balance and
send/receive in parallel.  This sometimes would crash spectacularly and when it
came back up the fs would be completely hosed.  Turns out this is because of a
bad interaction of balance and send/receive.  Send will hold onto its entire
path for the whole send, but its blocks could get relocated out from underneath
it, and because it doesn't old tree locks theres nothing to keep this from
happening.  So it will go to read in a slot with an old transid, and we could
have re-allocated this block for something else and it could have a completely
different transid.  But because we think it is invalid we clear uptodate and
re-read in the block.  If we do this before we actually write out the new block
we could write back stale data to the fs, and boom we're screwed.

Now we definitely need to fix this disconnect between send and balance, but we
really really need to not allow ourselves to accidently read in stale data over
new data.  So make sure we check if the extent buffer is not under io before
clearing uptodate, this will kick back EIO to the caller instead of reading in
stale data and keep us from corrupting the fs.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: Chris Mason <clm@fb.com>

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index d9698fda2d12c086edd5d0390202f809cd5c46ef..98fe701933976c71473f16eed8d571c3576940a2 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -329,6 +329,8 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
 {
        struct extent_state *cached_state = NULL;
        int ret;
+       bool need_lock = (current->journal_info ==
+                         (void *)BTRFS_SEND_TRANS_STUB);
 
        if (!parent_transid || btrfs_header_generation(eb) == parent_transid)
                return 0;
@@ -336,6 +338,11 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
        if (atomic)
                return -EAGAIN;
 
+       if (need_lock) {
+               btrfs_tree_read_lock(eb);
+               btrfs_set_lock_blocking_rw(eb, BTRFS_READ_LOCK);
+       }
+
        lock_extent_bits(io_tree, eb->start, eb->start + eb->len - 1,
                         0, &cached_state);
        if (extent_buffer_uptodate(eb) &&
@@ -347,10 +354,21 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
                       "found %llu\n",
                       eb->start, parent_transid, btrfs_header_generation(eb));
        ret = 1;
-       clear_extent_buffer_uptodate(eb);
+
+       /*
+        * Things reading via commit roots that don't have normal protection,
+        * like send, can have a really old block in cache that may point at a
+        * block that has been free'd and re-allocated.  So don't clear uptodate
+        * if we find an eb that is under IO (dirty/writeback) because we could
+        * end up reading in the stale data and then writing it back out and
+        * making everybody very sad.
+        */
+       if (!extent_buffer_under_io(eb))
+               clear_extent_buffer_uptodate(eb);
 out:
        unlock_extent_cached(io_tree, eb->start, eb->start + eb->len - 1,
                             &cached_state, GFP_NOFS);
+       btrfs_tree_read_unlock_blocking(eb);
        return ret;
 }
 

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index fd78c84821c8315f65a58b1293058e0b6a27ec7f..d35a3ca15fb5c446b39583faeaa625009838073e 100644 (file)
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4315,7 +4315,7 @@ static void __free_extent_buffer(struct extent_buffer *eb)
        kmem_cache_free(extent_buffer_cache, eb);
 }
 
-static int extent_buffer_under_io(struct extent_buffer *eb)
+int extent_buffer_under_io(struct extent_buffer *eb)
 {
        return (atomic_read(&eb->io_pages) ||
                test_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags) ||

diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 58b27e5ab52158a2d03a1bf7d7d612878489a9ac..c488b45237bf82c544471338d3f13ba6a1ab41d0 100644 (file)
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -320,6 +320,7 @@ int set_extent_buffer_dirty(struct extent_buffer *eb);
 int set_extent_buffer_uptodate(struct extent_buffer *eb);
 int clear_extent_buffer_uptodate(struct extent_buffer *eb);
 int extent_buffer_uptodate(struct extent_buffer *eb);
+int extent_buffer_under_io(struct extent_buffer *eb);
 int map_private_extent_buffer(struct extent_buffer *eb, unsigned long offset,
                      unsigned long min_len, char **map,
                      unsigned long *map_start,

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 9b6da9d55f9a8778c47cf64c15a143951fc2ccf7..d00534b78382df62eb7f2a310f64ffd6747f2d27 100644 (file)
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5718,7 +5718,9 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
                        NULL);
        sort_clone_roots = 1;
 
+       current->journal_info = (void *)BTRFS_SEND_TRANS_STUB;
        ret = send_subvol(sctx);
+       current->journal_info = NULL;
        if (ret < 0)
                goto out;
 

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index a04707f740d6aed809ccbf477cfc3305dfe5c44f..038177cfabbb3da52535e64273b43d21775ee3a4 100644 (file)
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -375,7 +375,8 @@ start_transaction(struct btrfs_root *root, u64 num_items, unsigned int type,
        if (test_bit(BTRFS_FS_STATE_ERROR, &root->fs_info->fs_state))
                return ERR_PTR(-EROFS);
 
-       if (current->journal_info) {
+       if (current->journal_info &&
+           current->journal_info != (void *)BTRFS_SEND_TRANS_STUB) {
                WARN_ON(type & TRANS_EXTWRITERS);
                h = current->journal_info;
                h->use_count++;

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 6ac037e9f9f0557524b7ed29b6b72a768f7a23cf..2bcba89d952e877d93ff924130c3258227fcc986 100644 (file)
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -78,6 +78,8 @@ struct btrfs_transaction {
 #define TRANS_EXTWRITERS       (__TRANS_USERSPACE | __TRANS_START |    \
                                 __TRANS_ATTACH)
 
+#define BTRFS_SEND_TRANS_STUB  1
+
 struct btrfs_trans_handle {
        u64 transid;
        u64 bytes_reserved;