All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan
@ 2018-08-09  7:08 Qu Wenruo
  2018-08-09  7:36 ` Misono Tomohiro
  2018-08-09  9:17 ` Filipe Manana
  0 siblings, 2 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-08-09  7:08 UTC (permalink / raw
  To: linux-btrfs

[BUG]
In the following case, rescan won't zero out the number of qgroup 1/0:
------
$ mkfs.btrfs -fq $DEV
$ mount $DEV /mnt

$ btrfs quota enable /mnt
$ btrfs qgroup create 1/0 /mnt
$ btrfs sub create /mnt/sub
$ btrfs qgroup assign 0/257 1/0 /mnt

$ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
$ btrfs sub snap /mnt/sub /mnt/snap
$ btrfs quota rescan -w /mnt
$ btrfs qgroup show -pcre /mnt
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          16.00KiB     16.00KiB         none         none ---     ---
0/257      1016.00KiB     16.00KiB         none         none 1/0     ---
0/258      1016.00KiB     16.00KiB         none         none ---     ---
1/0        1016.00KiB     16.00KiB         none         none ---     0/257

so far so good, but:

$ btrfs qgroup remove 0/257 1/0 /mnt
WARNING: quotas may be inconsistent, rescan needed
$ btrfs quota rescan -w /mnt
$ btrfs qgroup show -pcre  /mnt
qgoupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          16.00KiB     16.00KiB         none         none ---     ---
0/257      1016.00KiB     16.00KiB         none         none ---     ---
0/258      1016.00KiB     16.00KiB         none         none ---     ---
1/0        1016.00KiB     16.00KiB         none         none ---     ---
           ^^^^^^^^^^     ^^^^^^^^ not cleared
------

[CAUSE]
Before rescan we call qgroup_rescan_zero_tracking() to zero out all
qgroups' accounting numbers.

However we don't mark all qgroups dirty, but rely on rescan to mark
qgroups dirty.

If we have any high level qgroup but without any child (orphan group), it
won't be marked dirty during rescan, since we can not reach that qgroup.

This will cause QGROUP_INFO items of orphan qgroups never get updated in
quota tree, thus their numbers will stay the same in "btrfs qgroup show"
output.

[FIX]
Just mark all qgroups dirty in qgroup_rescan_zero_tracking(), so even we
have orphan qgroups their QGROUP_INFO items will still get updated during
rescan.

Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
  Fix some grammar errors in commit message.
---
 fs/btrfs/qgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 48c1c3e7baf3..5a5372b33d96 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2864,6 +2864,7 @@ qgroup_rescan_zero_tracking(struct btrfs_fs_info *fs_info)
 		qgroup->rfer_cmpr = 0;
 		qgroup->excl = 0;
 		qgroup->excl_cmpr = 0;
+		qgroup_dirty(fs_info, qgroup);
 	}
 	spin_unlock(&fs_info->qgroup_lock);
 }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan
  2018-08-09  7:08 [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan Qu Wenruo
@ 2018-08-09  7:36 ` Misono Tomohiro
  2018-08-09  9:17 ` Filipe Manana
  1 sibling, 0 replies; 4+ messages in thread
From: Misono Tomohiro @ 2018-08-09  7:36 UTC (permalink / raw
  To: Qu Wenruo, linux-btrfs

On 2018/08/09 16:08, Qu Wenruo wrote:
> [BUG]
> In the following case, rescan won't zero out the number of qgroup 1/0:
> ------
> $ mkfs.btrfs -fq $DEV
> $ mount $DEV /mnt
> 
> $ btrfs quota enable /mnt
> $ btrfs qgroup create 1/0 /mnt
> $ btrfs sub create /mnt/sub
> $ btrfs qgroup assign 0/257 1/0 /mnt
> 
> $ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
> $ btrfs sub snap /mnt/sub /mnt/snap
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre /mnt
> qgroupid         rfer         excl     max_rfer     max_excl parent  child
> --------         ----         ----     --------     -------- ------  -----
> 0/5          16.00KiB     16.00KiB         none         none ---     ---
> 0/257      1016.00KiB     16.00KiB         none         none 1/0     ---
> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
> 1/0        1016.00KiB     16.00KiB         none         none ---     0/257
> 
> so far so good, but:
> 
> $ btrfs qgroup remove 0/257 1/0 /mnt
> WARNING: quotas may be inconsistent, rescan needed
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre  /mnt
> qgoupid         rfer         excl     max_rfer     max_excl parent  child
> --------         ----         ----     --------     -------- ------  -----
> 0/5          16.00KiB     16.00KiB         none         none ---     ---
> 0/257      1016.00KiB     16.00KiB         none         none ---     ---
> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
> 1/0        1016.00KiB     16.00KiB         none         none ---     ---
>            ^^^^^^^^^^     ^^^^^^^^ not cleared
> ------
> 
> [CAUSE]
> Before rescan we call qgroup_rescan_zero_tracking() to zero out all
> qgroups' accounting numbers.
> 
> However we don't mark all qgroups dirty, but rely on rescan to mark
> qgroups dirty.
> 
> If we have any high level qgroup but without any child (orphan group), it
> won't be marked dirty during rescan, since we can not reach that qgroup.
> 
> This will cause QGROUP_INFO items of orphan qgroups never get updated in
> quota tree, thus their numbers will stay the same in "btrfs qgroup show"
> output.
> 
> [FIX]
> Just mark all qgroups dirty in qgroup_rescan_zero_tracking(), so even we
> have orphan qgroups their QGROUP_INFO items will still get updated during
> rescan.
> 
> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
>   Fix some grammar errors in commit message.
> ---
>  fs/btrfs/qgroup.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 48c1c3e7baf3..5a5372b33d96 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2864,6 +2864,7 @@ qgroup_rescan_zero_tracking(struct btrfs_fs_info *fs_info)
>  		qgroup->rfer_cmpr = 0;
>  		qgroup->excl = 0;
>  		qgroup->excl_cmpr = 0;
> +		qgroup_dirty(fs_info, qgroup);
>  	}
>  	spin_unlock(&fs_info->qgroup_lock);
>  }
> 
Yes, this and previous patch
  [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups
resolves the problem I see.

So, this will reset all the qgroup items not rescanned in first transaction commit
by btrfs_run_qgroups(), but I don't think it is a problem.

Tested-by/Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Thanks,
Misono


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan
  2018-08-09  7:08 [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan Qu Wenruo
  2018-08-09  7:36 ` Misono Tomohiro
@ 2018-08-09  9:17 ` Filipe Manana
  2018-08-09 11:24   ` Qu Wenruo
  1 sibling, 1 reply; 4+ messages in thread
From: Filipe Manana @ 2018-08-09  9:17 UTC (permalink / raw
  To: Qu Wenruo; +Cc: linux-btrfs

On Thu, Aug 9, 2018 at 8:08 AM, Qu Wenruo <wqu@suse.com> wrote:
> [BUG]
> In the following case, rescan won't zero out the number of qgroup 1/0:
> ------
> $ mkfs.btrfs -fq $DEV
> $ mount $DEV /mnt
>
> $ btrfs quota enable /mnt
> $ btrfs qgroup create 1/0 /mnt
> $ btrfs sub create /mnt/sub
> $ btrfs qgroup assign 0/257 1/0 /mnt
>
> $ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
> $ btrfs sub snap /mnt/sub /mnt/snap
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre /mnt
> qgroupid         rfer         excl     max_rfer     max_excl parent  child
> --------         ----         ----     --------     -------- ------  -----
> 0/5          16.00KiB     16.00KiB         none         none ---     ---
> 0/257      1016.00KiB     16.00KiB         none         none 1/0     ---
> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
> 1/0        1016.00KiB     16.00KiB         none         none ---     0/257
>
> so far so good, but:
>
> $ btrfs qgroup remove 0/257 1/0 /mnt
> WARNING: quotas may be inconsistent, rescan needed
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre  /mnt
> qgoupid         rfer         excl     max_rfer     max_excl parent  child
> --------         ----         ----     --------     -------- ------  -----
> 0/5          16.00KiB     16.00KiB         none         none ---     ---
> 0/257      1016.00KiB     16.00KiB         none         none ---     ---
> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
> 1/0        1016.00KiB     16.00KiB         none         none ---     ---
>            ^^^^^^^^^^     ^^^^^^^^ not cleared
> ------
>
> [CAUSE]
> Before rescan we call qgroup_rescan_zero_tracking() to zero out all
> qgroups' accounting numbers.
>
> However we don't mark all qgroups dirty, but rely on rescan to mark
> qgroups dirty.
>
> If we have any high level qgroup but without any child (orphan group),

That sentence is confusing. An orphan, by definition [1], is someone
(or something in this case) without parents.
But you mention a group without children, so that should be named
"childless" or simply say "without children".
So one part of the sentence is wrong, either what is in parenthesis or
what comes before them.

[1] https://www.thefreedictionary.com/orphan

> it
> won't be marked dirty during rescan, since we can not reach that qgroup.
>
> This will cause QGROUP_INFO items of orphan qgroups never get updated in
> quota tree, thus their numbers will stay the same in "btrfs qgroup show"
> output.
>
> [FIX]
> Just mark all qgroups dirty in qgroup_rescan_zero_tracking(), so even we
> have orphan qgroups their QGROUP_INFO items will still get updated during
> rescan.
>
> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
>   Fix some grammar errors in commit message.
> ---
>  fs/btrfs/qgroup.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 48c1c3e7baf3..5a5372b33d96 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2864,6 +2864,7 @@ qgroup_rescan_zero_tracking(struct btrfs_fs_info *fs_info)
>                 qgroup->rfer_cmpr = 0;
>                 qgroup->excl = 0;
>                 qgroup->excl_cmpr = 0;
> +               qgroup_dirty(fs_info, qgroup);
>         }
>         spin_unlock(&fs_info->qgroup_lock);
>  }
> --
> 2.18.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan
  2018-08-09  9:17 ` Filipe Manana
@ 2018-08-09 11:24   ` Qu Wenruo
  0 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-08-09 11:24 UTC (permalink / raw
  To: fdmanana, Qu Wenruo; +Cc: linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 3975 bytes --]



On 8/9/18 5:17 PM, Filipe Manana wrote:
> On Thu, Aug 9, 2018 at 8:08 AM, Qu Wenruo <wqu@suse.com> wrote:
>> [BUG]
>> In the following case, rescan won't zero out the number of qgroup 1/0:
>> ------
>> $ mkfs.btrfs -fq $DEV
>> $ mount $DEV /mnt
>>
>> $ btrfs quota enable /mnt
>> $ btrfs qgroup create 1/0 /mnt
>> $ btrfs sub create /mnt/sub
>> $ btrfs qgroup assign 0/257 1/0 /mnt
>>
>> $ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
>> $ btrfs sub snap /mnt/sub /mnt/snap
>> $ btrfs quota rescan -w /mnt
>> $ btrfs qgroup show -pcre /mnt
>> qgroupid         rfer         excl     max_rfer     max_excl parent  child
>> --------         ----         ----     --------     -------- ------  -----
>> 0/5          16.00KiB     16.00KiB         none         none ---     ---
>> 0/257      1016.00KiB     16.00KiB         none         none 1/0     ---
>> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
>> 1/0        1016.00KiB     16.00KiB         none         none ---     0/257
>>
>> so far so good, but:
>>
>> $ btrfs qgroup remove 0/257 1/0 /mnt
>> WARNING: quotas may be inconsistent, rescan needed
>> $ btrfs quota rescan -w /mnt
>> $ btrfs qgroup show -pcre  /mnt
>> qgoupid         rfer         excl     max_rfer     max_excl parent  child
>> --------         ----         ----     --------     -------- ------  -----
>> 0/5          16.00KiB     16.00KiB         none         none ---     ---
>> 0/257      1016.00KiB     16.00KiB         none         none ---     ---
>> 0/258      1016.00KiB     16.00KiB         none         none ---     ---
>> 1/0        1016.00KiB     16.00KiB         none         none ---     ---
>>            ^^^^^^^^^^     ^^^^^^^^ not cleared
>> ------
>>
>> [CAUSE]
>> Before rescan we call qgroup_rescan_zero_tracking() to zero out all
>> qgroups' accounting numbers.
>>
>> However we don't mark all qgroups dirty, but rely on rescan to mark
>> qgroups dirty.
>>
>> If we have any high level qgroup but without any child (orphan group),
> 
> That sentence is confusing. An orphan, by definition [1], is someone
> (or something in this case) without parents.
> But you mention a group without children, so that should be named
> "childless" or simply say "without children".

Makes sense, I'll correct the commit message in next version.

Thanks,
Qu

> So one part of the sentence is wrong, either what is in parenthesis or
> what comes before them.
> 
> [1] https://www.thefreedictionary.com/orphan
> 
>> it
>> won't be marked dirty during rescan, since we can not reach that qgroup.
>>
>> This will cause QGROUP_INFO items of orphan qgroups never get updated in
>> quota tree, thus their numbers will stay the same in "btrfs qgroup show"
>> output.
>>
>> [FIX]
>> Just mark all qgroups dirty in qgroup_rescan_zero_tracking(), so even we
>> have orphan qgroups their QGROUP_INFO items will still get updated during
>> rescan.
>>
>> Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> changelog:
>> v2:
>>   Fix some grammar errors in commit message.
>> ---
>>  fs/btrfs/qgroup.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 48c1c3e7baf3..5a5372b33d96 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -2864,6 +2864,7 @@ qgroup_rescan_zero_tracking(struct btrfs_fs_info *fs_info)
>>                 qgroup->rfer_cmpr = 0;
>>                 qgroup->excl = 0;
>>                 qgroup->excl_cmpr = 0;
>> +               qgroup_dirty(fs_info, qgroup);
>>         }
>>         spin_unlock(&fs_info->qgroup_lock);
>>  }
>> --
>> 2.18.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-08-09 13:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-09  7:08 [PATCH v2] btrfs: qgroup: Dirty all qgroups before rescan Qu Wenruo
2018-08-09  7:36 ` Misono Tomohiro
2018-08-09  9:17 ` Filipe Manana
2018-08-09 11:24   ` Qu Wenruo

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.