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);
It'd spare us from IORING_ENTER_GETEVENTS_TIMEOUT but we'd need
to find a way to disable some of them. E.g. don't use sigmask when
user don't want it, but sigsz == sizeof(io_uring_getevents_arg),
and parsing would look like
switch (argsz) {
case sizeof(struct io_uring_getevents_arg): {
struct __kernel_timespec ts = argp + ts_offset;
...
}
fallthrough;
case sizeof(sig): {
const sigset_t __user *sig = argp;
...
break;
}
default:
return -EINVAL;
}
2) and move all the parsing into io_cqring_wait(). That sounds better
performance-wise.
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 7e6945383907..2f533f6815ea 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -9158,8 +9158,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
> return -EINVAL;
> if (copy_from_user(&arg, argp, sizeof(arg)))
> return -EFAULT;
> - sig = arg.sigmask;
> - ts = arg.ts;
> + sig = u64_to_user_ptr(arg.sigmask);
> + ts = u64_to_user_ptr(arg.ts);
> } else {
> sig = (const sigset_t __user *)argp;
> ts = NULL;
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index fefee28c3ed8..0b104891df68 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -338,8 +338,8 @@ enum {
> };
>
> struct io_uring_getevents_arg {
> - sigset_t *sigmask;
> - struct __kernel_timespec *ts;
> + __u64 sigmask;
> + __u64 ts;
> };
>
> #endif
>
--
Pavel Begunkov
