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

[Filipe Manana]

On Wed, Nov 4, 2026 at 8:28 AM Shunbo Shi
<shishunbo@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> When a data extent is freed and reallocated (with a smaller size)
> before its delayed refs are processed, update_existing_head_ref()
> updates existing->num_bytes to the new size while the extent is
> still in the " csum reservation\ state (total_ref_mod < 0).

This last part of the sentence is unintelligible... an opening quote
without an ending quote, a backslash for no reason, and everything
after the opening quote doesn't make any sense.

Something like:

... updates existing->num_bytes but does 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, smaller num_bytes:
>
>  pending_csums -= existing->num_bytes;
>  btrfs_delayed_refs_rsv_release(fs_info, 0, csum_leaves);
>
> 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.
>
> Fix this by adjusting the csum accounting to the new size at the
> time existing->num_bytes is updated: if the head is still in the
> reservation state, release the difference in csum leaves (or add
> it, if the extent grew) and update pending_csums accordingly, so
> that the release in the total_ref_mod transition stays symmetric
> with the reservation.
>
> 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 | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index 8bc1929237f7..a3e32f08dacc 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -694,6 +694,34 @@ 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 releases less than
> +                * what was reserved and delayed_refs_rsv leaks the
> +                * difference.
> +                */
> +               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,
> +                                               old_leaves - new_leaves);
> +                       else if (new_leaves > old_leaves)
> +                               trans->delayed_ref_csum_deletions +=
> +                                       new_leaves - old_leaves;

We can't update delayed_ref_csums_deletions blindly without looking
into existing->total_ref_mod.
This else branch should be:

else if (new_leaves > old_leaves && existing->total_ref_mod < 0)

And I probably missed something more.
Sashiko is complaining about that I think and more:

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

Thanks.



> +
> +                       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
> --
> 2.53.0
>
>
>
>