Re: [PATCH v4 0/5] btrfs: io_uring interface for encoded reads

[Mark Harmstone <maharmstone@meta.com>]

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