Re: [PATCH v6 10/13] xfs: iomap COW-based atomic write support

John Garry
2025-03-20T09:50:03Z

On 20/03/2025 05:29, Christoph Hellwig wrote:
> On Wed, Mar 19, 2025 at 10:24:55AM +0000, John Garry wrote:
>> it seems to work ok, cheers
>
> Better test it very well, this was really just intended as a sketch..

Sure, I have been testing a lot so far.

I had been using fio in verify mode as a method to check racing threads
reading and atomically writing the same file range, so I need to ensure
that it covers the various paths in this function.

>
>>> + count_fsb = end_fsb - offset_fsb;
>>> + resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
>>> + xfs_get_cowextsz_hint(ip));
>>> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
>>> +
>>> + error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
>>> + XFS_DIOSTRAT_SPACE_RES(mp, resaligned), 0, false, &tp);
>>> if (error)
>>> return error;
>>> - error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
>>> - &nimaps, 0);
>>> - if (error)
>>> - goto out_unlock;
>>> + if (!xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &cmap))
>>> + cmap.br_startoff = end_fsb;
>>
>> Do we really need this logic?
>>
>> offset_fsb does not change, and logically cmap.br_startoff == end_fsb
>> already, right?
>
> Afte unlocking and relocking the ilock the extent layout could have
> changed.

ok, understood. Maybe a comment will help understanding that.

>