On 07 Apr 2022 at 07:18, Darrick J. Wong wrote:
> On Thu, Apr 07, 2022 at 11:13:11AM +1000, Dave Chinner wrote:
>> On Wed, Apr 06, 2022 at 11:48:59AM +0530, Chandan Babu R wrote:
>> > The maximum file size that can be represented by the data fork extent counter
>> > in the worst case occurs when all extents are 1 block in length and each block
>> > is 1KB in size.
>> >
>> > With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count and with
>> > 1KB sized blocks, a file can reach upto,
>> > (2^31) * 1KB = 2TB
>> >
>> > This is much larger than the theoretical maximum size of a directory
>> > i.e. XFS_DIR2_SPACE_SIZE * 3 = ~96GB.
>> >
>> > Since a directory's inode can never overflow its data fork extent counter,
>> > this commit removes all the overflow checks associated with
>> > it. xfs_dinode_verify() now performs a rough check to verify if a diretory's
>> > data fork is larger than 96GB.
>> >
>> > Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>>
>> Mostly OK, just a simple cleanup needed.
>>
>> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
>> > index ee8d4eb7d048..54b106ae77e1 100644
>> > --- a/fs/xfs/libxfs/xfs_inode_buf.c
>> > +++ b/fs/xfs/libxfs/xfs_inode_buf.c
>> > @@ -491,6 +491,15 @@ xfs_dinode_verify(
>> > if (mode && nextents + naextents > nblocks)
>> > return __this_address;
>> >
>> > + if (S_ISDIR(mode)) {
>> > + uint64_t max_dfork_nexts;
>> > +
>> > + max_dfork_nexts = (XFS_DIR2_MAX_SPACES * XFS_DIR2_SPACE_SIZE) >>
>> > + mp->m_sb.sb_blocklog;
>> > + if (nextents > max_dfork_nexts)
>> > + return __this_address;
>> > + }
>>
>> max_dfork_nexts for a directory is a constant that should be
>> calculated at mount time via xfs_da_mount() and stored in the
>> mp->m_dir_geo structure. Then this code simple becomes:
>>
>> if (S_ISDIR(mode) && nextents > mp->m_dir_geo->max_extents)
>> return __this_address;
>
> I have the same comment as Dave, FWIW. :)
>
Ok. I will apply the above suggestion.
--
chandan
