Re: [PATCH v3 RESEND] io_uring: add timeout support for io_uring_enter()

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

On 04/11/2020 19:34, Jens Axboe wrote:
> On 11/4/20 12:27 PM, Pavel Begunkov wrote:
>> On 04/11/2020 18:32, Jens Axboe wrote:
>>> On 11/4/20 10:50 AM, Jens Axboe wrote:
>>>> +struct io_uring_getevents_arg {
>>>> +	sigset_t *sigmask;
>>>> +	struct __kernel_timespec *ts;
>>>> +};
>>>> +
>>>
>>> I missed that this is still not right, I did bring it up in your last
>>> posting though - you can't have pointers as a user API, since the size
>>> of the pointer will vary depending on whether this is a 32-bit or 64-bit
>>> arch (or 32-bit app running on 64-bit kernel).
>>
>> Maybe it would be better 
>>
>> 1) to kill this extra indirection?
>>
>> struct io_uring_getevents_arg {
>> -	sigset_t *sigmask;
>> -	struct __kernel_timespec *ts;
>> +	sigset_t sigmask;
>> +	struct __kernel_timespec ts;
>> };
>>
>> then,
>>
>> sigset_t *sig = (...)arg;
>> __kernel_timespec* ts = (...)(arg + offset);
> 
> But then it's kind of hard to know which, if any, of them are set... I
> did think about this, and any solution seemed worse than just having the
> extra indirection.

struct io_uring_getevents_arg {
	sigset_t sigmask;
	u32 mask;
	struct __kernel_timespec ts;
};

if size > sizeof(sigmask), then use mask to determine that.
Though, not sure how horrid the rest of the code would be.

> 
> Yeah, not doing the extra indirection would save a copy, but don't think
> it's worth it for this path.

I much more don't like branching like IORING_ENTER_GETEVENTS_TIMEOUT, from
conceptual point. I may try it out to see how it looks like while it's still
for-next.

-- 
Pavel Begunkov