diff mbox series

[12/13] xfs: make the calculation generic in xfs_sb_validate_fsb_count()

Message ID 20240226094936.2677493-13-kernel@pankajraghav.com (mailing list archive)
State New
Headers show
Series enable bs > ps in XFS | expand

Commit Message

Pankaj Raghav (Samsung) Feb. 26, 2024, 9:49 a.m. UTC 
From: Pankaj Raghav <p.raghav@samsung.com>

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@samsung.com>
---
 fs/xfs/xfs_mount.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Dave Chinner Feb. 26, 2024, 12:47 p.m. UTC | #1 
On Mon, Feb 26, 2024 at 10:49:35AM +0100, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> 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@samsung.com>
> ---
>  fs/xfs/xfs_mount.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index aabb25dc3efa..69af3b06be99 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -133,9 +133,15 @@ xfs_sb_validate_fsb_count(
>  {
>  	ASSERT(PAGE_SHIFT >= sbp->sb_blocklog);
>  	ASSERT(sbp->sb_blocklog >= BBSHIFT);
> +	uint64_t mapping_count;
> +	uint64_t bytes;
>  
> +	if (check_mul_overflow(nblocks, (1 << sbp->sb_blocklog), &bytes))
> +		return -EFBIG;
> +
> +	mapping_count = bytes >> PAGE_SHIFT;

max_index, not a "mapping count". Also, put this after this comment:

>  	/* Limited by ULONG_MAX of page cache index */

So it is obvious what the max_index we are calculating belongs to.

-Dave.
Matthew Wilcox Feb. 26, 2024, 1:21 p.m. UTC | #2 
On Mon, Feb 26, 2024 at 10:49:35AM +0100, Pankaj Raghav (Samsung) wrote:
> +	if (check_mul_overflow(nblocks, (1 << sbp->sb_blocklog), &bytes))

Why would you not use check_shl_overflow()?

> +		return -EFBIG;
> +
> +	mapping_count = bytes >> PAGE_SHIFT;
>  	/* Limited by ULONG_MAX of page cache index */
> -	if (nblocks >> (PAGE_SHIFT - sbp->sb_blocklog) > ULONG_MAX)
> +	if (mapping_count > ULONG_MAX)
>  		return -EFBIG;
>  	return 0;
>  }
> -- 
> 2.43.0
>
Pankaj Raghav (Samsung) Feb. 27, 2024, 8:44 a.m. UTC | #3 
On Mon, Feb 26, 2024 at 01:21:56PM +0000, Matthew Wilcox wrote:
> On Mon, Feb 26, 2024 at 10:49:35AM +0100, Pankaj Raghav (Samsung) wrote:
> > +	if (check_mul_overflow(nblocks, (1 << sbp->sb_blocklog), &bytes))
> 
> Why would you not use check_shl_overflow()?

This looks better than check_mul_overflow. I will use this in the next
version.
> 
> > +		return -EFBIG;
> > +
> > +	mapping_count = bytes >> PAGE_SHIFT;
> >  	/* Limited by ULONG_MAX of page cache index */
> > -	if (nblocks >> (PAGE_SHIFT - sbp->sb_blocklog) > ULONG_MAX)
> > +	if (mapping_count > ULONG_MAX)
> >  		return -EFBIG;
> >  	return 0;
> >  }
> > -- 
> > 2.43.0
> >
diff mbox series

Patch

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index aabb25dc3efa..69af3b06be99 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -133,9 +133,15 @@  xfs_sb_validate_fsb_count(
 {
 	ASSERT(PAGE_SHIFT >= sbp->sb_blocklog);
 	ASSERT(sbp->sb_blocklog >= BBSHIFT);
+	uint64_t mapping_count;
+	uint64_t bytes;
 
+	if (check_mul_overflow(nblocks, (1 << sbp->sb_blocklog), &bytes))
+		return -EFBIG;
+
+	mapping_count = bytes >> PAGE_SHIFT;
 	/* Limited by ULONG_MAX of page cache index */
-	if (nblocks >> (PAGE_SHIFT - sbp->sb_blocklog) > ULONG_MAX)
+	if (mapping_count > ULONG_MAX)
 		return -EFBIG;
 	return 0;
 }