Re: [PATCH RFC 0/4] btrfs: qgroup: rescan enhancement related to INCONSISTENT flag

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2021/8/24 下午2:54, Qu Wenruo wrote:
>
>
> On 2021/8/24 上午1:30, David Sterba wrote:
>> On Sun, Aug 22, 2021 at 03:01:56PM +0800, Qu Wenruo wrote:
>>> There is a long existing window that if we did some operations marking
>>> qgroup INCONSISTENT during a qgroup rescan, the INCONSISTENT bit will be
>>> cleared by rescan, leaving incorrect qgroup numbers unnoticed.
>>>
>>> Furthermore, when we mark qgroup INCONSISTENT, we can in theory skip all
>>> qgroup accountings.
>>> Since the numbers are already crazy, we don't really need to waste time
>>> updating something that's already wrong.
>>>
>>> So here we introduce two runtime flags:
>>>
>>> - BTRFS_QGROUP_RUNTIME_FLAG_CANCEL_RESCAN
>>>    To inform any running rescan to exit immediately and don't clear
>>>    the INCONSISTENT bit on its exit.
>>>
>>> - BTRFS_QGROUP_RUNTIME_FLAG_NO_ACCOUNTING
>>>    To inform qgroup code not to do any accounting for dirty extents.
>>>
>>>    But still allow operations on qgroup relationship to be continued.
>>>
>>> Both flags will be set when an operation marks the qgroup INCONSISTENT
>>> and only get cleared when a new rescan is started.
>>>
>>>
>>> With those flags, we can have the following enhancement:
>>>
>>> - Prevent qgroup rescan to clear inconsistent flag which should be kept
>>>    If an operation marks qgroup inconsistent when a rescan is running,
>>>    qgroup rescan will clear the inconsistent flag while the qgroup
>>>    numbers are already wrong.
>>>
>>> - Skip qgroup accountings while qgroup numbers are already inconsistent
>>>
>>> - Skip huge subtree accounting when dropping subvolumes
>>>    With the obvious cost of marking qgroup inconsistent
>>>
>>>
>>> Reason for RFC:
>>> - If the runtime qgroup flags are acceptable
>>
>> As long as it's internal I think it's ok.
>>
>>> - If the behavior of marking qgroup inconsistent when dropping large
>>>    subvolumes
>>
>> That could be a bit problematic because user never knows if the rescan
>> has been started or not.
>
> I guess for the end users, the new behavior means they should know which
> operations could cancel rescan and cause qgroup to be inconsistent.
>
> For now, only qgroup inherit (snapshot creation with -i option or qgroup
> assign) and large subvolume dropping can cause such situation.
>
> If there is something like snapper running, we can teach snapper to
> check the qgroup status with an interval.
>
> But for non-snapper qgroup users, it can indeed be problematic,
> especially considering how frequently snapshot dropping can be.
>
> Any better idea on how to balance the performance and qgroup consistency?

Another idea is to allow the user to specify the threshold to skip
certain subtree drop.

And by default, we set the threshold to MAX_LEVEL, so no subtree will be
skipped (aka, super slow drop, but no sudden qgroup related slow down).

For the qgroup inherit case, it's no difference, any caller that would
bring qgroup inconsistent should know what they are doing.
Thus only the subvolume drop is the key.

Any idea for the interface? Per-fs sysfs? Or mount option?

Thanks,
Qu
>
> Thanks,
> Qu
>
>>
>>> - If the lifespan of runtime qgroup flags are acceptable
>>>    They have longer than needed lifespan (from inconsistent time
>>> point to
>>>    next rescan), not sure if it's OK.
>>
>> On first read I haven't found anything obviously problematic.
>>