| Message ID | 20241022145024.1046883-1-maharmstone@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: io_uring interface for encoded reads | expand |
On Tue, Oct 22, 2024 at 03:50:15PM +0100, Mark Harmstone wrote: > This is version 4 of a patch series to add an io_uring interface for > encoded reads. The principal use case for this is to eventually allow > btrfs send and receive to operate asynchronously, the lack of io_uring > encoded I/O being one of the main blockers for this. > > I've written a test program for this, which demonstrates the ioctl and > io_uring interface produce identical results: https://github.com/maharmstone/io_uring-encoded We'll need a test utility for fstests too. > Changelog: > v4: > * Rewritten to avoid taking function pointer > * Removed nowait parameter, as this could be derived from iocb flags > * Fixed structure not getting properly initialized > * Followed ioctl by capping uncompressed reads at EOF > * Rebased against btrfs/for-next > * Formatting fixes > * Rearranged structs to minimize holes > * Published test program > * Fixed potential data race with userspace > * Changed to use io_uring_cmd_to_pdu helper function > * Added comments for potentially confusing parts of the code There are some more style issues and changelog updates but overall looks ok to me. We're now in rc5 so we need to add it to for-next now or postpone for next cycle (but I don't see a reason why). I've noticed CONFIG_IO_URING is a config option, do we need some ifdef protection or are the definitions provided unconditionally. We may also need to ifdef out the ioctl code if io_uring is not built in.
On 22/10/24 22:50, Mark Harmstone wrote: > This is version 4 of a patch series to add an io_uring interface for > encoded reads. The principal use case for this is to eventually allow > btrfs send and receive to operate asynchronously, How would you define an asynchronously operated Btrfs send and receive? Are you referring to Btrfs send and receive the leveraging io_uring asynchronous operation? > the lack of io_uring > encoded I/O being one of the main blockers for this. > > I've written a test program for this, which demonstrates the ioctl and > io_uring interface produce identical results: https://github.com/maharmstone/io_uring-encoded Thanks, Anand > > Changelog: > v4: > * Rewritten to avoid taking function pointer > * Removed nowait parameter, as this could be derived from iocb flags > * Fixed structure not getting properly initialized > * Followed ioctl by capping uncompressed reads at EOF > * Rebased against btrfs/for-next > * Formatting fixes > * Rearranged structs to minimize holes > * Published test program > * Fixed potential data race with userspace > * Changed to use io_uring_cmd_to_pdu helper function > * Added comments for potentially confusing parts of the code > > v3: > * Redo of previous versions > > Mark Harmstone (5): > btrfs: remove pointless addition in btrfs_encoded_read > btrfs: change btrfs_encoded_read so that reading of extent is done by > caller > btrfs: don't sleep in btrfs_encoded_read if IOCB_NOWAIT set > btrfs: move priv off stack in btrfs_encoded_read_regular_fill_pages > btrfs: add io_uring command for encoded reads > > fs/btrfs/btrfs_inode.h | 13 +- > fs/btrfs/file.c | 1 + > fs/btrfs/inode.c | 175 ++++++++++++++------- > fs/btrfs/ioctl.c | 342 ++++++++++++++++++++++++++++++++++++++++- > fs/btrfs/ioctl.h | 2 + > fs/btrfs/send.c | 3 +- > 6 files changed, 473 insertions(+), 63 deletions(-) >
On 30/10/24 01:22, Anand Jain wrote: > On 22/10/24 22:50, Mark Harmstone wrote: >> This is version 4 of a patch series to add an io_uring interface for >> encoded reads. The principal use case for this is to eventually allow >> btrfs send and receive to operate asynchronously, > > How would you define an asynchronously operated Btrfs send and receive? > Are you referring to Btrfs send and receive the leveraging io_uring > asynchronous operation? Yes. The ideal for btrfs receive would be if we could mmap the stream file, loop through the TLV entries translating them into io_uring SQEs, then wait at the end for completion. Mark
Thanks David. On 29/10/24 21:29, David Sterba wrote: > On Tue, Oct 22, 2024 at 03:50:15PM +0100, Mark Harmstone wrote: >> This is version 4 of a patch series to add an io_uring interface for >> encoded reads. The principal use case for this is to eventually allow >> btrfs send and receive to operate asynchronously, the lack of io_uring >> encoded I/O being one of the main blockers for this. >> >> I've written a test program for this, which demonstrates the ioctl and >> io_uring interface produce identical results: https://github.com/maharmstone/io_uring-encoded > > We'll need a test utility for fstests too. Yes, no problem. >> Changelog: >> v4: >> * Rewritten to avoid taking function pointer >> * Removed nowait parameter, as this could be derived from iocb flags >> * Fixed structure not getting properly initialized >> * Followed ioctl by capping uncompressed reads at EOF >> * Rebased against btrfs/for-next >> * Formatting fixes >> * Rearranged structs to minimize holes >> * Published test program >> * Fixed potential data race with userspace >> * Changed to use io_uring_cmd_to_pdu helper function >> * Added comments for potentially confusing parts of the code > > There are some more style issues and changelog updates but overall looks > ok to me. We're now in rc5 so we need to add it to for-next now or > postpone for next cycle (but I don't see a reason why). > > I've noticed CONFIG_IO_URING is a config option, do we need some ifdef > protection or are the definitions provided unconditionally. We may also > need to ifdef out the ioctl code if io_uring is not built in. It compiles fine with CONFIG_IO_URING=n, the uring_cmd field in struct file_operations is there unconditionally. FWIW, the other users of uring_cmd don't ifdef their bits out, presumably because it's also gated behind CONFIG_EXPERT. Mark
This is version 4 of a patch series to add an io_uring interface for encoded reads. The principal use case for this is to eventually allow btrfs send and receive to operate asynchronously, the lack of io_uring encoded I/O being one of the main blockers for this. I've written a test program for this, which demonstrates the ioctl and io_uring interface produce identical results: https://github.com/maharmstone/io_uring-encoded Changelog: v4: * Rewritten to avoid taking function pointer * Removed nowait parameter, as this could be derived from iocb flags * Fixed structure not getting properly initialized * Followed ioctl by capping uncompressed reads at EOF * Rebased against btrfs/for-next * Formatting fixes * Rearranged structs to minimize holes * Published test program * Fixed potential data race with userspace * Changed to use io_uring_cmd_to_pdu helper function * Added comments for potentially confusing parts of the code v3: * Redo of previous versions Mark Harmstone (5): btrfs: remove pointless addition in btrfs_encoded_read btrfs: change btrfs_encoded_read so that reading of extent is done by caller btrfs: don't sleep in btrfs_encoded_read if IOCB_NOWAIT set btrfs: move priv off stack in btrfs_encoded_read_regular_fill_pages btrfs: add io_uring command for encoded reads fs/btrfs/btrfs_inode.h | 13 +- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 175 ++++++++++++++------- fs/btrfs/ioctl.c | 342 ++++++++++++++++++++++++++++++++++++++++- fs/btrfs/ioctl.h | 2 + fs/btrfs/send.c | 3 +- 6 files changed, 473 insertions(+), 63 deletions(-)