[PATCH] btrfs: fix invalid data space release when truncating block in NOCOW mode

[PATCH] btrfs: fix invalid data space release when truncating block in NOCOW mode

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

If when truncating a block we fail to reserve data space and then we
proceed anyway because we can do a NOCOW write, if we later get an error
when trying to get the folio from the inode's mapping, we end up releasing
data space that we haven't reserved, screwing up the bytes_may_use counter
from the data space_info, eventually resulting in an underflow when all
other reservations done by other tasks are released, if any, or right away
if there are no other reservations at the moment.

This is because when we get an error when trying to grab the block's folio
we call btrfs_delalloc_release_space(), which releases metadata (which we
have reserved) and data (which we haven't reserved).

Fix this by calling btrfs_delalloc_release_space() only if we did reserve
data space, that is, if we aren't falling back to NOCOW, meaning the local
variable @only_release_metadata has a false value, otherwise release only
metadata by calling btrfs_delalloc_release_metadata().

Fixes: 6d4572a9d71d ("btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d518acb7b01d..8c1f7196636a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4932,8 +4932,11 @@ int btrfs_truncate_block(struct btrfs_inode *inode, u64 offset, u64 start, u64 e
 	folio = __filemap_get_folio(mapping, index,
 				    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
 	if (IS_ERR(folio)) {
-		btrfs_delalloc_release_space(inode, data_reserved, block_start,
-					     blocksize, true);
+		if (only_release_metadata)
+			btrfs_delalloc_release_metadata(inode, blocksize, true);
+		else
+			btrfs_delalloc_release_space(inode, data_reserved,
+						     block_start, blocksize, true);
 		btrfs_delalloc_release_extents(inode, blocksize);
 		ret = -ENOMEM;
 		goto out;
-- 
2.47.2

Re: [PATCH] btrfs: fix invalid data space release when truncating block in NOCOW mode

[Qu Wenruo]

在 2025/5/10 02:35, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If when truncating a block we fail to reserve data space and then we
> proceed anyway because we can do a NOCOW write, if we later get an error
> when trying to get the folio from the inode's mapping, we end up releasing
> data space that we haven't reserved, screwing up the bytes_may_use counter
> from the data space_info, eventually resulting in an underflow when all
> other reservations done by other tasks are released, if any, or right away
> if there are no other reservations at the moment.
> 
> This is because when we get an error when trying to grab the block's folio
> we call btrfs_delalloc_release_space(), which releases metadata (which we
> have reserved) and data (which we haven't reserved).
> 
> Fix this by calling btrfs_delalloc_release_space() only if we did reserve
> data space, that is, if we aren't falling back to NOCOW, meaning the local
> variable @only_release_metadata has a false value, otherwise release only
> metadata by calling btrfs_delalloc_release_metadata().
> 
> Fixes: 6d4572a9d71d ("btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/inode.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d518acb7b01d..8c1f7196636a 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4932,8 +4932,11 @@ int btrfs_truncate_block(struct btrfs_inode *inode, u64 offset, u64 start, u64 e
>   	folio = __filemap_get_folio(mapping, index,
>   				    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
>   	if (IS_ERR(folio)) {
> -		btrfs_delalloc_release_space(inode, data_reserved, block_start,
> -					     blocksize, true);
> +		if (only_release_metadata)
> +			btrfs_delalloc_release_metadata(inode, blocksize, true);
> +		else
> +			btrfs_delalloc_release_space(inode, data_reserved,
> +						     block_start, blocksize, true);
>   		btrfs_delalloc_release_extents(inode, blocksize);
>   		ret = -ENOMEM;
>   		goto out;