On 22/01/2020 05:47, Jens Axboe wrote:
> 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.
>
Agree, it looks bad. I don't want to add it into sqe->splice_flags to not clash
with splice(2) in the future, but could have a separate field in @sqe...
or can leave in in sqe->flags, as it's done in the patch, but that's like a
portion of bits would be opcode specific and we would need to set rules for
their use.
--
Pavel Begunkov
Attachment:
signature.asc
Description: OpenPGP digital signature
