Re: [PATCH 0/7] fio: atomic write support

[John Garry <john.g.garry@oracle.com>]

On 10/09/2024 16:56, Jens Axboe wrote:
>>> t drop that patch.
> (side note - please wrap your email lines, I always re-wrap when
> replying)

ok

> 
>> Some background is that main selling point of atomic writes is that we
>> guarantee writes to storage will not be torn for a power failure or
>> kernel crash.
>>
>> Another aspect of atomic writes is that they handle racing writes and
>> reads, such that a read racing with a write will see all the data from
>> the write or none. Well, SCSI and NVMe guarantee this if using
>> RWF_ATOMIC, but it is not formally stated as a feature of RWF_ATOMIC.
>>
>> It can be argued that having racing reads and writes is an application
>> bug. Furthermore, as I understand, even if posix guarantees that
>> regular writes are "atomic", it is not the case generally.
>>
>> So one part of the relevance of atomic writes to fio verify is that we
>> can verify that atomic writes "safely" handle racing read and writes.
>> For this, the CRC checks would be successful if we have many jobs;
>> however header sequence numbers are not. Hence patch 4/7.
>>
>> I had also been using the verify feature to test atomic writes for
>> power failures. In this case, I run a single verify job with
>> --rw=write, power fail, and use verify in read mode to prove no
>> invalid data in the file, like:
>>
>> fio --filename=mnt/file --direct=1 --rw=read --bs=8k --iodepth=100 --na
>> me=iops --numjobs=1 --loops=1 --verify=crc64 --ioengine=libaio
>> --verify_fatal=1 --group_reporting  --exitall_on_error
>>
>> This power fail test is what I am mostly interested in.
>>
>> So my point is that the patch to ignore invalid headers could be
>> dropped, but let me know your thoughts.
> Gotcha, that makes sense. For atomic writes, it's totally fine to have
> overlapping writers if the write size is in the atomic units, but we can
> of course expect sequences to be out of order depending on which one
> makes it to stable storage. But you have no checks for whether or not
> the write size is within the atomic range?

I don't currently. If an atomic write size is out-of-range, the kernel 
will reject it with -EINVAL. However -EINVAL can be returned for a 
multitude of issues, so not much help. So I could add a statx call to 
get the limits and then error the fio config (if bs is out-of-range).

> 
> I think dropping patch 4 and adding a verify option to specifically
> ignore the sequence would make sense, leaving the control in the hands
> of the user. 

I already saw verify modes "no header" and "header only", so I was 
reluctant to add a potentially conflicting new separate option to ignore 
the header sequence. However, I can look to add a new verify option to 
ignore the header sequence and ensure it respects those mentioned verify 
modes.

> And then bonus points for adding an example job file (with
> comments) in your series that shows how to use atomic writes (and uses
> that option) would be useful.

ok, I'm happy to do that.

Thanks,
John