Re: [PATCH RFC 02/10] xfs: Refactor xfs_reflink_end_cow_extent()

John Garry
2025-02-06T10:35:43Z

On 05/02/2025 19:50, Darrick J. Wong wrote:
>> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
>> index 59f7fc16eb80..580469668334 100644
>> --- a/fs/xfs/xfs_reflink.c
>> +++ b/fs/xfs/xfs_reflink.c
>> @@ -786,35 +786,20 @@ xfs_reflink_update_quota(
>> * requirements as low as possible.
>> */
>> STATIC int
>> -xfs_reflink_end_cow_extent(
>> +xfs_reflink_end_cow_extent_locked(
>> struct xfs_inode *ip,
>> xfs_fileoff_t *offset_fsb,
>> - xfs_fileoff_t end_fsb)
>> + xfs_fileoff_t end_fsb,
>> + struct xfs_trans *tp,
>> + bool *commit)
> Transactions usually come before the inode in the parameter list.

ok

>
> You don't need to pass out a @commit flag -- if the function returns
> nonzero then the caller has to cancel the transaction; otherwise it will
> return zero and the caller should commit it.> There's no penalty for
> committing a non-dirty transaction.

If there is no penalty, then fine. But I don't feel totally comfortable
with this and would prefer to keep the same behavior.

Thanks,
John