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

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

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