Re: [PATCH v3 1/3] xfs: add xfs_bmap_alloc_or_convert_range function

[Pankaj Raghav <pankaj.raghav@linux.dev>]

On 4/24/26 15:52, Christoph Hellwig wrote:
>> +/*
>> + * This function is used to allocate written extents over holes
>> + * and/or convert unwritten extents to written extents based on the
>> + * @flags passed to it.
> 
> You can and should use up all 80 characters for comments.
> 
>> + */
>> +int
>> +xfs_bmap_alloc_or_convert_range(
>> +	struct xfs_inode	*ip,
>> +	xfs_off_t		offset,
>> +	xfs_off_t		count,
>> +	uint32_t		flags,
>> +	bool			update_isize)
>> +{
> 
> Please keep this function in the same please as the generalized
> version it was base on so that git diff/show shows what changed.
> If you have to move it around for later patches do that in
> an otherwise no-op prep patch.
> 

So IIUC, you are suggesting to add this function in xfs_iomap.c itself and move it to
bmap_util.c in an another prep patch.

>> +		} else {
>> +			/*
>> +			 * We might allocate data blocks (needs resblks + 1 split) or
>> +			 * convert an unwritten extent (needs 0 data blocks + 2 splits).
>> +			 * Ensure we have enough block reservation for the worst case.
>> +			 */
>> +			resblks = XFS_FILBLKS_MIN(count_fsb, XFS_MAX_BMBT_EXTLEN);
> 
> Please avoid the overly long lines.
> 

I will change them all in the next version. Thanks for your comments.

--
Pankaj