Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7cf55e21-4d68-8371-a4a5-08cd8278bcf9@gmx.com>
Date:   Thu, 13 Oct 2022 07:12:10 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH AUTOSEL 6.0 38/46] btrfs: introduce
 BTRFS_QGROUP_RUNTIME_FLAG_CANCEL_RESCAN
To:     dsterba@suse.cz, Sasha Levin <sashal@kernel.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Qu Wenruo <wqu@suse.com>, David Sterba <dsterba@suse.com>,
        clm@fb.com, josef@toxicpanda.com, linux-btrfs@vger.kernel.org
References: <20221011145015.1622882-1-sashal@kernel.org>
 <20221011145015.1622882-38-sashal@kernel.org>
 <20221012125648.GX13389@suse.cz>
Content-Language: en-US
From:   Qu Wenruo <quwenruo.btrfs@gmx.com>
In-Reply-To: <20221012125648.GX13389@suse.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


On 2022/10/12 20:56, David Sterba wrote:
> On Tue, Oct 11, 2022 at 10:50:06AM -0400, Sasha Levin wrote:
>> From: Qu Wenruo <wqu@suse.com>
>>
>> [ Upstream commit e562a8bdf652b010ce2525bcf15d145c9d3932bf ]
>>
>> Introduce a new runtime flag, BTRFS_QGROUP_RUNTIME_FLAG_CANCEL_RESCAN,
>> which will inform qgroup rescan to cancel its work asynchronously.
>>
>> This is to address the window when an operation makes qgroup numbers
>> inconsistent (like qgroup inheriting) while a qgroup rescan is running.
>>
>> In that case, qgroup inconsistent flag will be cleared when qgroup
>> rescan finishes.
>> But we changed the ownership of some extents, which means the rescan is
>> already meaningless, and the qgroup inconsistent flag should not be
>> cleared.
>>
>> With the new flag, each time we set INCONSISTENT flag, we also set this
>> new flag to inform any running qgroup rescan to exit immediately, and
>> leaving the INCONSISTENT flag there.
>>
>> The new runtime flag can only be cleared when a new rescan is started.
>
> Qu, does this patch make sense for stable on itself? It was part of a
> series adding some new flags and the sysfs knob.  As I read it there's a
> case where it can affect how the rescan is done and that it can be
> cancelled but still am not sure if it's worth the backport.

Considering the qgroup still lacks a way to handle large subvolume drop,
and a lot of things can mark qgroup inconsistent halfway, I think
backporting this patch itself is not that bad.

The problem is, why only backporting this one?

To me, it would make more sense to backport either all or none.

Sure, if we can cancel rescan it's an improvement, but rescan itself is
already relatively cheap compared to other qgroup operations.
Thus I prefer to backport all the qgroup patches.

Thanks,
Qu