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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 07, 2024 at 02:13:10PM -0700, Darrick J. Wong wrote:
> On Tue, May 07, 2024 at 09:40:58AM +0100, John Garry wrote:
> > 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;
> 
> Extra nit: the  ^ indentation of the names should have tabs, like the
> other xfs functions.
Thanks John and Darrick, I will fold it in the next series.

> 
> --D
> 
> > 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;
> > >   }
> > 
> > 

-- 
Pankaj Raghav




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux