[PATCH AUTOSEL 4.19 34/38] btrfs: file: reserve qgroup space after the hole punch range is locked

[PATCH AUTOSEL 4.19 34/38] btrfs: file: reserve qgroup space after the hole punch range is locked

[Sasha Levin]

From: Qu Wenruo <wqu@suse.com>

[ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]

The incoming qgroup reserved space timing will move the data reservation
to ordered extent completely.

However in btrfs_punch_hole_lock_range() will call
btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the
range.

In current stage it's OK, but if we're making ordered extents handle the
reserved space, then btrfs_punch_hole_lock_range() can clear the
QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup
reserved space leakage.

So here change the timing to make reserve data space after
btrfs_punch_hole_lock_range().
The new timing is fine for either current code or the new code.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/btrfs/file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index dc1841855a69a..646152f305843 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3010,14 +3010,14 @@ static int btrfs_zero_range(struct inode *inode,
 		if (ret < 0)
 			goto out;
 		space_reserved = true;
-		ret = btrfs_qgroup_reserve_data(inode, &data_reserved,
-						alloc_start, bytes_to_reserve);
-		if (ret)
-			goto out;
 		ret = btrfs_punch_hole_lock_range(inode, lockstart, lockend,
 						  &cached_state);
 		if (ret)
 			goto out;
+		ret = btrfs_qgroup_reserve_data(inode, &data_reserved,
+						alloc_start, bytes_to_reserve);
+		if (ret)
+			goto out;
 		ret = btrfs_prealloc_file_range(inode, mode, alloc_start,
 						alloc_end - alloc_start,
 						i_blocksize(inode),
-- 
2.25.1

Re: [PATCH AUTOSEL 4.19 34/38] btrfs: file: reserve qgroup space after the hole punch range is locked

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

Hi!

> [ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]
> 
> The incoming qgroup reserved space timing will move the data reservation
> to ordered extent completely.
> 
> However in btrfs_punch_hole_lock_range() will call
> btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the
> range.
> 
> In current stage it's OK, but if we're making ordered extents handle the
> reserved space, then btrfs_punch_hole_lock_range() can clear the
> QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup
> reserved space leakage.
> 
> So here change the timing to make reserve data space after
> btrfs_punch_hole_lock_range().
> The new timing is fine for either current code or the new code.

I'm not sure why this is queued for -stable. It is preparation for
future work, and that work is not queued for -stable.

Best regards,
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH AUTOSEL 4.19 34/38] btrfs: file: reserve qgroup space after the hole punch range is locked

[Sasha Levin]

On Sat, Aug 29, 2020 at 02:11:23PM +0200, Pavel Machek wrote:
>Hi!
>
>> [ Upstream commit a7f8b1c2ac21bf081b41264c9cfd6260dffa6246 ]
>>
>> The incoming qgroup reserved space timing will move the data reservation
>> to ordered extent completely.
>>
>> However in btrfs_punch_hole_lock_range() will call
>> btrfs_invalidate_page(), which will clear QGROUP_RESERVED bit for the
>> range.
>>
>> In current stage it's OK, but if we're making ordered extents handle the
>> reserved space, then btrfs_punch_hole_lock_range() can clear the
>> QGROUP_RESERVED bit before we submit ordered extent, leading to qgroup
>> reserved space leakage.
>>
>> So here change the timing to make reserve data space after
>> btrfs_punch_hole_lock_range().
>> The new timing is fine for either current code or the new code.
>
>I'm not sure why this is queued for -stable. It is preparation for
>future work, and that work is not queued for -stable.

So you understand why it was queued: it's preparation for a fix that is
relevant to 4.19 but didn't apply cleanly.

I can look into what happened next week, or if you'd sent me a backport
I'd be happy to take it.

-- 
Thanks,
Sasha