On 1/21/20 7:40 PM, Pavel Begunkov wrote:
>>> @@ -719,6 +730,11 @@ static const struct io_op_def io_op_defs[] = {
>>> .needs_file = 1,
>>> .fd_non_neg = 1,
>>> },
>>> + [IORING_OP_SPLICE] = {
>>> + .needs_file = 1,
>>> + .hash_reg_file = 1,
>>> + .unbound_nonreg_file = 1,
>>> + }
>>> };
>>>
>>> static void io_wq_submit_work(struct io_wq_work **workptr);
>>
>> I probably want to queue up a reservation for the EPOLL_CTL that I
>> haven't included yet, but which has been tested. But that's easily
>> manageable, so no biggy on my end.
>
> I didn't quite get it. Do you mean collision of opcode numbers?
Yeah that's all I meant, sorry wasn't too clear. But you can disregard,
I'll just pop a reservation in front if/when this is ready to go in if
it's before EPOLL_CTL op.
>>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>>> index 57d05cc5e271..f234b13e7ed3 100644
>>> --- a/include/uapi/linux/io_uring.h
>>> +++ b/include/uapi/linux/io_uring.h
>>> @@ -23,8 +23,14 @@ struct io_uring_sqe {
>>> __u64 off; /* offset into file */
>>> __u64 addr2;
>>> };
>>> - __u64 addr; /* pointer to buffer or iovecs */
>>> - __u32 len; /* buffer size or number of iovecs */
>>> + union {
>>> + __u64 addr; /* pointer to buffer or iovecs */
>>> + __u64 off_out;
>>> + };
>>> + union {
>>> + __u32 len; /* buffer size or number of iovecs */
>>> + __s32 fd_out;
>>> + };
>>> union {
>>> __kernel_rwf_t rw_flags;
>>> __u32 fsync_flags;
>>> @@ -37,10 +43,12 @@ struct io_uring_sqe {
>>> __u32 open_flags;
>>> __u32 statx_flags;
>>> __u32 fadvise_advice;
>>> + __u32 splice_flags;
>>> };
>>> __u64 user_data; /* data to be passed back at completion time */
>>> union {
>>> __u16 buf_index; /* index into fixed buffers, if used */
>>> + __u64 splice_len;
>>> __u64 __pad2[3];
>>> };
>>> };
>>
>> Not a huge fan of this, also mean splice can't ever used fixed buffers.
>> Hmm...
>
> But it's not like splice() ever uses user buffers. Isn't it? vmsplice
> does, but that's another opcode.
I guess that's true, I had vmsplice on my mind for this as well. But
won't be a problem there, since it doesn't take 6 arguments like splice
does.
Another option is to do an indirect for splice, stuff the arguments in a
struct that's passed in as a pointer in ->addr. A bit slower, but
probably not a huge deal.
>>> @@ -67,6 +75,9 @@ enum {
>>> /* always go async */
>>> #define IOSQE_ASYNC (1U << IOSQE_ASYNC_BIT)
>>>
>>> +/* op custom flags */
>>> +#define IOSQE_SPLICE_FIXED_OUT (1U << 16)
>>> +
>>
>> I don't think it's unreasonable to say that if you specify
>> IOSQE_FIXED_FILE, then both are fixed. If not, then none of them are.
>> What do you think?
>>
>
> It's plausible to register only one end for splicing, e.g. splice from
> short-lived sockets to pre-registered buffers-pipes. And it's clearer
> do it now.
You're probably right, though it's a bit nasty to add an unrelated flag
in the splice flag space... We should probably reserve it in splice
instead, and just not have it available from the regular system call.
--
Jens Axboe
