Re: [PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
On Wed, Aug 30, 2017 at 04:33:16PM +0900, Misono, Tomohiro wrote: > Currently, "btrfs quota enable" would fail after "btrfs quota disable" on > the first time with syslog output "qgroup_rescan_init failed with -22", but > it would succeed on the second time. > > When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be > set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd > in btrfs_run_qgroups() (which is called in btrfs_commit_transaction()) > because quota_root has already been freed. If "quota enable" is called > after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag > would be dropped in the btrfs_run_qgroups() since quota_root is not NULL. > This leads to the failure of "quota enable" on the first time. > > BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable" > context and is equivalent to whether quota_root is NULL or not. > btrfs_run_qgroups() checks whether quota_root is NULL or not in the first > place. > > So, let's remove BTRFS_FS_QUOTA_DISABLING flag. The state bits and transitions are messy and keeping them consistent could lead to the problmes you observe. By the analysis above it looks correct to me to remove the 'disabling' bit. Reviewed-by: David Sterba -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: [PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
Hello,
Does anyone have a comment on this?
Regards, Tomohiro
Forwarded Message
Subject: [PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
Date: Wed, 30 Aug 2017 16:33:16 +0900
From: Misono, Tomohiro
To: linux-btrfs@vger.kernel.org
Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
the first time with syslog output "qgroup_rescan_init failed with -22", but
it would succeed on the second time.
When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
because quota_root has already been freed. If "quota enable" is called
after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
This leads to the failure of "quota enable" on the first time.
BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
context and is equivalent to whether quota_root is NULL or not.
btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
place.
So, let's remove BTRFS_FS_QUOTA_DISABLING flag.
Signed-off-by: Tomohiro Misono
---
fs/btrfs/ctree.h | 1 -
fs/btrfs/qgroup.c | 4
2 files changed, 5 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3f3eb7b..49501c0 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -709,7 +709,6 @@ struct btrfs_delayed_root;
#define BTRFS_FS_OPEN 5
#define BTRFS_FS_QUOTA_ENABLED 6
#define BTRFS_FS_QUOTA_ENABLING7
-#define BTRFS_FS_QUOTA_DISABLING 8
#define BTRFS_FS_UPDATE_UUID_TREE_GEN 9
#define BTRFS_FS_CREATING_FREE_SPACE_TREE 10
#define BTRFS_FS_BTREE_ERR 11
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4ce351e..f0b111e 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct btrfs_trans_handle
*trans,
}
ret = 0;
out:
- set_bit(BTRFS_FS_QUOTA_DISABLING, &root->fs_info->flags);
btrfs_free_path(path);
return ret;
}
@@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
if (!fs_info->quota_root)
goto out;
clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- set_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags);
btrfs_qgroup_wait_for_completion(fs_info, false);
spin_lock(&fs_info->qgroup_lock);
quota_root = fs_info->quota_root;
@@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, &fs_info->flags))
set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags))
- clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
spin_lock(&fs_info->qgroup_lock);
while (!list_empty(&fs_info->dirty_qgroups)) {
--
2.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
the first time with syslog output "qgroup_rescan_init failed with -22", but
it would succeed on the second time.
When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
because quota_root has already been freed. If "quota enable" is called
after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
This leads to the failure of "quota enable" on the first time.
BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
context and is equivalent to whether quota_root is NULL or not.
btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
place.
So, let's remove BTRFS_FS_QUOTA_DISABLING flag.
Signed-off-by: Tomohiro Misono
---
fs/btrfs/ctree.h | 1 -
fs/btrfs/qgroup.c | 4
2 files changed, 5 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3f3eb7b..49501c0 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -709,7 +709,6 @@ struct btrfs_delayed_root;
#define BTRFS_FS_OPEN 5
#define BTRFS_FS_QUOTA_ENABLED 6
#define BTRFS_FS_QUOTA_ENABLING7
-#define BTRFS_FS_QUOTA_DISABLING 8
#define BTRFS_FS_UPDATE_UUID_TREE_GEN 9
#define BTRFS_FS_CREATING_FREE_SPACE_TREE 10
#define BTRFS_FS_BTREE_ERR 11
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4ce351e..f0b111e 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct btrfs_trans_handle
*trans,
}
ret = 0;
out:
- set_bit(BTRFS_FS_QUOTA_DISABLING, &root->fs_info->flags);
btrfs_free_path(path);
return ret;
}
@@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
if (!fs_info->quota_root)
goto out;
clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- set_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags);
btrfs_qgroup_wait_for_completion(fs_info, false);
spin_lock(&fs_info->qgroup_lock);
quota_root = fs_info->quota_root;
@@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, &fs_info->flags))
set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags))
- clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
spin_lock(&fs_info->qgroup_lock);
while (!list_empty(&fs_info->dirty_qgroups)) {
--
2.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
Sorry, this patch contains leading spaces, I will resend this soon.
On 2017/08/30 10:51, Misono, Tomohiro wrote:
> Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
> the first time with syslog output "qgroup_rescan_init failed with -22", but
> it would succeed on the second time.
>
> When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
> set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
> in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
> because quota_root has already been freed. If "quota enable" is called
> after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
> would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
> This leads to the failure of "quota enable" on the first time.
>
> BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
> context and is equivalent to whether quota_root is NULL or not.
> btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
> place.
>
> So, let's remove BTRFS_FS_QUOTA_DISABLING flag.
>
> Signed-off-by: Tomohiro Misono
> ---
> fs/btrfs/ctree.h | 1 -
> fs/btrfs/qgroup.c | 4
> 2 files changed, 5 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 3f3eb7b..49501c0 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -709,7 +709,6 @@ struct btrfs_delayed_root;
> #define BTRFS_FS_OPEN 5
> #define BTRFS_FS_QUOTA_ENABLED 6
> #define BTRFS_FS_QUOTA_ENABLING 7
> -#define BTRFS_FS_QUOTA_DISABLING 8
> #define BTRFS_FS_UPDATE_UUID_TREE_GEN 9
> #define BTRFS_FS_CREATING_FREE_SPACE_TREE 10
> #define BTRFS_FS_BTREE_ERR 11
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 4ce351e..f0b111e 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct
> btrfs_trans_handle *trans,
> }
> ret = 0;
> out:
> - set_bit(BTRFS_FS_QUOTA_DISABLING, &root->fs_info->flags);
> btrfs_free_path(path);
> return ret;
> }
> @@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle
> *trans,
> if (!fs_info->quota_root)
> goto out;
> clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
> - set_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags);
> btrfs_qgroup_wait_for_completion(fs_info, false);
> spin_lock(&fs_info->qgroup_lock);
> quota_root = fs_info->quota_root;
> @@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle
> *trans,
>
> if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, &fs_info->flags))
> set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
> - if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags))
> - clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
>
> spin_lock(&fs_info->qgroup_lock);
> while (!list_empty(&fs_info->dirty_qgroups)) {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: remove BTRFS_FS_QUOTA_DISABLING flag
Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
the first time with syslog output "qgroup_rescan_init failed with -22", but
it would succeed on the second time.
When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
because quota_root has already been freed. If "quota enable" is called
after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
This leads to the failure of "quota enable" on the first time.
BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
context and is equivalent to whether quota_root is NULL or not.
btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
place.
So, let's remove BTRFS_FS_QUOTA_DISABLING flag.
Signed-off-by: Tomohiro Misono
---
fs/btrfs/ctree.h | 1 -
fs/btrfs/qgroup.c | 4
2 files changed, 5 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3f3eb7b..49501c0 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -709,7 +709,6 @@ struct btrfs_delayed_root;
#define BTRFS_FS_OPEN 5
#define BTRFS_FS_QUOTA_ENABLED 6
#define BTRFS_FS_QUOTA_ENABLING7
-#define BTRFS_FS_QUOTA_DISABLING 8
#define BTRFS_FS_UPDATE_UUID_TREE_GEN 9
#define BTRFS_FS_CREATING_FREE_SPACE_TREE 10
#define BTRFS_FS_BTREE_ERR 11
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4ce351e..f0b111e 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct
btrfs_trans_handle *trans,
}
ret = 0;
out:
- set_bit(BTRFS_FS_QUOTA_DISABLING, &root->fs_info->flags);
btrfs_free_path(path);
return ret;
}
@@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle
*trans,
if (!fs_info->quota_root)
goto out;
clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- set_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags);
btrfs_qgroup_wait_for_completion(fs_info, false);
spin_lock(&fs_info->qgroup_lock);
quota_root = fs_info->quota_root;
@@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle
*trans,
if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, &fs_info->flags))
set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, &fs_info->flags))
- clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
spin_lock(&fs_info->qgroup_lock);
while (!list_empty(&fs_info->dirty_qgroups)) {
--
2.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
