Re: [PATCH V8 11/19] xfs: Use uint64_t to count maximum blocks that can be used by BMBT

Chandan Babu R
2022-03-25T06:01:54Z

On 25 Mar 2022 at 03:12, Dave Chinner wrote:
> On Mon, Mar 21, 2022 at 10:47:42AM +0530, Chandan Babu R wrote:
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>> fs/xfs/libxfs/xfs_bmap.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> index 9f38e33d6ce2..b317226fb4ba 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -52,9 +52,9 @@ xfs_bmap_compute_maxlevels(
>> xfs_mount_t *mp, /* file system mount structure */
>> int whichfork) /* data or attr fork */
>> {
>> - int level; /* btree level */
>> - uint maxblocks; /* max blocks at this level */
>> + uint64_t maxblocks; /* max blocks at this level */
>> xfs_extnum_t maxleafents; /* max leaf entries possible */
>> + int level; /* btree level */
>> int maxrootrecs; /* max records in root block */
>> int minleafrecs; /* min records in leaf block */
>> int minnoderecs; /* min records in node block */
>> @@ -88,7 +88,7 @@ xfs_bmap_compute_maxlevels(
>> if (maxblocks <= maxrootrecs)
>> maxblocks = 1;
>> else
>> - maxblocks = (maxblocks + minnoderecs - 1) / minnoderecs;
>> + maxblocks = howmany_64(maxblocks, minnoderecs);
>> }
>> mp->m_bm_maxlevels[whichfork] = level;
>> ASSERT(mp->m_bm_maxlevels[whichfork] <= xfs_bmbt_maxlevels_ondisk());
>
> Hmmmm. Shouldn't this be rolled up into the earlier patch that
> converts a seperate part of this function to use howmany_64()?
> That was done in "[PATCH V8 07/19] xfs: Promote xfs_extnum_t and
> xfs_aextnum_t to 64 and 32-bits respectively" - it seems to me like
> this should definitely be part of the type size extension rather
> than a stand-alone change.

The goal of the earlier patch was to extend the basic data types underlying
xfs_extnum_t and xfs_aextnum_t. The "maxblocks" data type change was included
in a separate patch since the change does not fall into the purview of
extending either xfs_extnum_t or xfs_aextnum_t.

--
chandan