Re: [PATCH V2 1/6] Btrfs: introduce a mutex lock for btrfs quota operations

[Jan Schmidt <list.btrfs@jan-o-sch.net>]

Hi Wang,

On Thu, March 28, 2013 at 11:53 (+0100), Wang Shilong wrote:
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> This patch introduces mutex lock 'quota_lock', and makes
> all the user change for quota protected by quota_lock.

Can you please add a few lines why this lock is needed? I.e., which ioctls fail
without that kind of synchronization?

> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>  fs/btrfs/ctree.h   |    3 +++
>  fs/btrfs/disk-io.c |    1 +
>  fs/btrfs/ioctl.c   |   16 ++++++++++++----
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 6e81860..a11a8ed 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1584,6 +1584,9 @@ struct btrfs_fs_info {
>  	struct rb_root qgroup_tree;
>  	spinlock_t qgroup_lock;
>  
> +	/* protect user change operations for quota */
> +	struct mutex quota_lock;

Having fs_info->qgroup_lock and fs_info->quota_lock going to be a major source
of confusion. I'd call the new one qgroup_ioctl_lock or ioctl_qgroup_lock instead.

Furthermore, the term "quota" was intentionally left unused to leave room for
other quota implementations later (user quota).

And, please, use --thread with git send-email for related patches to get correct
headers.

Thanks,
-Jan

> [snip]