[PATCH] btrfs: set cache_block_group_error if we find an error

[PATCH] btrfs: set cache_block_group_error if we find an error

[Josef Bacik]

We set cache_block_group_error if btrfs_cache_block_group() returns an
error, this is because we could end up not finding space to allocate and
mistakenly return -ENOSPC, and which could then abort the transaction
with the incorrect errno, and in the case of ENOSPC result in a
WARN_ON() that will trip up tests like generic/475.

However there's the case where multiple threads can be racing, one
thread gets the proper error, and the other thread doesn't actually call
btrfs_cache_block_group(), it instead sees ->cached ==
BTRFS_CACHE_ERROR.  Again the result is the same, we fail to allocate
our space and return -ENOSPC.  Instead we need to set
cache_block_group_error to -EIO in this case to make sure that if we do
not make our allocation we get the appropriate error returned back to
the caller.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/extent-tree.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 6a3414545e01..7fce05cc6090 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4277,8 +4277,11 @@ static noinline int find_free_extent(struct btrfs_root *root,
 			ret = 0;
 		}
 
-		if (unlikely(block_group->cached == BTRFS_CACHE_ERROR))
+		if (unlikely(block_group->cached == BTRFS_CACHE_ERROR)) {
+			if (!cache_block_group_error)
+				cache_block_group_error = -EIO;
 			goto loop;
+		}
 
 		if (!find_free_extent_check_size_class(ffe_ctl, block_group))
 			goto loop;
-- 
2.41.0

Re: [PATCH] btrfs: set cache_block_group_error if we find an error

[David Sterba]

On Wed, Aug 02, 2023 at 09:20:24AM -0400, Josef Bacik wrote:
> We set cache_block_group_error if btrfs_cache_block_group() returns an
> error, this is because we could end up not finding space to allocate and
> mistakenly return -ENOSPC, and which could then abort the transaction
> with the incorrect errno, and in the case of ENOSPC result in a
> WARN_ON() that will trip up tests like generic/475.
> 
> However there's the case where multiple threads can be racing, one
> thread gets the proper error, and the other thread doesn't actually call
> btrfs_cache_block_group(), it instead sees ->cached ==
> BTRFS_CACHE_ERROR.  Again the result is the same, we fail to allocate
> our space and return -ENOSPC.  Instead we need to set
> cache_block_group_error to -EIO in this case to make sure that if we do
> not make our allocation we get the appropriate error returned back to
> the caller.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.