On 12/12/2023 16:32, Christoph Hellwig wrote:
On Tue, Dec 12, 2023 at 11:08:28AM +0000, John Garry wrote:
Two new fields are added to struct statx - atomic_write_unit_min and
atomic_write_unit_max. For each atomic individual write, the total length
of a write must be a between atomic_write_unit_min and
atomic_write_unit_max, inclusive, and a power-of-2. The write must also be
at a natural offset in the file wrt the write length.
SCSI sd.c and scsi_debug and NVMe kernel support is added.
Some open questions:
- How to make API extensible for when we have no HW support? In that case,
we would prob not have to follow rule of power-of-2 length et al.
As a possible solution, maybe we can say that atomic writes are
supported for the file via statx, but not set unit_min and max values,
and this means that writes need to be just FS block aligned there.
I don't think the power of two length is much of a problem to be
honest, and if we every want to lift it we can still do that easily
by adding a new flag or limit.
ok, but it would be nice to have some idea on what that flag or limit
change would be.
What I'm a lot more worried about is how to tell the file system that
allocations are done right for these requirement. There is no way
a user can know that allocations in an existing file are properly
aligned, so atomic writes will just fail on existing files.
I suspect we need an on-disk flag that forces allocations to be
aligned to the atomic write limit, in some ways similar how the
XFS rt flag works. You'd need to set it on an empty file, and all
allocations after that are guaranteed to be properly aligned.
Hmmm... so how is this different to the XFS forcealign feature?
For XFS, I thought that your idea was to always CoW new extents for
misaligned extents or writes which spanned multiple extents.
- For block layer, should atomic_write_unit_max be limited by
max_sectors_kb? Currently it is not.
Well. It must be limited to max_hw_sectors to actually work.
Sure, as max_hw_sectors may also be limited by DMA controller max
mapping size.
max_sectors is a software limit below that, which with modern hardware
is actually pretty silly and a real performance issue with todays
workloads when people don't tweak it..
Right, so we should limit atomic write queue limits to max_hw_sectors.
But people can still tweak max_sectors, and I am inclined to say that
atomic_write_unit_max et al should be (dynamically) limited to
max_sectors also.
- How to improve requirement that iovecs are PAGE-aligned.
There are 2x issues:
a. We impose this rule to not split BIOs due to virt boundary for
NVMe, but there virt boundary is 4K (and not PAGE size, so broken for
16K/64K pages). Easy solution is to impose requirement that iovecs
are 4K-aligned.
b. We don't enforce this rule for virt boundary == 0, i.e. SCSI
.. we require any device that wants to support atomic writes to not
have that silly limit. For NVMe that would require SGL support
(and some driver changes I've been wanting to make for long where
we always use SGLs for transfers larger than a single PRP if supported)
If we could avoid dealing with a virt boundary, then that would be nice.
Are there any patches yet for the change to always use SGLs for
transfers larger than a single PRP?
On a related topic, I am not sure about how - or if we even should -
enforce iovec PAGE-alignment or length; rather, the user could just be
advised that iovecs must be PAGE-aligned and min PAGE length to achieve
atomic_write_unit_max.
- Since debugging torn-writes due to unwanted kernel BIO splitting/merging
would be horrible, should we add some kernel storage stack software
integrity checks?
Yes, I think we'll need asserts in the drivers. At least for NVMe I
will insist on them.
Please see 16/16 in this series.
For SCSI I think the device actually checks
because the atomic writes are a different command anyway, or am I
misunderstanding how SCSI works here?
Right, for WRITE ATOMIC (16) the target will reject a command when it
does not respect the device atomic write limits.
Thanks,
John
