* [PATCH] Btrfs: save us a mutex_lock usage when doing quota rescan
@ 2013-05-07 6:15 Wang Shilong
2013-05-07 7:04 ` Jan Schmidt
0 siblings, 1 reply; 3+ messages in thread
From: Wang Shilong @ 2013-05-07 6:15 UTC (permalink / raw)
To: Linux Btrfs; +Cc: Jan Schmidt
If qgroup_rescan worker is in progress, we should ignore
the extent that has not been dealt with qgroup_rescan worker,just
let them dealt later otherwise we may get wrong qgroup accounting.
However, we have checked this before find_all_roots() without spin_lock.
When doing qgroup accounting, we don't have to check it again, because
during this period,qgroup_rescan worker can deal with more extents and
qgroup_rescan_extent->objectid can only go larger, so here the check
is unnecessary.
Just remove this check, so that we don't need hold qgroup_rescan_lock
when doing qgroup accounting.
Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
fs/btrfs/qgroup.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index d059d86..2710784 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1445,15 +1445,7 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
if (ret < 0)
return ret;
- mutex_lock(&fs_info->qgroup_rescan_lock);
spin_lock(&fs_info->qgroup_lock);
- if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
- if (fs_info->qgroup_rescan_progress.objectid <= node->bytenr) {
- ret = 0;
- goto unlock;
- }
- }
-
quota_root = fs_info->quota_root;
if (!quota_root)
goto unlock;
@@ -1492,7 +1484,6 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
unlock:
spin_unlock(&fs_info->qgroup_lock);
- mutex_unlock(&fs_info->qgroup_rescan_lock);
ulist_free(roots);
return ret;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Btrfs: save us a mutex_lock usage when doing quota rescan
2013-05-07 6:15 [PATCH] Btrfs: save us a mutex_lock usage when doing quota rescan Wang Shilong
@ 2013-05-07 7:04 ` Jan Schmidt
2013-05-07 7:56 ` Wang Shilong
0 siblings, 1 reply; 3+ messages in thread
From: Jan Schmidt @ 2013-05-07 7:04 UTC (permalink / raw)
To: Wang Shilong; +Cc: Linux Btrfs
On Tue, May 07, 2013 at 08:15 (+0200), Wang Shilong wrote:
> If qgroup_rescan worker is in progress, we should ignore
> the extent that has not been dealt with qgroup_rescan worker,just
> let them dealt later otherwise we may get wrong qgroup accounting.
>
> However, we have checked this before find_all_roots() without spin_lock.
> When doing qgroup accounting, we don't have to check it again, because
> during this period,qgroup_rescan worker can deal with more extents and
> qgroup_rescan_extent->objectid can only go larger, so here the check
> is unnecessary.
>
> Just remove this check, so that we don't need hold qgroup_rescan_lock
> when doing qgroup accounting.
NAK.
After a discussion on that lock the last thing in this thread I see is ...
On Wed, May 01, 2013 at 13:57 (+0200), Jan Schmidt wrote:
> Now I see what you mean. The second check is only required when we start
> a rescan operation after the initial check in btrfs_qgroup_account_ref.
Please continue on that argument, your commit message doesn't explain at all why
we should be safe to remove this check.
-Jan
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> ---
> fs/btrfs/qgroup.c | 9 ---------
> 1 files changed, 0 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index d059d86..2710784 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1445,15 +1445,7 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
> if (ret < 0)
> return ret;
>
> - mutex_lock(&fs_info->qgroup_rescan_lock);
> spin_lock(&fs_info->qgroup_lock);
> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
> - if (fs_info->qgroup_rescan_progress.objectid <= node->bytenr) {
> - ret = 0;
> - goto unlock;
> - }
> - }
> -
> quota_root = fs_info->quota_root;
> if (!quota_root)
> goto unlock;
> @@ -1492,7 +1484,6 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
>
> unlock:
> spin_unlock(&fs_info->qgroup_lock);
> - mutex_unlock(&fs_info->qgroup_rescan_lock);
> ulist_free(roots);
>
> return ret;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Btrfs: save us a mutex_lock usage when doing quota rescan
2013-05-07 7:04 ` Jan Schmidt
@ 2013-05-07 7:56 ` Wang Shilong
0 siblings, 0 replies; 3+ messages in thread
From: Wang Shilong @ 2013-05-07 7:56 UTC (permalink / raw)
To: Jan Schmidt; +Cc: Linux Btrfs
Hello Jan,
> On Tue, May 07, 2013 at 08:15 (+0200), Wang Shilong wrote:
>> If qgroup_rescan worker is in progress, we should ignore
>> the extent that has not been dealt with qgroup_rescan worker,just
>> let them dealt later otherwise we may get wrong qgroup accounting.
>>
>> However, we have checked this before find_all_roots() without spin_lock.
>> When doing qgroup accounting, we don't have to check it again, because
>> during this period,qgroup_rescan worker can deal with more extents and
>> qgroup_rescan_extent->objectid can only go larger, so here the check
>> is unnecessary.
>>
>> Just remove this check, so that we don't need hold qgroup_rescan_lock
>> when doing qgroup accounting.
>
> NAK.
>
> After a discussion on that lock the last thing in this thread I see is ...
>
> On Wed, May 01, 2013 at 13:57 (+0200), Jan Schmidt wrote:
>> Now I see what you mean. The second check is only required when we start
>> a rescan operation after the initial check in btrfs_qgroup_account_ref.
>
> Please continue on that argument, your commit message doesn't explain at all why
> we should be safe to remove this check.
You are right!
Thanks,
Wang
>
> -Jan
>
>> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>> ---
>> fs/btrfs/qgroup.c | 9 ---------
>> 1 files changed, 0 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index d059d86..2710784 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -1445,15 +1445,7 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
>> if (ret < 0)
>> return ret;
>>
>> - mutex_lock(&fs_info->qgroup_rescan_lock);
>> spin_lock(&fs_info->qgroup_lock);
>> - if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) {
>> - if (fs_info->qgroup_rescan_progress.objectid <= node->bytenr) {
>> - ret = 0;
>> - goto unlock;
>> - }
>> - }
>> -
>> quota_root = fs_info->quota_root;
>> if (!quota_root)
>> goto unlock;
>> @@ -1492,7 +1484,6 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans,
>>
>> unlock:
>> spin_unlock(&fs_info->qgroup_lock);
>> - mutex_unlock(&fs_info->qgroup_rescan_lock);
>> ulist_free(roots);
>>
>> return ret;
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-07 8:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-07 6:15 [PATCH] Btrfs: save us a mutex_lock usage when doing quota rescan Wang Shilong
2013-05-07 7:04 ` Jan Schmidt
2013-05-07 7:56 ` Wang Shilong
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.