Re: [PATCH RFC 03/10] iomap: Support CoW-based atomic writes

John Garry
2025-02-06T11:21:42Z

>> if (iomi.pos >= dio->i_size ||
>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
>> index 75bf54e76f3b..0a0b6798f517 100644
>> --- a/include/linux/iomap.h
>> +++ b/include/linux/iomap.h
>> @@ -56,6 +56,8 @@ struct vm_fault;
>> *
>> * IOMAP_F_BOUNDARY indicates that I/O and I/O completions for this iomap must
>> * never be merged with the mapping before it.
>> + *
>> + * IOMAP_F_ATOMIC_COW indicates that we require atomic CoW end IO handling.
>
> It more indicates that the filesystem is using copy on write to handle
> an untorn write, and will provide the ioend support necessary to commit
> the remapping atomically, right?

yes, correct

>
>> */
>> #define IOMAP_F_NEW (1U << 0)
>> #define IOMAP_F_DIRTY (1U << 1)
>> @@ -68,6 +70,7 @@ struct vm_fault;
>> #endif /* CONFIG_BUFFER_HEAD */
>> #define IOMAP_F_XATTR (1U << 5)
>> #define IOMAP_F_BOUNDARY (1U << 6)
>> +#define IOMAP_F_ATOMIC_COW (1U << 7)
>>
>> /*
>> * Flags set by the core iomap code during operations:
>> @@ -183,6 +186,7 @@ struct iomap_folio_ops {
>> #define IOMAP_DAX 0
>> #endif /* CONFIG_FS_DAX */
>> #define IOMAP_ATOMIC (1 << 9)
>> +#define IOMAP_ATOMIC_COW (1 << 10)
>
> What does IOMAP_ATOMIC_COW do? There's no description for it (or for
> IOMAP_ATOMIC).

I'll add a description for both.

> Can you have IOMAP_ATOMIC and IOMAP_ATOMIC_COW both set?

Yes

> Or are they mutually exclusive?

I am not thinking that it might be neater to have a distinct flag for
IOMAP_ATOMIC when we want to try an atomic bio - maybe IOMAP_ATOMIC_HW
or IOMAP_ATOMIC_BIO? And then also IOMAP_DIO_ATOMIC_BIO (in addition to
IOMAP_DIO_ATOMIC_COW).

>
> I'm guessing from the code that ATOMIC_COW requires ATOMIC to be set,
> but I wonder why because there's no documentation update in the header
> files or in Documentation/filesystems/iomap/.

Will do.

Thanks,
John