Re: [PATCH v2 1/2] splice: add splice(2) helpers

[Pavel Begunkov <asml.silence@gmail.com>]

On 2/17/2020 6:48 PM, Stefan Metzmacher wrote:
> Hi Pavel,
> 
>> diff --git a/src/include/liburing.h b/src/include/liburing.h
>> index 8ca6cd9..9ff2a54 100644
>> --- a/src/include/liburing.h
>> +++ b/src/include/liburing.h
>> @@ -191,6 +191,17 @@ static inline void io_uring_prep_rw(int op, struct io_uring_sqe *sqe, int fd,
>>  	sqe->__pad2[0] = sqe->__pad2[1] = sqe->__pad2[2] = 0;
>>  }
>>  
>> +static inline void io_uring_prep_splice(struct io_uring_sqe *sqe,
>> +					int fd_in, loff_t off_in,
>> +					int fd_out, loff_t off_out,
>> +					unsigned int nbytes, int splice_flags)
>> +{
> 
> The splice() syscall takes 'size_t len' and 'unsigned int flags',

sqe->len is u32, so can't fit size_t there, it was discussed before.

> I think we should at least change this to 'unsigned int splice_flags'.
> 

Good point, for the comments below as well. I'll resend.

>> diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
>> index 424fb4b..dc78697 100644
>> --- a/src/include/liburing/io_uring.h
>> +++ b/src/include/liburing/io_uring.h
>> @@ -23,7 +23,10 @@ struct io_uring_sqe {
>>  		__u64	off;	/* offset into file */
>>  		__u64	addr2;
>>  	};
>> -	__u64	addr;		/* pointer to buffer or iovecs */
>> +	union {
>> +		__u64	addr;	/* pointer to buffer or iovecs */
>> +		__u64	off_in;
>> +	};
>>  	__u32	len;		/* buffer size or number of iovecs */
>>  	union {
>>  		__kernel_rwf_t	rw_flags;
>> @@ -37,6 +40,7 @@ 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 {
>> @@ -45,6 +49,7 @@ struct io_uring_sqe {
>>  			__u16	buf_index;
>>  			/* personality to use, if used */
>>  			__u16	personality;
>> +			__u32	splice_fd_in;
> 
> Shouldn't this be __s32 in order to match "__s32 fd"?
> 
>>  		};
> 
> Can you please add BUILD_BUG_SQE_ELEM() checks for the new elements
> in io_uring_init()?
> 
> Thanks!
> metze
> 

-- 
Pavel Begunkov