| Message ID | 8717f1907f699058ab6a6941c007ad43c903a3ca.1690982408.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: set cache_block_group_error if we find an error | expand |
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.
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;
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(-)