[PATCH] btrfs: qgroup: Auto kick in rescan if qgroup tree is initialized without rescan initialized

[PATCH] btrfs: qgroup: Auto kick in rescan if qgroup tree is initialized without rescan initialized

[Qu Wenruo]

Under certain case, btrfs/166 could cause power loss just after quota
tree initialized but rescan not kicked in.

In this case, since flags of qgroup status item is just ON |
INCONSISTENT, without RESCAN flag, rescan won't be kicked in in next
mount.

Now kick in rescan automatically for such situation, so user won't need
to do rescan manually.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index c25dc47210a3..e62598fc354f 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -392,6 +392,17 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
 			fs_info->qgroup_flags = btrfs_qgroup_status_flags(l,
 									  ptr);
 			rescan_progress = btrfs_qgroup_status_rescan(l, ptr);
+
+			/*
+			 * Qgroup is enabled but rescan hans't kicked in and
+			 * power loss happened, kick rescan in
+			 */
+			if (rescan_progress == 0 &&
+			    (BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT |
+			     BTRFS_QGROUP_STATUS_FLAG_ON) ==
+			    fs_info->qgroup_flags)
+				fs_info->qgroup_flags |=
+					BTRFS_QGROUP_STATUS_FLAG_RESCAN;
 			goto next1;
 		}
 
-- 
2.18.0

Re: [PATCH] btrfs: qgroup: Auto kick in rescan if qgroup tree is initialized without rescan initialized

[Misono Tomohiro]

On 2018/07/26 15:59, Qu Wenruo wrote:
> Under certain case, btrfs/166 could cause power loss just after quota
> tree initialized but rescan not kicked in.
> 
> In this case, since flags of qgroup status item is just ON |
> INCONSISTENT, without RESCAN flag, rescan won't be kicked in in next
> mount.
> 
> Now kick in rescan automatically for such situation, so user won't need
> to do rescan manually.

How about setting all of BTRFS_QGROUP_STATUS_FLAG_ON/INCONSISTENT/RESCAN
at first place and calling qgroup_rescan_init(fs_info, 0, 0) 
(currently it is (fs_info, 0, 1)) in btrfs_quota_enable()?

I think with this approach, current btrfs-progs can work as expected too.

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/qgroup.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index c25dc47210a3..e62598fc354f 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -392,6 +392,17 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
>  			fs_info->qgroup_flags = btrfs_qgroup_status_flags(l,
>  									  ptr);
>  			rescan_progress = btrfs_qgroup_status_rescan(l, ptr);
> +
> +			/*
> +			 * Qgroup is enabled but rescan hans't kicked in and
> +			 * power loss happened, kick rescan in
> +			 */
> +			if (rescan_progress == 0 &&
> +			    (BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT |
> +			     BTRFS_QGROUP_STATUS_FLAG_ON) ==
> +			    fs_info->qgroup_flags)
> +				fs_info->qgroup_flags |=
> +					BTRFS_QGROUP_STATUS_FLAG_RESCAN;
>  			goto next1;
>  		}
>  
> 

Re: [PATCH] btrfs: qgroup: Auto kick in rescan if qgroup tree is initialized without rescan initialized

[Qu Wenruo]

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



On 2018年07月26日 15:21, Misono Tomohiro wrote:
> On 2018/07/26 15:59, Qu Wenruo wrote:
>> Under certain case, btrfs/166 could cause power loss just after quota
>> tree initialized but rescan not kicked in.
>>
>> In this case, since flags of qgroup status item is just ON |
>> INCONSISTENT, without RESCAN flag, rescan won't be kicked in in next
>> mount.
>>
>> Now kick in rescan automatically for such situation, so user won't need
>> to do rescan manually.
> 
> How about setting all of BTRFS_QGROUP_STATUS_FLAG_ON/INCONSISTENT/RESCAN
> at first place and calling qgroup_rescan_init(fs_info, 0, 0) 
> (currently it is (fs_info, 0, 1)) in btrfs_quota_enable()?

I'm wondering if there will any race window between we set RESCAN flag
and we really kick in rescan.

But the idea looks doable and I'll look into it.

> 
> I think with this approach, current btrfs-progs can work as expected too.

In theory, we still need to fix btrfs check/kernel since we may need to
handle old fs.
But since it's pretty rare, I think if we could fix kernel using the
idea you purposed, it would be enough for this case.

Thanks,
Qu

> 
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/qgroup.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index c25dc47210a3..e62598fc354f 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -392,6 +392,17 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
>>  			fs_info->qgroup_flags = btrfs_qgroup_status_flags(l,
>>  									  ptr);
>>  			rescan_progress = btrfs_qgroup_status_rescan(l, ptr);
>> +
>> +			/*
>> +			 * Qgroup is enabled but rescan hans't kicked in and
>> +			 * power loss happened, kick rescan in
>> +			 */
>> +			if (rescan_progress == 0 &&
>> +			    (BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT |
>> +			     BTRFS_QGROUP_STATUS_FLAG_ON) ==
>> +			    fs_info->qgroup_flags)
>> +				fs_info->qgroup_flags |=
>> +					BTRFS_QGROUP_STATUS_FLAG_RESCAN;
>>  			goto next1;
>>  		}
>>  
>>
> 
> --
> 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 --]

Re: [PATCH] btrfs: qgroup: Auto kick in rescan if qgroup tree is initialized without rescan initialized

[Qu Wenruo]

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



On 2018年07月26日 14:59, Qu Wenruo wrote:
> Under certain case, btrfs/166 could cause power loss just after quota
> tree initialized but rescan not kicked in.
> 
> In this case, since flags of qgroup status item is just ON |
> INCONSISTENT, without RESCAN flag, rescan won't be kicked in in next
> mount.
> 
> Now kick in rescan automatically for such situation, so user won't need
> to do rescan manually.

Please ignore this patch, as just suggested by Misono, it's addressed by
later patch "btrfs: qgroup: Init flags with RESCAN bit at quota enable
time".

Thanks,
Qu

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/qgroup.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index c25dc47210a3..e62598fc354f 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -392,6 +392,17 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
>  			fs_info->qgroup_flags = btrfs_qgroup_status_flags(l,
>  									  ptr);
>  			rescan_progress = btrfs_qgroup_status_rescan(l, ptr);
> +
> +			/*
> +			 * Qgroup is enabled but rescan hans't kicked in and
> +			 * power loss happened, kick rescan in
> +			 */
> +			if (rescan_progress == 0 &&
> +			    (BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT |
> +			     BTRFS_QGROUP_STATUS_FLAG_ON) ==
> +			    fs_info->qgroup_flags)
> +				fs_info->qgroup_flags |=
> +					BTRFS_QGROUP_STATUS_FLAG_RESCAN;
>  			goto next1;
>  		}
>  
> 


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