Re: [PATCH v2] btrfs: fix delayed_refs_rsv leak when realloc shrinks an extent

[Filipe Manana]

On Thu, Nov 5, 2026 at 10:45 AM Shunbo Shi
<shishunbo@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> When a data extent is freed and reallocated (with a different size)
> before its delayed refs are processed,
> btrfs_alloc_reserved_file_extent() reuses the existing delayed ref
> head and update_existing_head_ref() updates existing->num_bytes to
> the new size, but neither adjusts the space used for checksums nor
> releases excess space from the delayed refs block reserve.
>
> The csum space for deleting the checksums was reserved for the
> original size in add_delayed_ref_head():
>
>         pending_csums += head_ref->num_bytes;
>         trans->delayed_ref_csum_deletions += csum_leaves(num_bytes);
>
> but when total_ref_mod later turns non-negative,
> update_existing_head_ref() releases the reservation based on the
> new num_bytes:
>
>         pending_csums -= existing->num_bytes;
>         btrfs_delayed_refs_rsv_release(fs_info, 0, csum_leaves);
>
> For a smaller new size the difference between the old and the new
> size is never released, so delayed_refs_rsv.size (and .reserved)
> stay above zero forever.  At unmount this triggers
>
>         WARNING: fs_info->delayed_refs_rsv.reserved > 0
>         at btrfs_release_global_block_rsv
>
> and at runtime the leaked reservation keeps inflating
> space_info->bytes_may_use, which can lead to premature ENOSPC.
>
> For a larger new size the release subtracts more than what was
> reserved for the head: if the rsv does not hold reservations of
> other delayed refs, delayed_refs_rsv.size underflows and leaks
> reserved bytes; otherwise the rsv prematurely releases reservations
> belonging to other delayed refs.
>
> Fix this by adjusting the csum accounting to the new size at the
> time existing->num_bytes is updated, while the head is still in the
> reservation state (total_ref_mod < 0): release the difference in
> csum leaves for a smaller size, and grow the rsv size by the
> difference for a larger size.  The grow direction must take effect
> immediately (btrfs_delayed_refs_rsv_add()) rather than via
> trans->delayed_ref_csum_deletions, because the total_ref_mod
> transition below may release the new size in the same call, while
> the trans counter is only folded into the rsv afterwards by
> btrfs_update_delayed_refs_rsv().
>
> Fixes: ba2c4d4e3bda ("btrfs: introduce delayed_refs_rsv")
> Reported-by: syzbot+7ec60f4482abeecb0fd8@xxxxxxxxxxxxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/linux-btrfs/6ae88b9e.64e51036.15cf6a.0027.GAE@xxxxxxxxxx/
> Signed-off-by: Shunbo Shi <shishunbo@xxxxxxxxxxxxxxxxxxxx>
> ---
>  fs/btrfs/delayed-ref.c | 61 ++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/delayed-ref.h |  4 ++-
>  2 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index 8bc1929237f7..45e269670068 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -63,6 +63,31 @@ bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info)
>   * Drops the delayed ref head's count from the delayed refs rsv and free any
>   * excess reservation we had.
>   */
> +/*
> + * Grow the delayed refs rsv size for csum items to be deleted.
> + *
> + * Unlike trans->delayed_ref_csum_deletions, which is folded into the
> + * delayed refs rsv by  at a later point,
> + * this adjusts the size immediately.  It is needed when a ref head's
> + * num_bytes is updated to a larger size and the head's reservation is
> + * released in the same call to update_existing_head_ref(): the release
> + * path subtracts from the rsv right away, so the size must cover the
> + * new csum leaves before that happens, otherwise the rsv would
> + * over-release reservations of other delayed refs.
> + */
> +void btrfs_delayed_refs_rsv_add(struct btrfs_fs_info *fs_info, int nr_csums)
> +{
> +       struct btrfs_block_rsv *block_rsv = &fs_info->delayed_refs_rsv;
> +       u64 num_bytes;
> +
> +       num_bytes = btrfs_calc_delayed_ref_csum_bytes(fs_info, nr_csums);
> +
> +       spin_lock(&block_rsv->lock);
> +       block_rsv->size += num_bytes;
> +       block_rsv->full = (block_rsv->reserved >= block_rsv->size);
> +       spin_unlock(&block_rsv->lock);
> +}

