Re: [PATCH 09/14] xfs: fix maxlevels comparisons in the btree staging code

Chandan Babu R
2021-09-20T10:11:21Z

On 18 Sep 2021 at 06:59, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The btree geometry computation function has an off-by-one error in that
> it does not allow maximally tall btrees (nlevels == XFS_BTREE_MAXLEVELS).
> This can result in repairs failing unnecessarily on very fragmented
> filesystems. Subsequent patches to remove MAXLEVELS usage in favor of
> the per-btree type computations will make this a much more likely
> occurrence.

Looks good.

Reviewed-by: Chandan Babu R <chandan.babu@oracle.com>

>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_btree_staging.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_btree_staging.c b/fs/xfs/libxfs/xfs_btree_staging.c
> index 26143297bb7b..cc56efc2b90a 100644
> --- a/fs/xfs/libxfs/xfs_btree_staging.c
> +++ b/fs/xfs/libxfs/xfs_btree_staging.c
> @@ -662,7 +662,7 @@ xfs_btree_bload_compute_geometry(
> xfs_btree_bload_ensure_slack(cur, &bbl->node_slack, 1);
>
> bbl->nr_records = nr_this_level = nr_records;
> - for (cur->bc_nlevels = 1; cur->bc_nlevels < XFS_BTREE_MAXLEVELS;) {
> + for (cur->bc_nlevels = 1; cur->bc_nlevels <= XFS_BTREE_MAXLEVELS;) {
> uint64_t level_blocks;
> uint64_t dontcare64;
> unsigned int level = cur->bc_nlevels - 1;
> @@ -726,7 +726,7 @@ xfs_btree_bload_compute_geometry(
> nr_this_level = level_blocks;
> }
>
> - if (cur->bc_nlevels == XFS_BTREE_MAXLEVELS)
> + if (cur->bc_nlevels > XFS_BTREE_MAXLEVELS)
> return -EOVERFLOW;
>
> bbl->btree_height = cur->bc_nlevels;

--
chandan