Re: [PATCH v5 10/11] xfs: make the calculation generic in xfs_sb_validate_fsb_count()

[John Garry <john.g.garry@xxxxxxxxxx>]

On 03/05/2024 10:53, Luis Chamberlain wrote:
> From: Pankaj Raghav <p.raghav@xxxxxxxxxxx>
>
> Instead of assuming that PAGE_SHIFT is always higher than the blocklog,
> make the calculation generic so that page cache count can be calculated
> correctly for LBS.
>
> Signed-off-by: Pankaj Raghav <p.raghav@xxxxxxxxxxx>
> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
>   fs/xfs/xfs_mount.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index df370eb5dc15..56d71282972a 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -133,9 +133,16 @@ xfs_sb_validate_fsb_count(
>   {
>   	ASSERT(PAGE_SHIFT >= sbp->sb_blocklog);
>   	ASSERT(sbp->sb_blocklog >= BBSHIFT);
> +	uint64_t max_index;
> +	uint64_t max_bytes;

nit: any other XFS code which I have seen puts the declarations before 
any ASSERT() calls

> +
> +	if (check_shl_overflow(nblocks, sbp->sb_blocklog, &max_bytes))
> +		return -EFBIG;
>   
>   	/* Limited by ULONG_MAX of page cache index */
> -	if (nblocks >> (PAGE_SHIFT - sbp->sb_blocklog) > ULONG_MAX)
> +	max_index = max_bytes >> PAGE_SHIFT;
> +
> +	if (max_index > ULONG_MAX)
>   		return -EFBIG;
>   	return 0;
>   }