Skip to content

Commit

Permalink
Btrfs: introduce a mutex lock for btrfs quota operations
Browse files Browse the repository at this point in the history 
The original code has one spin_lock 'qgroup_lock' to protect quota
configurations in memory. If we want to add a BTRFS_QGROUP_INFO_KEY,
it will be added to Btree firstly, and then update configurations in
memory,however, a race condition may happen between these operations.
For example:
	->add_qgroup_info_item()
		->add_qgroup_rb()

For the above case, del_qgroup_info_item() may happen just before
add_qgroup_rb().

What's worse, when we want to add a qgroup relation:
	->add_qgroup_relation_item()
		->add_qgroup_relations()

We don't have any checks whether 'src' and 'dst' exist before
add_qgroup_relation_item(), a race condition can also happen for
the above case.

To avoid race condition and have all the necessary checks, we introduce
a mutex lock 'qgroup_ioctl_lock', and we make all the user change operations
protected by the mutex lock.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
  • Loading branch information
Wang Shilong authored and Josef Bacik committed May 6, 2013
1 parent 7708f02 commit f2f6ed3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 25 deletions.
3 changes: 3 additions & 0 deletions fs/btrfs/ctree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,9 @@ struct btrfs_fs_info {
struct rb_root qgroup_tree;
spinlock_t qgroup_lock;

/* protect user change for quota operations */
struct mutex qgroup_ioctl_lock;

/* list of dirty qgroups to be written at next commit */
struct list_head dirty_qgroups;

Expand Down
1 change: 1 addition & 0 deletions fs/btrfs/disk-io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,7 @@ int open_ctree(struct super_block *sb,
mutex_init(&fs_info->dev_replace.lock);

spin_lock_init(&fs_info->qgroup_lock);
mutex_init(&fs_info->qgroup_ioctl_lock);
fs_info->qgroup_tree = RB_ROOT;
INIT_LIST_HEAD(&fs_info->dirty_qgroups);
fs_info->qgroup_seq = 1;
Expand Down
82 changes: 57 additions & 25 deletions fs/btrfs/qgroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
int ret = 0;
int slot;

mutex_lock(&fs_info->qgroup_ioctl_lock);
spin_lock(&fs_info->qgroup_lock);
if (fs_info->quota_root) {
fs_info->pending_quota_state = 1;
Expand Down Expand Up @@ -900,6 +901,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
kfree(quota_root);
}
out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand All @@ -910,10 +912,11 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
struct btrfs_root *quota_root;
int ret = 0;

mutex_lock(&fs_info->qgroup_ioctl_lock);
spin_lock(&fs_info->qgroup_lock);
if (!fs_info->quota_root) {
spin_unlock(&fs_info->qgroup_lock);
return 0;
goto out;
}
fs_info->quota_enabled = 0;
fs_info->pending_quota_state = 0;
Expand All @@ -922,8 +925,10 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
btrfs_free_qgroup_config(fs_info);
spin_unlock(&fs_info->qgroup_lock);

if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

ret = btrfs_clean_quota_tree(trans, quota_root);
if (ret)
Expand All @@ -944,6 +949,7 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
free_extent_buffer(quota_root->commit_root);
kfree(quota_root);
out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand All @@ -959,24 +965,28 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans,
struct btrfs_root *quota_root;
int ret = 0;

mutex_lock(&fs_info->qgroup_ioctl_lock);
quota_root = fs_info->quota_root;
if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

ret = add_qgroup_relation_item(trans, quota_root, src, dst);
if (ret)
return ret;
goto out;

ret = add_qgroup_relation_item(trans, quota_root, dst, src);
if (ret) {
del_qgroup_relation_item(trans, quota_root, src, dst);
return ret;
goto out;
}

spin_lock(&fs_info->qgroup_lock);
ret = add_relation_rb(quota_root->fs_info, src, dst);
spin_unlock(&fs_info->qgroup_lock);

out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand All @@ -987,9 +997,12 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
int ret = 0;
int err;

mutex_lock(&fs_info->qgroup_ioctl_lock);
quota_root = fs_info->quota_root;
if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

ret = del_qgroup_relation_item(trans, quota_root, src, dst);
err = del_qgroup_relation_item(trans, quota_root, dst, src);
Expand All @@ -1000,7 +1013,8 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans,
del_relation_rb(fs_info, src, dst);

spin_unlock(&fs_info->qgroup_lock);

out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand All @@ -1011,9 +1025,12 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans,
struct btrfs_qgroup *qgroup;
int ret = 0;

mutex_lock(&fs_info->qgroup_ioctl_lock);
quota_root = fs_info->quota_root;
if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

ret = add_qgroup_item(trans, quota_root, qgroupid);

Expand All @@ -1023,7 +1040,8 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans,

if (IS_ERR(qgroup))
ret = PTR_ERR(qgroup);

out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand All @@ -1034,17 +1052,21 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans,
struct btrfs_qgroup *qgroup;
int ret = 0;

mutex_lock(&fs_info->qgroup_ioctl_lock);
quota_root = fs_info->quota_root;
if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

/* check if there are no relations to this qgroup */
spin_lock(&fs_info->qgroup_lock);
qgroup = find_qgroup_rb(fs_info, qgroupid);
if (qgroup) {
if (!list_empty(&qgroup->groups) || !list_empty(&qgroup->members)) {
spin_unlock(&fs_info->qgroup_lock);
return -EBUSY;
ret = -EBUSY;
goto out;
}
}
spin_unlock(&fs_info->qgroup_lock);
Expand All @@ -1054,20 +1076,25 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans,
spin_lock(&fs_info->qgroup_lock);
del_qgroup_rb(quota_root->fs_info, qgroupid);
spin_unlock(&fs_info->qgroup_lock);

out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

int btrfs_limit_qgroup(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info, u64 qgroupid,
struct btrfs_qgroup_limit *limit)
{
struct btrfs_root *quota_root = fs_info->quota_root;
struct btrfs_root *quota_root;
struct btrfs_qgroup *qgroup;
int ret = 0;

if (!quota_root)
return -EINVAL;
mutex_lock(&fs_info->qgroup_ioctl_lock);
quota_root = fs_info->quota_root;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

ret = update_qgroup_limit_item(trans, quota_root, qgroupid,
limit->flags, limit->max_rfer,
Expand All @@ -1094,7 +1121,8 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans,

unlock:
spin_unlock(&fs_info->qgroup_lock);

out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand Down Expand Up @@ -1392,11 +1420,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
struct btrfs_qgroup *dstgroup;
u32 level_size = 0;

mutex_lock(&fs_info->qgroup_ioctl_lock);
if (!fs_info->quota_enabled)
return 0;
goto out;

if (!quota_root)
return -EINVAL;
if (!quota_root) {
ret = -EINVAL;
goto out;
}

/*
* create a tracking group for the subvol itself
Expand Down Expand Up @@ -1523,6 +1554,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
unlock:
spin_unlock(&fs_info->qgroup_lock);
out:
mutex_unlock(&fs_info->qgroup_ioctl_lock);
return ret;
}

Expand Down

0 comments on commit f2f6ed3

Please sign in to comment.