Commit 9ca9ee09 authored by Sage Weil's avatar Sage Weil Committed by Chris Mason

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: default avatarSage 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: default avatarChris Mason <chris.mason@oracle.com>
parent 3117a773
...@@ -518,6 +518,7 @@ struct btrfs_fs_info { ...@@ -518,6 +518,7 @@ struct btrfs_fs_info {
u64 generation; u64 generation;
u64 last_trans_committed; u64 last_trans_committed;
u64 open_ioctl_trans;
unsigned long mount_opt; unsigned long mount_opt;
u64 max_extent; u64 max_extent;
u64 max_inline; u64 max_inline;
......
...@@ -715,7 +715,12 @@ long btrfs_ioctl_trans_start(struct file *file) ...@@ -715,7 +715,12 @@ long btrfs_ioctl_trans_start(struct file *file)
ret = -EINPROGRESS; ret = -EINPROGRESS;
goto out; goto out;
} }
trans = btrfs_start_transaction(root, 0);
mutex_lock(&root->fs_info->trans_mutex);
root->fs_info->open_ioctl_trans++;
mutex_unlock(&root->fs_info->trans_mutex);
trans = btrfs_start_ioctl_transaction(root, 0);
if (trans) if (trans)
file->private_data = trans; file->private_data = trans;
else else
...@@ -745,6 +750,11 @@ long btrfs_ioctl_trans_end(struct file *file) ...@@ -745,6 +750,11 @@ long btrfs_ioctl_trans_end(struct file *file)
} }
btrfs_end_transaction(trans, root); btrfs_end_transaction(trans, root);
file->private_data = 0; file->private_data = 0;
mutex_lock(&root->fs_info->trans_mutex);
root->fs_info->open_ioctl_trans--;
mutex_unlock(&root->fs_info->trans_mutex);
out: out:
return ret; return ret;
} }
......
...@@ -152,14 +152,14 @@ static void wait_current_trans(struct btrfs_root *root) ...@@ -152,14 +152,14 @@ static void wait_current_trans(struct btrfs_root *root)
} }
struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, struct btrfs_trans_handle *start_transaction(struct btrfs_root *root,
int num_blocks, int join) int num_blocks, int wait)
{ {
struct btrfs_trans_handle *h = struct btrfs_trans_handle *h =
kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS); kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS);
int ret; int ret;
mutex_lock(&root->fs_info->trans_mutex); mutex_lock(&root->fs_info->trans_mutex);
if (!join) if ((wait == 1 && !root->fs_info->open_ioctl_trans) || wait == 2)
wait_current_trans(root); wait_current_trans(root);
ret = join_transaction(root); ret = join_transaction(root);
BUG_ON(ret); BUG_ON(ret);
...@@ -180,14 +180,21 @@ struct btrfs_trans_handle *start_transaction(struct btrfs_root *root, ...@@ -180,14 +180,21 @@ struct btrfs_trans_handle *start_transaction(struct btrfs_root *root,
struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root, struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
int num_blocks) int num_blocks)
{ {
return start_transaction(root, num_blocks, 0); return start_transaction(root, num_blocks, 1);
} }
struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root,
int num_blocks) int num_blocks)
{ {
return start_transaction(root, num_blocks, 1); return start_transaction(root, num_blocks, 0);
} }
struct btrfs_trans_handle *btrfs_start_ioctl_transaction(struct btrfs_root *r,
int num_blocks)
{
return start_transaction(r, num_blocks, 2);
}
static noinline int wait_for_commit(struct btrfs_root *root, static noinline int wait_for_commit(struct btrfs_root *root,
struct btrfs_transaction *commit) struct btrfs_transaction *commit)
{ {
...@@ -247,7 +254,8 @@ static void throttle_on_drops(struct btrfs_root *root) ...@@ -247,7 +254,8 @@ static void throttle_on_drops(struct btrfs_root *root)
void btrfs_throttle(struct btrfs_root *root) void btrfs_throttle(struct btrfs_root *root)
{ {
mutex_lock(&root->fs_info->trans_mutex); mutex_lock(&root->fs_info->trans_mutex);
wait_current_trans(root); if (!root->fs_info->open_ioctl_trans)
wait_current_trans(root);
mutex_unlock(&root->fs_info->trans_mutex); mutex_unlock(&root->fs_info->trans_mutex);
throttle_on_drops(root); throttle_on_drops(root);
......
...@@ -83,6 +83,8 @@ struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root, ...@@ -83,6 +83,8 @@ struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
int num_blocks); int num_blocks);
struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root, struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root,
int num_blocks); int num_blocks);
struct btrfs_trans_handle *btrfs_start_ioctl_transaction(struct btrfs_root *r,
int num_blocks);
int btrfs_write_and_wait_transaction(struct btrfs_trans_handle *trans, int btrfs_write_and_wait_transaction(struct btrfs_trans_handle *trans,
struct btrfs_root *root); struct btrfs_root *root);
int btrfs_commit_tree_roots(struct btrfs_trans_handle *trans, int btrfs_commit_tree_roots(struct btrfs_trans_handle *trans,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment