On Tue, Mar 18, 2025 at 08:22:53AM +0000, John Garry wrote:
>> Is xfs_reflink_allocate_cow even the right helper to use? We know we
>> absolutely want a a COW fork extent, we know there can't be delalloc
>> extent to convert as we flushed dirty data, so most of the logic in it
>> is pointless.
>
> Well xfs_reflink_allocate_cow gives us what we want when we set some flag
> (XFS_REFLINK_FORCE_COW).
>
> Are you hinting at a dedicated helper? Note that
> xfs_reflink_fill_cow_hole() also handles the XFS_REFLINK_FORCE_COW flag.
We might not even need much of a helper. This all comes from my
experience with the zoned code that always writes out of place as well.
I initially tried to reuse the existing iomap_begin methods and all
these helpers, but in the end it turned out to much, much simpler to
just open code the logic. Now the atomic cow code will be a little
more complex in some aspect (unwritten extents, speculative
preallocations), but also simpler in others (no need to support buffered
I/O including zeroing and sub-block writes that require the ugly
srcmap based read-modify-write), but I suspect the overall trade-off
will be similar.
After all the high-level logic for the atomic COW iomap_begin really
should just be:
- take the ilock
- check the COW fork if there is an extent for the start of the range.
- If yes:
- if the extent is unwritten, convert the part overlapping with
the range to written
- return the extent
- If no:
- allocate a new extent covering the range in the COW fork
Doing this without going down multiple levels of helpers designed for
an entirely different use case will probably simplify things
significantly.
