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

[David Sterba <dsterba@suse.cz>]

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.

> - 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.