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
