Btrfs: fix wrong handle at error path of create_snapshot() when the commit fails
There are several bugs at error path of create_snapshot() when the
transaction commitment failed.
- access the freed transaction handler. At the end of the
transaction commitment, the transaction handler was freed, so we
should not access it after the transaction commitment.
- we were not aware of the error which happened during the snapshot
creation if we submitted a async transaction commitment.
- pending snapshot access vs pending snapshot free. when something
wrong happened after we submitted a async transaction commitment,
the transaction committer would cleanup the pending snapshots and
free them. But the snapshot creators were not aware of it, they
would access the freed pending snapshots.
This patch fixes the above problems by:
- remove the dangerous code that accessed the freed handler
- assign ->error if the error happens during the snapshot creation
- the transaction committer doesn't free the pending snapshots,
just assigns the error number and evicts them before we unblock
the transaction.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 02369a3..7d84651 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -62,7 +62,7 @@
static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
struct btrfs_root *root);
-static void btrfs_destroy_pending_snapshots(struct btrfs_transaction *t);
+static void btrfs_evict_pending_snapshots(struct btrfs_transaction *t);
static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root);
static int btrfs_destroy_marked_extents(struct btrfs_root *root,
struct extent_io_tree *dirty_pages,
@@ -3687,7 +3687,7 @@
return ret;
}
-static void btrfs_destroy_pending_snapshots(struct btrfs_transaction *t)
+static void btrfs_evict_pending_snapshots(struct btrfs_transaction *t)
{
struct btrfs_pending_snapshot *snapshot;
struct list_head splice;
@@ -3700,10 +3700,8 @@
snapshot = list_entry(splice.next,
struct btrfs_pending_snapshot,
list);
-
+ snapshot->error = -ECANCELED;
list_del_init(&snapshot->list);
-
- kfree(snapshot);
}
}
@@ -3840,6 +3838,8 @@
cur_trans->blocked = 1;
wake_up(&root->fs_info->transaction_blocked_wait);
+ btrfs_evict_pending_snapshots(cur_trans);
+
cur_trans->blocked = 0;
wake_up(&root->fs_info->transaction_wait);
@@ -3849,8 +3849,6 @@
btrfs_destroy_delayed_inodes(root);
btrfs_assert_delayed_root_empty(root);
- btrfs_destroy_pending_snapshots(cur_trans);
-
btrfs_destroy_marked_extents(root, &cur_trans->dirty_pages,
EXTENT_DIRTY);
btrfs_destroy_pinned_extent(root,
@@ -3894,6 +3892,8 @@
if (waitqueue_active(&root->fs_info->transaction_blocked_wait))
wake_up(&root->fs_info->transaction_blocked_wait);
+ btrfs_evict_pending_snapshots(t);
+
t->blocked = 0;
smp_mb();
if (waitqueue_active(&root->fs_info->transaction_wait))
@@ -3907,8 +3907,6 @@
btrfs_destroy_delayed_inodes(root);
btrfs_assert_delayed_root_empty(root);
- btrfs_destroy_pending_snapshots(t);
-
btrfs_destroy_delalloc_inodes(root);
spin_lock(&root->fs_info->trans_lock);
