On Mon, Mar 10, 2025 at 06:39:41PM +0000, John Garry wrote:
> In cases of an atomic write occurs for misaligned or discontiguous disk
> blocks, we will use a CoW-based method to issue the atomic write.
>
> So, for that case, return -EAGAIN to request that the write be issued in
> CoW atomic write mode. The dio write path should detect this, similar to
> how misaligned regular DIO writes are handled.
How is -EAGAIN going to work here given that it is also used to defer
non-blocking requests to the caller blocking context?
What is the probem with only setting the flag that causes REQ_ATOMIC
to be set from the file system instead of forcing it when calling
iomap_dio_rw?
Also how you ensure this -EAGAIN only happens on the first extent
mapped and you doesn't cause double writes?
> + bool atomic_hw = flags & IOMAP_ATOMIC_HW;
Again, atomic_hw is not a very useful variable name. But the
whole idea of using a non-descriptive bool variable for a flags
field feels like an antipattern to me.
> - if (shared)
> + if (shared) {
> + if (atomic_hw &&
> + !xfs_bmap_valid_for_atomic_write(&cmap,
> + offset_fsb, end_fsb)) {
> + error = -EAGAIN;
> + goto out_unlock;
> + }
> goto out_found_cow;
This needs a big fat comment explaining why bailing out here is
fine and how it works.
> + /*
> + * Use CoW method for when we need to alloc > 1 block,
> + * otherwise we might allocate less than what we need here and
> + * have multiple mappings.
> + */
Describe why this is done, not just what is done.
