Re: [PATCH V7 17/17] xfs: Define max extent length based on on-disk format definition

Chandan Babu R
2022-03-05T12:46:14Z

On 04 Mar 2022 at 13:45, Dave Chinner wrote:
> On Tue, Mar 01, 2022 at 04:09:38PM +0530, Chandan Babu R wrote:
>> The maximum extent length depends on maximum block count that can be stored in
>> a BMBT record. Hence this commit defines MAXEXTLEN based on
>> BMBT_BLOCKCOUNT_BITLEN.
>>
>> While at it, the commit also renames MAXEXTLEN to XFS_MAX_BMBT_EXTLEN.
>>
>> Suggested-by: Darrick J. Wong <djwong@kernel.org>
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>
> Looks fine, but this should be up near the top of the series where
> all the extent count definitions are being changed. Also, minor
> formatting nit below.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
>> @@ -299,7 +299,8 @@ xfs_calc_write_reservation(
>> * the agf for each of the ags: 2 * sector size
>> * the agfl for each of the ags: 2 * sector size
>> * the super block to reflect the freed blocks: sector size
>> - * the realtime bitmap: 2 exts * ((MAXEXTLEN / rtextsize) / NBBY) bytes
>> + * the realtime bitmap: 2 exts * ((XFS_BMBT_MAX_EXTLEN / rtextsize) / NBBY)
>> + * bytes
>
> Break the line at the ":"
>
> * the realtime bitmap:
> * 2 exts * ((XFS_BMBT_MAX_EXTLEN / rtextsize) / NBBY) bytes
>
> Which makes it consistent with the rest of the comment:
>
>> * the realtime summary: 2 exts * 1 block
>> * worst case split in allocation btrees per extent assuming 2 extents:
>> * 2 exts * 2 trees * (2 * max depth - 1) * block size
>

Ok. I will include this in the next version of the patchset.

Thanks a lot for reviewing the entire patchset.

--
chandan