diff mbox series

[1/2] btrfs: qgroup: fix wrong qgroup metadata reserve for delayed inode

Message ID 20200724064610.69442-1-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series [1/2] btrfs: qgroup: fix wrong qgroup metadata reserve for delayed inode | expand

Commit Message

Qu Wenruo July 24, 2020, 6:46 a.m. UTC
For delayed inode facility, qgroup metadata is reserved for it, and
later freed.

However we're freeing more bytes than we reserved.
In btrfs_delayed_inode_reserve_metadata():

	num_bytes = btrfs_calc_metadata_size(fs_info, 1);
	...
		ret = btrfs_qgroup_reserve_meta_prealloc(root,
				fs_info->nodesize, true);
		...
		if (!ret) {
			node->bytes_reserved = num_bytes;

But in btrfs_delayed_inode_release_metadata():

	if (qgroup_free)
		btrfs_qgroup_free_meta_prealloc(node->root,
				node->bytes_reserved);
	else
		btrfs_qgroup_convert_reserved_meta(node->root,
				node->bytes_reserved);

This means, we're always releasing more qgroup metadata rsv than we have
reserved.

This won't trigger selftest warning, as btrfs qgroup metadata rsv has
extra protection against cases like quota enabled half-way.

But we still need to fix this problem any way.

This patch will use the same num_bytes for qgroup metadata rsv so we
could handle it correctly.

Fixes: f218ea6c4792 ("btrfs: delayed-inode: Remove wrong qgroup meta reservation calls")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/delayed-inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Josef Bacik Aug. 24, 2020, 6:02 p.m. UTC | #1
On 7/24/20 2:46 AM, Qu Wenruo wrote:
> For delayed inode facility, qgroup metadata is reserved for it, and
> later freed.
> 
> However we're freeing more bytes than we reserved.
> In btrfs_delayed_inode_reserve_metadata():
> 
> 	num_bytes = btrfs_calc_metadata_size(fs_info, 1);
> 	...
> 		ret = btrfs_qgroup_reserve_meta_prealloc(root,
> 				fs_info->nodesize, true);
> 		...
> 		if (!ret) {
> 			node->bytes_reserved = num_bytes;
> 
> But in btrfs_delayed_inode_release_metadata():
> 
> 	if (qgroup_free)
> 		btrfs_qgroup_free_meta_prealloc(node->root,
> 				node->bytes_reserved);
> 	else
> 		btrfs_qgroup_convert_reserved_meta(node->root,
> 				node->bytes_reserved);
> 
> This means, we're always releasing more qgroup metadata rsv than we have
> reserved.
> 
> This won't trigger selftest warning, as btrfs qgroup metadata rsv has
> extra protection against cases like quota enabled half-way.
> 
> But we still need to fix this problem any way.
> 
> This patch will use the same num_bytes for qgroup metadata rsv so we
> could handle it correctly.
> 
> Fixes: f218ea6c4792 ("btrfs: delayed-inode: Remove wrong qgroup meta reservation calls")
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
David Sterba Aug. 27, 2020, 11:32 a.m. UTC | #2
On Fri, Jul 24, 2020 at 02:46:09PM +0800, Qu Wenruo wrote:
> For delayed inode facility, qgroup metadata is reserved for it, and
> later freed.
> 
> However we're freeing more bytes than we reserved.
> In btrfs_delayed_inode_reserve_metadata():
> 
> 	num_bytes = btrfs_calc_metadata_size(fs_info, 1);
> 	...
> 		ret = btrfs_qgroup_reserve_meta_prealloc(root,
> 				fs_info->nodesize, true);
> 		...
> 		if (!ret) {
> 			node->bytes_reserved = num_bytes;
> 
> But in btrfs_delayed_inode_release_metadata():
> 
> 	if (qgroup_free)
> 		btrfs_qgroup_free_meta_prealloc(node->root,
> 				node->bytes_reserved);
> 	else
> 		btrfs_qgroup_convert_reserved_meta(node->root,
> 				node->bytes_reserved);
> 
> This means, we're always releasing more qgroup metadata rsv than we have
> reserved.
> 
> This won't trigger selftest warning, as btrfs qgroup metadata rsv has
> extra protection against cases like quota enabled half-way.
> 
> But we still need to fix this problem any way.
> 
> This patch will use the same num_bytes for qgroup metadata rsv so we
> could handle it correctly.
> 
> Fixes: f218ea6c4792 ("btrfs: delayed-inode: Remove wrong qgroup meta reservation calls")
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to misc-next.

Patch 2/2 should go to stable (4.19+) but it does not apply even to 5.8
due to some other intermediate changes. I have tagged it for 4.19 but it
will expectedly fail to apply. A backport would be needed then.
diff mbox series

Patch

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index bf1595a42a98..0727b10a9a89 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -627,8 +627,7 @@  static int btrfs_delayed_inode_reserve_metadata(
 	 */
 	if (!src_rsv || (!trans->bytes_reserved &&
 			 src_rsv->type != BTRFS_BLOCK_RSV_DELALLOC)) {
-		ret = btrfs_qgroup_reserve_meta_prealloc(root,
-				fs_info->nodesize, true);
+		ret = btrfs_qgroup_reserve_meta_prealloc(root, num_bytes, true);
 		if (ret < 0)
 			return ret;
 		ret = btrfs_block_rsv_add(root, dst_rsv, num_bytes,