Re: [PATCH V4 13/16] xfs: Conditionally upgrade existing inodes to use 64-bit extent counters

[Chandan Babu R <chandan.babu@oracle.com>]

On 05 Jan 2022 at 05:48, Darrick J. Wong wrote:
> On Tue, Dec 14, 2021 at 02:15:16PM +0530, Chandan Babu R wrote:
>> This commit upgrades inodes to use 64-bit extent counters when they are read
>> from disk. Inodes are upgraded only when the filesystem instance has
>> XFS_SB_FEAT_INCOMPAT_NREXT64 incompat flag set.
>> 
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>>  fs/xfs/libxfs/xfs_inode_buf.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
>> index fe21e9808f80..b8e4e1f69989 100644
>> --- a/fs/xfs/libxfs/xfs_inode_buf.c
>> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
>> @@ -253,6 +253,12 @@ xfs_inode_from_disk(
>>  	}
>>  	if (xfs_is_reflink_inode(ip))
>>  		xfs_ifork_init_cow(ip);
>> +
>> +	if ((from->di_version == 3) &&
>> +		xfs_has_nrext64(ip->i_mount) &&
>> +		!xfs_dinode_has_nrext64(from))
>> +		ip->i_diflags2 |= XFS_DIFLAG2_NREXT64;
>
> The indentation levels of the if test should not be aligned with the if
> body, and this should be in xfs_trans_log_inode so that the metadata
> update is staged properly with a transaction.  V3 did it this way, so
> I'm a little surprised to see V4 regressing that...?

The following is the thought process behind upgrading an inode to
XFS_DIFLAG2_NREXT64 when it is read from the disk,

1. With support for dynamic upgrade, The extent count limits of an inode needs
   to be determined by checking flags present within the inode i.e. we need to
   satisfy self-describing metadata property. This helps tools like xfs_repair
   and scrub to verify inode's extent count limits without having to refer to
   other metadata objects (e.g. superblock feature flags).
   
2. Upgrade when performed inside xfs_trans_log_inode() may cause
   xfs_iext_count_may_overflow() to return -EFBIG when the inode's data/attr
   extent count is already close to 2^31/2^15 respectively. Hence none of the
   file operations will be able to add new extents to a file.

-- 
chandan