On 05 Jan 2022 at 06:17, Darrick J. Wong wrote:
> On Tue, Dec 14, 2021 at 02:15:19PM +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@xxxxxxxxxx>
>> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>> ---
>> fs/xfs/libxfs/xfs_alloc.c | 2 +-
>> fs/xfs/libxfs/xfs_bmap.c | 57 +++++++++++++++++-----------------
>> fs/xfs/libxfs/xfs_format.h | 21 +++++++------
>> fs/xfs/libxfs/xfs_inode_buf.c | 4 +--
>> fs/xfs/libxfs/xfs_trans_resv.c | 11 ++++---
>> fs/xfs/scrub/bmap.c | 2 +-
>> fs/xfs/xfs_bmap_util.c | 14 +++++----
>> fs/xfs/xfs_iomap.c | 28 ++++++++---------
>> 8 files changed, 72 insertions(+), 67 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
>> index 353e53b892e6..3f9b9cbfef43 100644
>> --- a/fs/xfs/libxfs/xfs_alloc.c
>> +++ b/fs/xfs/libxfs/xfs_alloc.c
>> @@ -2493,7 +2493,7 @@ __xfs_free_extent_later(
>>
>> ASSERT(bno != NULLFSBLOCK);
>> ASSERT(len > 0);
>> - ASSERT(len <= MAXEXTLEN);
>> + ASSERT(len <= XFS_MAX_BMBT_EXTLEN);
>> ASSERT(!isnullstartblock(bno));
>> agno = XFS_FSB_TO_AGNO(mp, bno);
>> agbno = XFS_FSB_TO_AGBNO(mp, bno);
>
> Yessss another unprefixed constant goes away.
>
> <snip>
>
>> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
>> index 3183f78fe7a3..dd5cffe63be3 100644
>> --- a/fs/xfs/libxfs/xfs_format.h
>> +++ b/fs/xfs/libxfs/xfs_format.h
>> @@ -885,15 +885,6 @@ enum xfs_dinode_fmt {
>> { XFS_DINODE_FMT_BTREE, "btree" }, \
>> { XFS_DINODE_FMT_UUID, "uuid" }
>>
>> -/*
>> - * Max values for extlen, extnum, aextnum.
>> - */
>> -#define MAXEXTLEN ((xfs_extlen_t)0x1fffff) /* 21 bits */
>> -#define XFS_MAX_EXTCNT_DATA_FORK ((xfs_extnum_t)0xffffffffffff) /* Unsigned 48-bits */
>> -#define XFS_MAX_EXTCNT_ATTR_FORK ((xfs_aextnum_t)0xffffffff) /* Unsigned 32-bits */
>> -#define XFS_MAX_EXTCNT_DATA_FORK_OLD ((xfs_extnum_t)0x7fffffff) /* Signed 32-bits */
>> -#define XFS_MAX_EXTCNT_ATTR_FORK_OLD ((xfs_aextnum_t)0x7fff) /* Signed 16-bits */
>> -
>> /*
>> * Inode minimum and maximum sizes.
>> */
>> @@ -1623,7 +1614,17 @@ typedef struct xfs_bmdr_block {
>> #define BMBT_BLOCKCOUNT_BITLEN 21
>>
>> #define BMBT_STARTOFF_MASK ((1ULL << BMBT_STARTOFF_BITLEN) - 1)
>> -#define BMBT_BLOCKCOUNT_MASK ((1ULL << BMBT_BLOCKCOUNT_BITLEN) - 1)
>> +
>> +/*
>> + * Max values for extlen and disk inode's extent counters.
>
> Nit: 'ondisk inode'
>
>
>> + */
>> +#define XFS_MAX_BMBT_EXTLEN ((xfs_extlen_t)(1ULL << BMBT_BLOCKCOUNT_BITLEN) - 1)
>> +#define XFS_MAX_EXTCNT_DATA_FORK ((xfs_extnum_t)0xffffffffffff) /* Unsigned 48-bits */
>> +#define XFS_MAX_EXTCNT_ATTR_FORK ((xfs_aextnum_t)0xffffffff) /* Unsigned 32-bits */
>> +#define XFS_MAX_EXTCNT_DATA_FORK_OLD ((xfs_extnum_t)0x7fffffff) /* Signed 32-bits */
>> +#define XFS_MAX_EXTCNT_ATTR_FORK_OLD ((xfs_aextnum_t)0x7fff) /* Signed 16-bits */
>> +
>> +#define BMBT_BLOCKCOUNT_MASK XFS_MAX_BMBT_EXTLEN
>
> Would this be simpler if XFS_MAX_EXTCNT* stay where they are, and only
> XFS_MAX_BMBT_EXTLEN moves down to be defined as an alias of
> BMBT_BLOCKCOUNT_MASK?
>
Yes, I think so. Also, all the *BMBT* macros defined around the same place
will probably help make the organization better.
--
chandan
