Re: [PATCH] btrfs: qgroup: fix a wrong length calculation in qgroup_free_reserved_data()

Filipe Manana <[email protected]> Wed, 29 Jul 2026 13:27:33 +0100
Newsgroups org.kernel.vger.linux-btrfs
Message-ID <CAL3q7H43GFWphbXtW=HJfT+ix2Lh9p8umf1nGNu0OG363WGt3A@mail.gmail.com>
On Tue, Jul 28, 2026 at 3:40=E2=80=AFAM Qu Wenruo <[email protected]> wrote:
>
> In that function, we round down the start position and round up the
> ending position.
>
> But during the calculation of @len, we use "round_up(start + len,
> sectorsize)", which is the rounded up end position, not the rounded up
> length.
>
> Which results a much larger length, and later we are still using "start
> + len", which is completely incorrect.
>
> Fix it by declaring a local @algined_start and @aligned_len and use them

typo: algined_start -> aligned_start

Otherwise:

Reviewed-by: Filipe Manana <[email protected]>

Thanks.

> instead.
>
> Fixes: bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow =
by only freeing reserved ranges")
> Signed-off-by: Qu Wenruo <[email protected]>
> ---
>  fs/btrfs/qgroup.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 210af4d7d4b5..f68b696b4bf7 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -4339,12 +4339,13 @@ static int qgroup_free_reserved_data(struct btrfs=
_inode *inode,
>         struct ulist_node *unode;
>         struct ulist_iterator uiter;
>         struct extent_changeset changeset;
> +       const u32 sectorsize =3D root->fs_info->sectorsize;
> +       const u64 aligned_start =3D round_down(start, sectorsize);
> +       const u64 aligned_len =3D round_up(start + len, sectorsize) - ali=
gned_start;
>         u64 freed =3D 0;
>         int ret;
>
>         extent_changeset_init_bytes_only(&changeset);
> -       len =3D round_up(start + len, root->fs_info->sectorsize);
> -       start =3D round_down(start, root->fs_info->sectorsize);
>
>         ULIST_ITER_INIT(&uiter);
>         while ((unode =3D ulist_next(&reserved->range_changed, &uiter))) =
{
> @@ -4356,12 +4357,15 @@ static int qgroup_free_reserved_data(struct btrfs=
_inode *inode,
>
>                 extent_changeset_release(&changeset);
>
> -               /* Only free range in range [start, start + len) */
> -               if (range_start >=3D start + len ||
> -                   range_start + range_len <=3D start)
> +               /*
> +                * Only free the range within
> +                * [aligned_start, aligned_start + aligned_len).
> +                */
> +               if (range_start >=3D aligned_start + aligned_len ||
> +                   range_start + range_len <=3D aligned_start)
>                         continue;
> -               free_start =3D max(range_start, start);
> -               free_len =3D min(start + len, range_start + range_len) -
> +               free_start =3D max(range_start, aligned_start);
> +               free_len =3D min(aligned_start + aligned_len, range_start=
 + range_len) -
>                            free_start;
>                 /*
>                  * TODO: To also modify reserved->ranges_reserved to refl=
ect
> --
> 2.54.0
>
>