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