First thing:

You are adding a new function, and its comment, between an existing
function and its comment.
This is messy. Also the limit for comments is 80 characters per line,
and this comment uses significantly less - the lines can be widened
for better readability.

Second:

Why export the function? Since it's only used in this file, it should be static.

Third:

Why exactly do we need it?
We should update trans->delayed_ref_csum_deletions and then let the
call to btrfs_update_delayed_refs_rsv() done up the callchain (at
add_delayed_ref()) do the updates to the global delayed refs reserve.
It's not clear to me why we can't simply do that.


> +
>  void btrfs_delayed_refs_rsv_release(struct btrfs_fs_info *fs_info, int nr_refs, int nr_csums)
>  {
>         struct btrfs_block_rsv *block_rsv = &fs_info->delayed_refs_rsv;
> @@ -694,6 +719,42 @@ static noinline void update_existing_head_ref(struct btrfs_trans_handle *trans,
>                 existing->must_insert_reserved = update->must_insert_reserved;
>                 existing->owning_root = update->owning_root;
>
> +               /*
> +                * The extent may have been reallocated with a different
> +                * size before its delayed refs were processed.  The csum
> +                * reservation was made for the old size (see
> +                * add_delayed_ref_head()), so adjust the accounting to the
> +                * new size here; otherwise the release done when
> +                * total_ref_mod turns non-negative below is based on the
> +                * new size while only the old size was reserved: for a
> +                * smaller size the difference is leaked, for a larger size
> +                * the rsv over-releases reservations of other delayed refs.
> +                *
> +                * Both adjustments must take effect immediately: the
> +                * total_ref_mod transition below may release new_leaves in
> +                * this same call, while trans->delayed_ref_csum_deletions
> +                * is only folded into the rsv by
> +                * btrfs_update_delayed_refs_rsv() after we return.
> +                */
> +               if (existing->is_data && existing->total_ref_mod < 0 &&
> +                   existing->num_bytes != update->num_bytes) {
> +                       u64 old_leaves = btrfs_csum_bytes_to_leaves(fs_info,
> +                                                          existing->num_bytes);
> +                       u64 new_leaves = btrfs_csum_bytes_to_leaves(fs_info,
> +                                                           update->num_bytes);
> +
> +                       if (new_leaves < old_leaves) {
> +                               btrfs_delayed_refs_rsv_release(fs_info, 0,
> +                                              (int)(old_leaves - new_leaves));
> +                       } else if (new_leaves > old_leaves) {
> +                               btrfs_delayed_refs_rsv_add(fs_info,
> +                                              (int)(new_leaves - old_leaves));
> +                       }

For if statements, we only use { } when there's more than one
statement in a branch. Since in both cases it's a single statement,
please remove the  { }.

> +
> +                       delayed_refs->pending_csums += update->num_bytes;
> +                       delayed_refs->pending_csums -= existing->num_bytes;
> +               }
> +
>                 /*
>                  * update the num_bytes so we make sure the accounting
>                  * is done correctly
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index 5ce940532144..f8434347b80b 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -408,7 +408,9 @@ struct btrfs_delayed_ref_node *btrfs_select_delayed_ref(struct btrfs_delayed_ref
>
>  int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info, u64 seq);
>
> -void btrfs_delayed_refs_rsv_release(struct btrfs_fs_info *fs_info, int nr_refs, int nr_csums);
> +void btrfs_delayed_refs_rsv_release(struct btrfs_fs_info *fs_info, int nr_refs,
> +                                    int nr_csums);

We don't mix formatting changes without functional changes.
Leave it as it was. It's a 94 characters line, and that's fine for
today's standards.

Finally, sashiko is still complaining about the same issues as in v1:

https://sashiko.dev/#/patchset/20261105104453.2151-1-shishunbo%40cmss.chinamobile.com

Thanks.

> +void btrfs_delayed_refs_rsv_add(struct btrfs_fs_info *fs_info, int nr_csums);
>  void btrfs_update_delayed_refs_rsv(struct btrfs_trans_handle *trans);
>  void btrfs_inc_delayed_refs_rsv_bg_inserts(struct btrfs_fs_info *fs_info);
>  void btrfs_dec_delayed_refs_rsv_bg_inserts(struct btrfs_fs_info *fs_info);
> --
> 2.53.0
>
>
>