| Message ID | 20240503095353.3798063-11-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | enable bs > ps in XFS | expand |
On 03/05/2024 10:53, Luis Chamberlain 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> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > --- > 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; > }
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@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> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --- > > 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. --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; > > } > >
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@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> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > 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; > > > } > > > >
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; + + 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; }