On 9/10/24 15:20, Christoph Hellwig wrote:
On Tue, Sep 10, 2024 at 01:17:48PM +0100, Pavel Begunkov wrote:
Add a command that writes the zero page to the drive. Apart from passing
the zero page instead of actual data it uses the normal write path and
doesn't do any further acceleration, nor it requires any special
hardware support. The indended use is to have a fallback when
BLOCK_URING_CMD_WRITE_ZEROES is not supported.
That's just a horrible API. The user should not have to care if the
kernel is using different kinds of implementations.
It's rather not a good api when instead of issuing a presumably low
overhead fast command the user expects sending a good bunch of actual
writes with different performance characteristics.
The normal use case (at least the ones I've been involved with) are
simply zero these blocks or the entire device, and please do it as
good as you can. Needing asynchronous error handling in userspace
for that is extremely counter productive.
If we expect any error handling from the user space at all (we do),
it'll and have to be asynchronous, it's async commands and io_uring.
Asking the user to reissue a command in some form is normal.
In my experience,
such fallbacks cause more pain when a more explicit approach is
possible. And let me note that it's already exposed via fallocate, even
though in a bit different way.
Do you mean the FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE case in
blkdev_fallocate? As far as I can tell this is actually a really bad
example, as even a hardware offloaded write zeroes can and often does
write physical zeroes to the media, and does so from a firmware path
that is often slower than the kernel loop.
That's a shame, I agree, which is why I call it "presumably" faster,
but that actually gives more reasons why you might want this cmd
separately from write zeroes, considering the user might know
its hardware and the kernel doesn't try to choose which approach
faster.
But you have an actual use case where you want to send a write zeroes
command but never a loop of writes, it would be good to document that
and add a flag for it. And if we don't have that case it would still
Users who know more about hw and e.g. prefer writes with 0 page as
per above. Users with lots of devices who care about pcie / memory
bandwidth, there is enough of those, they might want to do
something different like adjusting algorithms and throttling.
Better/easier testing, though of lesser importance.
Those I made up just now on the spot, but the reporter did
specifically ask about some way to differentiate fallbacks.
be good to have a reserved flags field to add it later if needed.
if (unlikely(sqe->ioprio || sqe->__pad1 || sqe->len ||
sqe->rw_flags || sqe->file_index))
return -EINVAL;
There is a good bunch of sqe fields that can used for that later.
Btw, do you have API documentation (e.g. in the form of a man page)
for these new calls somewhere?
Mentioned in the cover:
tests and docs:
https://github.com/isilence/liburing.git discard-cmd
man page specifically:
https://github.com/isilence/liburing/commit/a6fa2bc2400bf7fcb80496e322b5db4c8b3191f0
I'll send them once the kernel is set in place.
--
Pavel Begunkov
