diff options
author | Sage Weil <sage@newdream.net> | 2008-08-04 10:41:27 -0400 |
---|---|---|
committer | Chris Mason <chris.mason@oracle.com> | 2008-09-25 11:04:06 -0400 |
commit | 9ca9ee09c176a814189063c8b88f75c8f8e4ad19 (patch) | |
tree | 3092ed3f5dd472e66a61da9306dfa6839604c42c | |
parent | 3117a77370b6cb902191568e4e647cdcba083d0a (diff) |
Btrfs: fix ioctl-initiated transactions vs wait_current_trans()
Commit 597:466b27332893 (btrfs_start_transaction: wait for commits in
progress) breaks the transaction start/stop ioctls by making
btrfs_start_transaction conditionally wait for the next transaction to
start. If an application artificially is holding a transaction open,
things deadlock.
This workaround maintains a count of open ioctl-initiated transactions in
fs_info, and avoids wait_current_trans() if any are currently open (in
start_transaction() and btrfs_throttle()). The start transaction ioctl
uses a new btrfs_start_ioctl_transaction() that _does_ call
wait_current_trans(), effectively pushing the join/wait decision to the
outer ioctl-initiated transaction.
This more or less neuters btrfs_throttle() when ioctl-initiated
transactions are in use, but that seems like a pretty fundamental
consequence of wrapping lots of write()'s in a transaction. Btrfs has no
way to tell if the application considers a given operation as part of it's
transaction.
Obviously, if the transaction start/stop ioctls aren't being used, there
is no effect on current behavior.
Signed-off-by: Sage Weil <sage@newdream.net>
---
ctree.h | 1 +
ioctl.c | 12 +++++++++++-
transaction.c | 18 +++++++++++++-----
transaction.h | 2 ++
4 files changed, 27 insertions(+), 6 deletions(-)
Signed-off-by: Chris Mason <chris.mason@oracle.com>
-rw-r--r-- | fs/btrfs/ctree.h | 1 | ||||
-rw-r--r-- | fs/btrfs/ioctl.c | 12 | ||||
-rw-r--r-- | fs/btrfs/transaction.c | 18 | ||||
-rw-r--r-- | fs/btrfs/transaction.h | 2 |
4 files changed, 27 insertions, 6 deletions
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 9b025960bbde..62499dd761b7 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h | |||
@@ -518,6 +518,7 @@ struct btrfs_fs_info { | |||
518 | 518 | ||
519 | u64 generation; | 519 | u64 generation; |
520 | u64 last_trans_committed; | 520 | u64 last_trans_committed; |
521 | u64 open_ioctl_trans; | ||
521 | unsigned long mount_opt; | 522 | unsigned long mount_opt; |
522 | u64 max_extent; | 523 | u64 max_extent; |
523 | u64 max_inline; | 524 | u64 max_inline; |
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 224da287b3ed..0b63c3c77cfd 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c | |||
@@ -715,7 +715,12 @@ long btrfs_ioctl_trans_start(struct file *file) | |||
715 | ret = -EINPROGRESS; | 715 | ret = -EINPROGRESS; |
716 | goto out; | 716 | goto out; |
717 | } | 717 | } |
718 | trans = btrfs_start_transaction(root, 0); | 718 | |
719 | mutex_lock(&root->fs_info->trans_mutex); | ||
720 | root->fs_info->open_ioctl_trans++; | ||
721 | mutex_unlock(&root->fs_info->trans_mutex); | ||
722 | |||
723 | trans = btrfs_start_ioctl_transaction(root, 0); | ||
719 | if (trans) | 724 | if (trans) |
720 | file->private_data = trans; | 725 | file->private_data = trans; |
721 | else | 726 | else |
@@ -745,6 +750,11 @@ long btrfs_ioctl_trans_end(struct file *file) | |||
745 | } | 750 | } |
746 | btrfs_end_transaction(trans, root); | 751 | btrfs_end_transaction(trans, root); |
747 | file->private_data = 0; | 752 | file->private_data = 0; |
753 | |||
754 | mutex_lock(&root->fs_info->trans_mutex); | ||
755 | root->fs_info->open_ioctl_trans--; | ||
756 | mutex_unlock(&root->fs_info->trans_mutex); | ||
757 | |||
748 | out: | 758 | out: |
749 | return ret; | 759 | return ret; |
750 | } | 760 | } |
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index cf73342e8215..a2c821e3c3a7 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c | |||
@@ -152,14 +152,14 @@ static void wait_current_trans(struct btrfs_root *root) | |||
152 | } | 152 | } |
153 | 153 | ||
154 | struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, | 154 | struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, |
155 | int num_blocks, int join) | 155 | int num_blocks, int wait) |
156 | { | 156 | { |
157 | struct btrfs_trans_handle *h = | 157 | struct btrfs_trans_handle *h = |
158 | kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); | 158 | kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); |
159 | int ret; | 159 | int ret; |
160 | 160 | ||
161 | mutex_lock(&root->fs_info->trans_mutex); | 161 | mutex_lock(&root->fs_info->trans_mutex); |
162 | if (!join) | 162 | if ((wait == 1 && !root->fs_info->open_ioctl_trans) || wait == 2) |
163 | wait_current_trans(root); | 163 | wait_current_trans(root); |
164 | ret = join_transaction(root); | 164 | ret = join_transaction(root); |
165 | BUG_ON(ret); | 165 | BUG_ON(ret); |
@@ -180,14 +180,21 @@ struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, | |||
180 | struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root, | 180 | struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root, |
181 | int num_blocks) | 181 | int num_blocks) |
182 | { | 182 | { |
183 | return start_transaction(root, num_blocks, 0); | 183 | return start_transaction(root, num_blocks, 1); |
184 | } | 184 | } |
185 | struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, | 185 | struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, |
186 | int num_blocks) | 186 | int num_blocks) |
187 | { | 187 | { |
188 | return start_transaction(root, num_blocks, 1); | 188 | return start_transaction(root, num_blocks, 0); |
189 | } | 189 | } |
190 | 190 | ||
191 | struct btrfs_trans_handle *btrfs_start_ioctl_transaction(struct btrfs_root *r, | ||
192 | int num_blocks) | ||
193 | { | ||
194 | return start_transaction(r, num_blocks, 2); | ||
195 | } | ||
196 | |||
197 | |||
191 | static noinline int wait_for_commit(struct btrfs_root *root, | 198 | static noinline int wait_for_commit(struct btrfs_root *root, |
192 | struct btrfs_transaction *commit) | 199 | struct btrfs_transaction *commit) |
193 | { | 200 | { |
@@ -247,7 +254,8 @@ harder: | |||
247 | void btrfs_throttle(struct btrfs_root *root) | 254 | void btrfs_throttle(struct btrfs_root *root) |
248 | { | 255 | { |
249 | mutex_lock(&root->fs_info->trans_mutex); | 256 | mutex_lock(&root->fs_info->trans_mutex); |
250 | wait_current_trans(root); | 257 | if (!root->fs_info->open_ioctl_trans) |
258 | wait_current_trans(root); | ||
251 | mutex_unlock(&root->fs_info->trans_mutex); | 259 | mutex_unlock(&root->fs_info->trans_mutex); |
252 | 260 | ||
253 | throttle_on_drops(root); | 261 | throttle_on_drops(root); |
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h index f5adb23151fb..2c73caeebb2c 100644 --- a/fs/btrfs/transaction.h +++ b/fs/btrfs/transaction.h | |||
@@ -83,6 +83,8 @@ struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root, | |||
83 | int num_blocks); | 83 | int num_blocks); |
84 | struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, | 84 | struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, |
85 | int num_blocks); | 85 | int num_blocks); |
86 | struct btrfs_trans_handle *btrfs_start_ioctl_transaction(struct btrfs_root *r, | ||
87 | int num_blocks); | ||
86 | int btrfs_write_and_wait_transaction(struct btrfs_trans_handle *trans, | 88 | int btrfs_write_and_wait_transaction(struct btrfs_trans_handle *trans, |
87 | struct btrfs_root *root); | 89 | struct btrfs_root *root); |
88 | int btrfs_commit_tree_roots(struct btrfs_trans_handle *trans, | 90 | int btrfs_commit_tree_roots(struct btrfs_trans_handle *trans, |