* [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
@ 2022-02-01 12:53 Sidong Yang
2022-02-01 13:06 ` Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Sidong Yang @ 2022-02-01 12:53 UTC (permalink / raw)
To: linux-btrfs, quwenruo.btrfs; +Cc: Sidong Yang
This patch replaces the code modifying or checking qgroup_flags to
bitops like set_bit or test_bit. These bit operations works atomically
that it protects qgroup_flags value.
Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
fs/btrfs/ioctl.c | 2 +-
fs/btrfs/qgroup.c | 68 ++++++++++++++++++++++-------------------------
2 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index cc61813213d8..19b0b59abe77 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4435,7 +4435,7 @@ static long btrfs_ioctl_quota_rescan_status(struct btrfs_fs_info *fs_info,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
+ if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
qsa.flags = 1;
qsa.progress = fs_info->qgroup_rescan_progress.objectid;
}
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index db680f5be745..9fabc62ff2a5 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -499,16 +499,16 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
out:
btrfs_free_path(path);
fs_info->qgroup_flags |= flags;
- if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_ON))
+ if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags))
clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- else if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN &&
+ else if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags) &&
ret >= 0)
ret = qgroup_rescan_init(fs_info, rescan_progress, 0);
if (ret < 0) {
ulist_free(fs_info->qgroup_ulist);
fs_info->qgroup_ulist = NULL;
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
btrfs_sysfs_del_qgroups(fs_info);
}
@@ -1197,7 +1197,7 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
spin_lock(&fs_info->qgroup_lock);
quota_root = fs_info->quota_root;
fs_info->quota_root = NULL;
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
spin_unlock(&fs_info->qgroup_lock);
btrfs_free_qgroup_config(fs_info);
@@ -1353,7 +1353,7 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
}
out:
if (ret)
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
return ret;
}
@@ -1659,7 +1659,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
ret = update_qgroup_limit_item(trans, qgroup);
if (ret) {
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
btrfs_info(fs_info, "unable to update quota limit for %llu",
qgroupid);
}
@@ -1735,7 +1735,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
ret = btrfs_find_all_roots(NULL, trans->fs_info, bytenr, 0, &old_root,
true);
if (ret < 0) {
- trans->fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &trans->fs_info->qgroup_flags);
btrfs_warn(trans->fs_info,
"error accounting new delayed refs extent (err code: %d), quota inconsistent",
ret);
@@ -2211,7 +2211,7 @@ static int qgroup_trace_subtree_swap(struct btrfs_trans_handle *trans,
out:
btrfs_free_path(dst_path);
if (ret < 0)
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
return ret;
}
@@ -2581,7 +2581,7 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
}
mutex_lock(&fs_info->qgroup_rescan_lock);
- if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
+ if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
if (fs_info->qgroup_rescan_progress.objectid <= bytenr) {
mutex_unlock(&fs_info->qgroup_rescan_lock);
ret = 0;
@@ -2715,23 +2715,23 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
spin_unlock(&fs_info->qgroup_lock);
ret = update_qgroup_info_item(trans, qgroup);
if (ret)
- fs_info->qgroup_flags |=
- BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
+ &fs_info->qgroup_flags);
ret = update_qgroup_limit_item(trans, qgroup);
if (ret)
- fs_info->qgroup_flags |=
- BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
+ &fs_info->qgroup_flags);
spin_lock(&fs_info->qgroup_lock);
}
if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_ON;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
else
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
spin_unlock(&fs_info->qgroup_lock);
ret = update_qgroup_status_item(trans);
if (ret)
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
return ret;
}
@@ -2849,7 +2849,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
ret = update_qgroup_limit_item(trans, dstgroup);
if (ret) {
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
btrfs_info(fs_info,
"unable to update quota limit for %llu",
dstgroup->qgroupid);
@@ -2955,7 +2955,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
if (!committing)
mutex_unlock(&fs_info->qgroup_ioctl_lock);
if (need_rescan)
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
return ret;
}
@@ -3270,10 +3270,10 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
mutex_lock(&fs_info->qgroup_rescan_lock);
if (err > 0 &&
- fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT) {
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ test_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags)) {
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
} else if (err < 0) {
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
}
mutex_unlock(&fs_info->qgroup_rescan_lock);
@@ -3292,7 +3292,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
mutex_lock(&fs_info->qgroup_rescan_lock);
if (!stopped)
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
if (trans) {
ret = update_qgroup_status_item(trans);
if (ret < 0) {
@@ -3332,13 +3332,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
if (!init_flags) {
/* we're resuming qgroup rescan at mount time */
- if (!(fs_info->qgroup_flags &
- BTRFS_QGROUP_STATUS_FLAG_RESCAN)) {
+ if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
btrfs_warn(fs_info,
"qgroup rescan init failed, qgroup rescan is not queued");
ret = -EINVAL;
- } else if (!(fs_info->qgroup_flags &
- BTRFS_QGROUP_STATUS_FLAG_ON)) {
+ } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
btrfs_warn(fs_info,
"qgroup rescan init failed, qgroup is not enabled");
ret = -EINVAL;
@@ -3351,12 +3349,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
mutex_lock(&fs_info->qgroup_rescan_lock);
if (init_flags) {
- if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
+ if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
btrfs_warn(fs_info,
"qgroup rescan is already in progress");
ret = -EINPROGRESS;
- } else if (!(fs_info->qgroup_flags &
- BTRFS_QGROUP_STATUS_FLAG_ON)) {
+ } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
btrfs_warn(fs_info,
"qgroup rescan init failed, qgroup is not enabled");
ret = -EINVAL;
@@ -3366,7 +3363,7 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
mutex_unlock(&fs_info->qgroup_rescan_lock);
return ret;
}
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_RESCAN;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
}
memset(&fs_info->qgroup_rescan_progress, 0,
@@ -3422,12 +3419,12 @@ btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info)
trans = btrfs_join_transaction(fs_info->fs_root);
if (IS_ERR(trans)) {
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
return PTR_ERR(trans);
}
ret = btrfs_commit_transaction(trans);
if (ret) {
- fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
+ clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
return ret;
}
@@ -3471,7 +3468,7 @@ int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info,
void
btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info)
{
- if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
+ if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
mutex_lock(&fs_info->qgroup_rescan_lock);
fs_info->qgroup_rescan_running = true;
btrfs_queue_work(fs_info->qgroup_rescan_workers,
@@ -4167,8 +4164,7 @@ int btrfs_qgroup_add_swapped_blocks(struct btrfs_trans_handle *trans,
spin_unlock(&blocks->lock);
out:
if (ret < 0)
- fs_info->qgroup_flags |=
- BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
return ret;
}
@@ -4255,7 +4251,7 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
btrfs_err_rl(fs_info,
"failed to account subtree at bytenr %llu: %d",
subvol_eb->start, ret);
- fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
}
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
2022-02-01 12:53 [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops Sidong Yang
@ 2022-02-01 13:06 ` Qu Wenruo
2022-02-01 13:22 ` Sidong Yang
2022-02-01 13:56 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2022-02-01 13:06 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs
On 2022/2/1 20:53, Sidong Yang wrote:
> This patch replaces the code modifying or checking qgroup_flags to
> bitops like set_bit or test_bit. These bit operations works atomically
> that it protects qgroup_flags value.
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> fs/btrfs/ioctl.c | 2 +-
> fs/btrfs/qgroup.c | 68 ++++++++++++++++++++++-------------------------
Oh, no btrfs_tree.h nor qgroup.h, I already know what the typical
pitfall you fell into...
> 2 files changed, 33 insertions(+), 37 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index cc61813213d8..19b0b59abe77 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4435,7 +4435,7 @@ static long btrfs_ioctl_quota_rescan_status(struct btrfs_fs_info *fs_info,
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
>
> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
set/clear/test_bit() all use bit number, not the flag value...
It will still work and pass tests, as instead of
setting/clearing/testing the first, the second or the third bit, now
it's testing the 2nd, 4th and 8th bit.
It's not triggering any obvious bug because there are only 3 bits used.
So I'm afraid you need to first convert those flag values into bit
number first, which would either show up in btrfs_tree.h or internally
defined in qgroup.h.
Thanks,
Qu
> qsa.flags = 1;
> qsa.progress = fs_info->qgroup_rescan_progress.objectid;
> }
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index db680f5be745..9fabc62ff2a5 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -499,16 +499,16 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
> out:
> btrfs_free_path(path);
> fs_info->qgroup_flags |= flags;
> - if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_ON))
> + if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags))
> clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
> - else if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN &&
> + else if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags) &&
> ret >= 0)
> ret = qgroup_rescan_init(fs_info, rescan_progress, 0);
>
> if (ret < 0) {
> ulist_free(fs_info->qgroup_ulist);
> fs_info->qgroup_ulist = NULL;
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> btrfs_sysfs_del_qgroups(fs_info);
> }
>
> @@ -1197,7 +1197,7 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
> spin_lock(&fs_info->qgroup_lock);
> quota_root = fs_info->quota_root;
> fs_info->quota_root = NULL;
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> spin_unlock(&fs_info->qgroup_lock);
>
> btrfs_free_qgroup_config(fs_info);
> @@ -1353,7 +1353,7 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
> }
> out:
> if (ret)
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> return ret;
> }
>
> @@ -1659,7 +1659,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>
> ret = update_qgroup_limit_item(trans, qgroup);
> if (ret) {
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> btrfs_info(fs_info, "unable to update quota limit for %llu",
> qgroupid);
> }
> @@ -1735,7 +1735,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
> ret = btrfs_find_all_roots(NULL, trans->fs_info, bytenr, 0, &old_root,
> true);
> if (ret < 0) {
> - trans->fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &trans->fs_info->qgroup_flags);
> btrfs_warn(trans->fs_info,
> "error accounting new delayed refs extent (err code: %d), quota inconsistent",
> ret);
> @@ -2211,7 +2211,7 @@ static int qgroup_trace_subtree_swap(struct btrfs_trans_handle *trans,
> out:
> btrfs_free_path(dst_path);
> if (ret < 0)
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> return ret;
> }
>
> @@ -2581,7 +2581,7 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> }
>
> mutex_lock(&fs_info->qgroup_rescan_lock);
> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> if (fs_info->qgroup_rescan_progress.objectid <= bytenr) {
> mutex_unlock(&fs_info->qgroup_rescan_lock);
> ret = 0;
> @@ -2715,23 +2715,23 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
> spin_unlock(&fs_info->qgroup_lock);
> ret = update_qgroup_info_item(trans, qgroup);
> if (ret)
> - fs_info->qgroup_flags |=
> - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
> + &fs_info->qgroup_flags);
> ret = update_qgroup_limit_item(trans, qgroup);
> if (ret)
> - fs_info->qgroup_flags |=
> - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
> + &fs_info->qgroup_flags);
> spin_lock(&fs_info->qgroup_lock);
> }
> if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_ON;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> else
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> spin_unlock(&fs_info->qgroup_lock);
>
> ret = update_qgroup_status_item(trans);
> if (ret)
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
>
> return ret;
> }
> @@ -2849,7 +2849,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
>
> ret = update_qgroup_limit_item(trans, dstgroup);
> if (ret) {
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> btrfs_info(fs_info,
> "unable to update quota limit for %llu",
> dstgroup->qgroupid);
> @@ -2955,7 +2955,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
> if (!committing)
> mutex_unlock(&fs_info->qgroup_ioctl_lock);
> if (need_rescan)
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> return ret;
> }
>
> @@ -3270,10 +3270,10 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>
> mutex_lock(&fs_info->qgroup_rescan_lock);
> if (err > 0 &&
> - fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT) {
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + test_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags)) {
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> } else if (err < 0) {
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> }
> mutex_unlock(&fs_info->qgroup_rescan_lock);
>
> @@ -3292,7 +3292,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>
> mutex_lock(&fs_info->qgroup_rescan_lock);
> if (!stopped)
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> if (trans) {
> ret = update_qgroup_status_item(trans);
> if (ret < 0) {
> @@ -3332,13 +3332,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
>
> if (!init_flags) {
> /* we're resuming qgroup rescan at mount time */
> - if (!(fs_info->qgroup_flags &
> - BTRFS_QGROUP_STATUS_FLAG_RESCAN)) {
> + if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> btrfs_warn(fs_info,
> "qgroup rescan init failed, qgroup rescan is not queued");
> ret = -EINVAL;
> - } else if (!(fs_info->qgroup_flags &
> - BTRFS_QGROUP_STATUS_FLAG_ON)) {
> + } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
> btrfs_warn(fs_info,
> "qgroup rescan init failed, qgroup is not enabled");
> ret = -EINVAL;
> @@ -3351,12 +3349,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
> mutex_lock(&fs_info->qgroup_rescan_lock);
>
> if (init_flags) {
> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
> btrfs_warn(fs_info,
> "qgroup rescan is already in progress");
> ret = -EINPROGRESS;
> - } else if (!(fs_info->qgroup_flags &
> - BTRFS_QGROUP_STATUS_FLAG_ON)) {
> + } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
> btrfs_warn(fs_info,
> "qgroup rescan init failed, qgroup is not enabled");
> ret = -EINVAL;
> @@ -3366,7 +3363,7 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
> mutex_unlock(&fs_info->qgroup_rescan_lock);
> return ret;
> }
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> }
>
> memset(&fs_info->qgroup_rescan_progress, 0,
> @@ -3422,12 +3419,12 @@ btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info)
>
> trans = btrfs_join_transaction(fs_info->fs_root);
> if (IS_ERR(trans)) {
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> return PTR_ERR(trans);
> }
> ret = btrfs_commit_transaction(trans);
> if (ret) {
> - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> return ret;
> }
>
> @@ -3471,7 +3468,7 @@ int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info,
> void
> btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info)
> {
> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> mutex_lock(&fs_info->qgroup_rescan_lock);
> fs_info->qgroup_rescan_running = true;
> btrfs_queue_work(fs_info->qgroup_rescan_workers,
> @@ -4167,8 +4164,7 @@ int btrfs_qgroup_add_swapped_blocks(struct btrfs_trans_handle *trans,
> spin_unlock(&blocks->lock);
> out:
> if (ret < 0)
> - fs_info->qgroup_flags |=
> - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> return ret;
> }
>
> @@ -4255,7 +4251,7 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
> btrfs_err_rl(fs_info,
> "failed to account subtree at bytenr %llu: %d",
> subvol_eb->start, ret);
> - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> }
> return ret;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
2022-02-01 13:06 ` Qu Wenruo
@ 2022-02-01 13:22 ` Sidong Yang
0 siblings, 0 replies; 8+ messages in thread
From: Sidong Yang @ 2022-02-01 13:22 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Feb 01, 2022 at 09:06:18PM +0800, Qu Wenruo wrote:
>
>
> On 2022/2/1 20:53, Sidong Yang wrote:
> > This patch replaces the code modifying or checking qgroup_flags to
> > bitops like set_bit or test_bit. These bit operations works atomically
> > that it protects qgroup_flags value.
> >
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> > fs/btrfs/ioctl.c | 2 +-
> > fs/btrfs/qgroup.c | 68 ++++++++++++++++++++++-------------------------
>
> Oh, no btrfs_tree.h nor qgroup.h, I already know what the typical
> pitfall you fell into...
Sorry, I realized this error.
>
> > 2 files changed, 33 insertions(+), 37 deletions(-)
> >
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index cc61813213d8..19b0b59abe77 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -4435,7 +4435,7 @@ static long btrfs_ioctl_quota_rescan_status(struct btrfs_fs_info *fs_info,
> > if (!capable(CAP_SYS_ADMIN))
> > return -EPERM;
> >
> > - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> > + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
>
> set/clear/test_bit() all use bit number, not the flag value...
>
> It will still work and pass tests, as instead of
> setting/clearing/testing the first, the second or the third bit, now
> it's testing the 2nd, 4th and 8th bit.
>
> It's not triggering any obvious bug because there are only 3 bits used.
>
> So I'm afraid you need to first convert those flag values into bit
> number first, which would either show up in btrfs_tree.h or internally
> defined in qgroup.h.
Thanks for detailed guide. I would write a new patch for this.
Thanks,
Sidong
>
> Thanks,
> Qu
>
> > qsa.flags = 1;
> > qsa.progress = fs_info->qgroup_rescan_progress.objectid;
> > }
> > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > index db680f5be745..9fabc62ff2a5 100644
> > --- a/fs/btrfs/qgroup.c
> > +++ b/fs/btrfs/qgroup.c
> > @@ -499,16 +499,16 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
> > out:
> > btrfs_free_path(path);
> > fs_info->qgroup_flags |= flags;
> > - if (!(fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_ON))
> > + if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags))
> > clear_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
> > - else if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN &&
> > + else if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags) &&
> > ret >= 0)
> > ret = qgroup_rescan_init(fs_info, rescan_progress, 0);
> >
> > if (ret < 0) {
> > ulist_free(fs_info->qgroup_ulist);
> > fs_info->qgroup_ulist = NULL;
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> > btrfs_sysfs_del_qgroups(fs_info);
> > }
> >
> > @@ -1197,7 +1197,7 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
> > spin_lock(&fs_info->qgroup_lock);
> > quota_root = fs_info->quota_root;
> > fs_info->quota_root = NULL;
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> > spin_unlock(&fs_info->qgroup_lock);
> >
> > btrfs_free_qgroup_config(fs_info);
> > @@ -1353,7 +1353,7 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
> > }
> > out:
> > if (ret)
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > return ret;
> > }
> >
> > @@ -1659,7 +1659,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
> >
> > ret = update_qgroup_limit_item(trans, qgroup);
> > if (ret) {
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > btrfs_info(fs_info, "unable to update quota limit for %llu",
> > qgroupid);
> > }
> > @@ -1735,7 +1735,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
> > ret = btrfs_find_all_roots(NULL, trans->fs_info, bytenr, 0, &old_root,
> > true);
> > if (ret < 0) {
> > - trans->fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &trans->fs_info->qgroup_flags);
> > btrfs_warn(trans->fs_info,
> > "error accounting new delayed refs extent (err code: %d), quota inconsistent",
> > ret);
> > @@ -2211,7 +2211,7 @@ static int qgroup_trace_subtree_swap(struct btrfs_trans_handle *trans,
> > out:
> > btrfs_free_path(dst_path);
> > if (ret < 0)
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > return ret;
> > }
> >
> > @@ -2581,7 +2581,7 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> > }
> >
> > mutex_lock(&fs_info->qgroup_rescan_lock);
> > - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> > + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> > if (fs_info->qgroup_rescan_progress.objectid <= bytenr) {
> > mutex_unlock(&fs_info->qgroup_rescan_lock);
> > ret = 0;
> > @@ -2715,23 +2715,23 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
> > spin_unlock(&fs_info->qgroup_lock);
> > ret = update_qgroup_info_item(trans, qgroup);
> > if (ret)
> > - fs_info->qgroup_flags |=
> > - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
> > + &fs_info->qgroup_flags);
> > ret = update_qgroup_limit_item(trans, qgroup);
> > if (ret)
> > - fs_info->qgroup_flags |=
> > - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT,
> > + &fs_info->qgroup_flags);
> > spin_lock(&fs_info->qgroup_lock);
> > }
> > if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_ON;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> > else
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_ON;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags);
> > spin_unlock(&fs_info->qgroup_lock);
> >
> > ret = update_qgroup_status_item(trans);
> > if (ret)
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> >
> > return ret;
> > }
> > @@ -2849,7 +2849,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
> >
> > ret = update_qgroup_limit_item(trans, dstgroup);
> > if (ret) {
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > btrfs_info(fs_info,
> > "unable to update quota limit for %llu",
> > dstgroup->qgroupid);
> > @@ -2955,7 +2955,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
> > if (!committing)
> > mutex_unlock(&fs_info->qgroup_ioctl_lock);
> > if (need_rescan)
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > return ret;
> > }
> >
> > @@ -3270,10 +3270,10 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
> >
> > mutex_lock(&fs_info->qgroup_rescan_lock);
> > if (err > 0 &&
> > - fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT) {
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + test_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags)) {
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > } else if (err < 0) {
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > }
> > mutex_unlock(&fs_info->qgroup_rescan_lock);
> >
> > @@ -3292,7 +3292,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
> >
> > mutex_lock(&fs_info->qgroup_rescan_lock);
> > if (!stopped)
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> > if (trans) {
> > ret = update_qgroup_status_item(trans);
> > if (ret < 0) {
> > @@ -3332,13 +3332,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
> >
> > if (!init_flags) {
> > /* we're resuming qgroup rescan at mount time */
> > - if (!(fs_info->qgroup_flags &
> > - BTRFS_QGROUP_STATUS_FLAG_RESCAN)) {
> > + if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> > btrfs_warn(fs_info,
> > "qgroup rescan init failed, qgroup rescan is not queued");
> > ret = -EINVAL;
> > - } else if (!(fs_info->qgroup_flags &
> > - BTRFS_QGROUP_STATUS_FLAG_ON)) {
> > + } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
> > btrfs_warn(fs_info,
> > "qgroup rescan init failed, qgroup is not enabled");
> > ret = -EINVAL;
> > @@ -3351,12 +3349,11 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
> > mutex_lock(&fs_info->qgroup_rescan_lock);
> >
> > if (init_flags) {
> > - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> > + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
> > btrfs_warn(fs_info,
> > "qgroup rescan is already in progress");
> > ret = -EINPROGRESS;
> > - } else if (!(fs_info->qgroup_flags &
> > - BTRFS_QGROUP_STATUS_FLAG_ON)) {
> > + } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
> > btrfs_warn(fs_info,
> > "qgroup rescan init failed, qgroup is not enabled");
> > ret = -EINVAL;
> > @@ -3366,7 +3363,7 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
> > mutex_unlock(&fs_info->qgroup_rescan_lock);
> > return ret;
> > }
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> > }
> >
> > memset(&fs_info->qgroup_rescan_progress, 0,
> > @@ -3422,12 +3419,12 @@ btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info)
> >
> > trans = btrfs_join_transaction(fs_info->fs_root);
> > if (IS_ERR(trans)) {
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> > return PTR_ERR(trans);
> > }
> > ret = btrfs_commit_transaction(trans);
> > if (ret) {
> > - fs_info->qgroup_flags &= ~BTRFS_QGROUP_STATUS_FLAG_RESCAN;
> > + clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
> > return ret;
> > }
> >
> > @@ -3471,7 +3468,7 @@ int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info,
> > void
> > btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info)
> > {
> > - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> > + if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
> > mutex_lock(&fs_info->qgroup_rescan_lock);
> > fs_info->qgroup_rescan_running = true;
> > btrfs_queue_work(fs_info->qgroup_rescan_workers,
> > @@ -4167,8 +4164,7 @@ int btrfs_qgroup_add_swapped_blocks(struct btrfs_trans_handle *trans,
> > spin_unlock(&blocks->lock);
> > out:
> > if (ret < 0)
> > - fs_info->qgroup_flags |=
> > - BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > return ret;
> > }
> >
> > @@ -4255,7 +4251,7 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
> > btrfs_err_rl(fs_info,
> > "failed to account subtree at bytenr %llu: %d",
> > subvol_eb->start, ret);
> > - fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> > + set_bit(BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT, &fs_info->qgroup_flags);
> > }
> > return ret;
> > }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
2022-02-01 12:53 [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops Sidong Yang
@ 2022-02-01 13:56 ` kernel test robot
2022-02-01 13:56 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-01 13:56 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs, quwenruo.btrfs; +Cc: kbuild-all, Sidong Yang
Hi Sidong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc2]
[cannot apply to kdave/for-next next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202012137.VlmV1OLt-lkp@intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
git checkout ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:26:61: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
26 | static inline void set_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3318:60: error: passing argument 2 of 'clear_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3318 | clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:39:63: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
39 | static inline void clear_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c: In function 'qgroup_rescan_init':
fs/btrfs/qgroup.c:3358:64: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3358 | if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3362:67: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3362 | } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> fs/btrfs/qgroup.c:3375:70: warning: passing argument 2 of 'arch_test_bit' makes pointer from integer without a cast [-Wint-conversion]
3375 | if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
| ~~~~~~~^~~~~~~~~~~~~~
| |
| u64 {aka long long unsigned int}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64' {aka 'long long unsigned int'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3379:67: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3379 | } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3389:58: error: passing argument 2 of 'set_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3389 | set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:26:61: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
26 | static inline void set_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c: In function 'btrfs_qgroup_rescan':
fs/btrfs/qgroup.c:3445:60: error: passing argument 2 of 'clear_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3445 | clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
vim +/arch_test_bit +3375 fs/btrfs/qgroup.c
3345
3346 /*
3347 * Checks that (a) no rescan is running and (b) quota is enabled. Allocates all
3348 * memory required for the rescan context.
3349 */
3350 static int
3351 qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
3352 int init_flags)
3353 {
3354 int ret = 0;
3355
3356 if (!init_flags) {
3357 /* we're resuming qgroup rescan at mount time */
3358 if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
3359 btrfs_warn(fs_info,
3360 "qgroup rescan init failed, qgroup rescan is not queued");
3361 ret = -EINVAL;
3362 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3363 btrfs_warn(fs_info,
3364 "qgroup rescan init failed, qgroup is not enabled");
3365 ret = -EINVAL;
3366 }
3367
3368 if (ret)
3369 return ret;
3370 }
3371
3372 mutex_lock(&fs_info->qgroup_rescan_lock);
3373
3374 if (init_flags) {
> 3375 if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
3376 btrfs_warn(fs_info,
3377 "qgroup rescan is already in progress");
3378 ret = -EINPROGRESS;
3379 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3380 btrfs_warn(fs_info,
3381 "qgroup rescan init failed, qgroup is not enabled");
3382 ret = -EINVAL;
3383 }
3384
3385 if (ret) {
3386 mutex_unlock(&fs_info->qgroup_rescan_lock);
3387 return ret;
3388 }
3389 set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
3390 }
3391
3392 memset(&fs_info->qgroup_rescan_progress, 0,
3393 sizeof(fs_info->qgroup_rescan_progress));
3394 fs_info->qgroup_rescan_progress.objectid = progress_objectid;
3395 init_completion(&fs_info->qgroup_rescan_completion);
3396 mutex_unlock(&fs_info->qgroup_rescan_lock);
3397
3398 btrfs_init_work(&fs_info->qgroup_rescan_work,
3399 btrfs_qgroup_rescan_worker, NULL, NULL);
3400 return 0;
3401 }
3402
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
@ 2022-02-01 13:56 ` kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-01 13:56 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 18091 bytes --]
Hi Sidong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc2]
[cannot apply to kdave/for-next next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202012137.VlmV1OLt-lkp(a)intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
git checkout ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:26:61: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
26 | static inline void set_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3318:60: error: passing argument 2 of 'clear_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3318 | clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:39:63: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
39 | static inline void clear_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c: In function 'qgroup_rescan_init':
fs/btrfs/qgroup.c:3358:64: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3358 | if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3362:67: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3362 | } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>> fs/btrfs/qgroup.c:3375:70: warning: passing argument 2 of 'arch_test_bit' makes pointer from integer without a cast [-Wint-conversion]
3375 | if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
| ~~~~~~~^~~~~~~~~~~~~~
| |
| u64 {aka long long unsigned int}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64' {aka 'long long unsigned int'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3379:67: error: passing argument 2 of 'arch_test_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3379 | } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops.h:34,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/non-atomic.h:116:62: note: expected 'const volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
116 | arch_test_bit(unsigned int nr, const volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c:3389:58: error: passing argument 2 of 'set_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3389 | set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
from ./arch/nds32/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from fs/btrfs/qgroup.c:6:
include/asm-generic/bitops/instrumented-atomic.h:26:61: note: expected 'volatile long unsigned int *' but argument is of type 'u64 *' {aka 'long long unsigned int *'}
26 | static inline void set_bit(long nr, volatile unsigned long *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
fs/btrfs/qgroup.c: In function 'btrfs_qgroup_rescan':
fs/btrfs/qgroup.c:3445:60: error: passing argument 2 of 'clear_bit' from incompatible pointer type [-Werror=incompatible-pointer-types]
3445 | clear_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
| ^~~~~~~~~~~~~~~~~~~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from include/asm-generic/bitops/atomic.h:74,
from include/asm-generic/bitops.h:33,
from ./arch/nds32/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:33,
from include/linux/log2.h:12,
from include/asm-generic/div64.h:55,
from ./arch/nds32/include/generated/asm/div64.h:1,
from include/linux/math.h:5,
from include/linux/math64.h:6,
from include/linux/time64.h:5,
from include/linux/restart_block.h:10,
from include/linux/thread_info.h:14,
from include/asm-generic/preempt.h:5,
vim +/arch_test_bit +3375 fs/btrfs/qgroup.c
3345
3346 /*
3347 * Checks that (a) no rescan is running and (b) quota is enabled. Allocates all
3348 * memory required for the rescan context.
3349 */
3350 static int
3351 qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
3352 int init_flags)
3353 {
3354 int ret = 0;
3355
3356 if (!init_flags) {
3357 /* we're resuming qgroup rescan at mount time */
3358 if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
3359 btrfs_warn(fs_info,
3360 "qgroup rescan init failed, qgroup rescan is not queued");
3361 ret = -EINVAL;
3362 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3363 btrfs_warn(fs_info,
3364 "qgroup rescan init failed, qgroup is not enabled");
3365 ret = -EINVAL;
3366 }
3367
3368 if (ret)
3369 return ret;
3370 }
3371
3372 mutex_lock(&fs_info->qgroup_rescan_lock);
3373
3374 if (init_flags) {
> 3375 if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
3376 btrfs_warn(fs_info,
3377 "qgroup rescan is already in progress");
3378 ret = -EINPROGRESS;
3379 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3380 btrfs_warn(fs_info,
3381 "qgroup rescan init failed, qgroup is not enabled");
3382 ret = -EINVAL;
3383 }
3384
3385 if (ret) {
3386 mutex_unlock(&fs_info->qgroup_rescan_lock);
3387 return ret;
3388 }
3389 set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
3390 }
3391
3392 memset(&fs_info->qgroup_rescan_progress, 0,
3393 sizeof(fs_info->qgroup_rescan_progress));
3394 fs_info->qgroup_rescan_progress.objectid = progress_objectid;
3395 init_completion(&fs_info->qgroup_rescan_completion);
3396 mutex_unlock(&fs_info->qgroup_rescan_lock);
3397
3398 btrfs_init_work(&fs_info->qgroup_rescan_work,
3399 btrfs_qgroup_rescan_worker, NULL, NULL);
3400 return 0;
3401 }
3402
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
2022-02-01 12:53 [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops Sidong Yang
2022-02-01 13:06 ` Qu Wenruo
2022-02-01 13:56 ` kernel test robot
@ 2022-02-01 14:02 ` David Sterba
2022-02-01 14:07 ` kernel test robot
3 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2022-02-01 14:02 UTC (permalink / raw)
To: Sidong Yang; +Cc: linux-btrfs, quwenruo.btrfs
On Tue, Feb 01, 2022 at 12:53:31PM +0000, Sidong Yang wrote:
> This patch replaces the code modifying or checking qgroup_flags to
> bitops like set_bit or test_bit. These bit operations works atomically
> that it protects qgroup_flags value.
Also please explain if this is fixing a problem or what's the reason for
the change (cleanup, preparatory work, ...).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
2022-02-01 12:53 [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops Sidong Yang
@ 2022-02-01 14:07 ` kernel test robot
2022-02-01 13:56 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-01 14:07 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs, quwenruo.btrfs; +Cc: kbuild-all, Sidong Yang
Hi Sidong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc2]
[cannot apply to kdave/for-next next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202012156.wyQPJZyD-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
git checkout ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/btrfs/qgroup.c: In function 'qgroup_rescan_init':
>> fs/btrfs/qgroup.c:3375:70: warning: passing argument 2 of 'test_bit' makes pointer from integer without a cast [-Wint-conversion]
3375 | if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
| ~~~~~~~^~~~~~~~~~~~~~
| |
| u64 {aka long long unsigned int}
In file included from include/linux/bitops.h:33,
from include/linux/thread_info.h:27,
from include/asm-generic/current.h:5,
from ./arch/alpha/include/generated/asm/current.h:1,
from include/linux/sched.h:12,
from fs/btrfs/qgroup.c:6:
arch/alpha/include/asm/bitops.h:287:40: note: expected 'const volatile void *' but argument is of type 'u64' {aka 'long long unsigned int'}
287 | test_bit(int nr, const volatile void * addr)
| ~~~~~~~~~~~~~~~~~~~~~~^~~~
vim +/test_bit +3375 fs/btrfs/qgroup.c
3345
3346 /*
3347 * Checks that (a) no rescan is running and (b) quota is enabled. Allocates all
3348 * memory required for the rescan context.
3349 */
3350 static int
3351 qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
3352 int init_flags)
3353 {
3354 int ret = 0;
3355
3356 if (!init_flags) {
3357 /* we're resuming qgroup rescan at mount time */
3358 if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
3359 btrfs_warn(fs_info,
3360 "qgroup rescan init failed, qgroup rescan is not queued");
3361 ret = -EINVAL;
3362 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3363 btrfs_warn(fs_info,
3364 "qgroup rescan init failed, qgroup is not enabled");
3365 ret = -EINVAL;
3366 }
3367
3368 if (ret)
3369 return ret;
3370 }
3371
3372 mutex_lock(&fs_info->qgroup_rescan_lock);
3373
3374 if (init_flags) {
> 3375 if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
3376 btrfs_warn(fs_info,
3377 "qgroup rescan is already in progress");
3378 ret = -EINPROGRESS;
3379 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3380 btrfs_warn(fs_info,
3381 "qgroup rescan init failed, qgroup is not enabled");
3382 ret = -EINVAL;
3383 }
3384
3385 if (ret) {
3386 mutex_unlock(&fs_info->qgroup_rescan_lock);
3387 return ret;
3388 }
3389 set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
3390 }
3391
3392 memset(&fs_info->qgroup_rescan_progress, 0,
3393 sizeof(fs_info->qgroup_rescan_progress));
3394 fs_info->qgroup_rescan_progress.objectid = progress_objectid;
3395 init_completion(&fs_info->qgroup_rescan_completion);
3396 mutex_unlock(&fs_info->qgroup_rescan_lock);
3397
3398 btrfs_init_work(&fs_info->qgroup_rescan_work,
3399 btrfs_qgroup_rescan_worker, NULL, NULL);
3400 return 0;
3401 }
3402
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops
@ 2022-02-01 14:07 ` kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-02-01 14:07 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5018 bytes --]
Hi Sidong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.17-rc2]
[cannot apply to kdave/for-next next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202012156.wyQPJZyD-lkp(a)intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sidong-Yang/btrfs-qgroup-replace-modifing-qgroup_flags-to-bitops/20220201-205452
git checkout ba6f4f3d431a14034f4e4da2b8f342fe17ec78bf
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/btrfs/qgroup.c: In function 'qgroup_rescan_init':
>> fs/btrfs/qgroup.c:3375:70: warning: passing argument 2 of 'test_bit' makes pointer from integer without a cast [-Wint-conversion]
3375 | if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
| ~~~~~~~^~~~~~~~~~~~~~
| |
| u64 {aka long long unsigned int}
In file included from include/linux/bitops.h:33,
from include/linux/thread_info.h:27,
from include/asm-generic/current.h:5,
from ./arch/alpha/include/generated/asm/current.h:1,
from include/linux/sched.h:12,
from fs/btrfs/qgroup.c:6:
arch/alpha/include/asm/bitops.h:287:40: note: expected 'const volatile void *' but argument is of type 'u64' {aka 'long long unsigned int'}
287 | test_bit(int nr, const volatile void * addr)
| ~~~~~~~~~~~~~~~~~~~~~~^~~~
vim +/test_bit +3375 fs/btrfs/qgroup.c
3345
3346 /*
3347 * Checks that (a) no rescan is running and (b) quota is enabled. Allocates all
3348 * memory required for the rescan context.
3349 */
3350 static int
3351 qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
3352 int init_flags)
3353 {
3354 int ret = 0;
3355
3356 if (!init_flags) {
3357 /* we're resuming qgroup rescan at mount time */
3358 if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags)) {
3359 btrfs_warn(fs_info,
3360 "qgroup rescan init failed, qgroup rescan is not queued");
3361 ret = -EINVAL;
3362 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3363 btrfs_warn(fs_info,
3364 "qgroup rescan init failed, qgroup is not enabled");
3365 ret = -EINVAL;
3366 }
3367
3368 if (ret)
3369 return ret;
3370 }
3371
3372 mutex_lock(&fs_info->qgroup_rescan_lock);
3373
3374 if (init_flags) {
> 3375 if (test_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, fs_info->qgroup_flags)) {
3376 btrfs_warn(fs_info,
3377 "qgroup rescan is already in progress");
3378 ret = -EINPROGRESS;
3379 } else if (!test_bit(BTRFS_QGROUP_STATUS_FLAG_ON, &fs_info->qgroup_flags)) {
3380 btrfs_warn(fs_info,
3381 "qgroup rescan init failed, qgroup is not enabled");
3382 ret = -EINVAL;
3383 }
3384
3385 if (ret) {
3386 mutex_unlock(&fs_info->qgroup_rescan_lock);
3387 return ret;
3388 }
3389 set_bit(BTRFS_QGROUP_STATUS_FLAG_RESCAN, &fs_info->qgroup_flags);
3390 }
3391
3392 memset(&fs_info->qgroup_rescan_progress, 0,
3393 sizeof(fs_info->qgroup_rescan_progress));
3394 fs_info->qgroup_rescan_progress.objectid = progress_objectid;
3395 init_completion(&fs_info->qgroup_rescan_completion);
3396 mutex_unlock(&fs_info->qgroup_rescan_lock);
3397
3398 btrfs_init_work(&fs_info->qgroup_rescan_work,
3399 btrfs_qgroup_rescan_worker, NULL, NULL);
3400 return 0;
3401 }
3402
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-01 14:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 12:53 [PATCH] btrfs: qgroup: replace modifing qgroup_flags to bitops Sidong Yang
2022-02-01 13:06 ` Qu Wenruo
2022-02-01 13:22 ` Sidong Yang
2022-02-01 13:56 ` kernel test robot
2022-02-01 13:56 ` kernel test robot
2022-02-01 14:02 ` David Sterba
2022-02-01 14:07 ` kernel test robot
2022-02-01 14:07 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.