Re: [PATCH 13/16] io_uring: add support for pre-mapped user IO buffers

On 1/16/19 8:41 AM, Arnd Bergmann wrote:

On Wed, Jan 16, 2019 at 4:32 PM Jens Axboe [off-list ref] wrote:

quoted

On 1/16/19 8:14 AM, Jens Axboe wrote:

quoted

On 1/16/19 3:53 AM, Arnd Bergmann wrote:

quoted

On Tue, Jan 15, 2019 at 3:56 AM Jens Axboe [off-list ref] wrote:

quoted

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 542757a4c898..e36c264d74e8 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h

@@ -314,6 +314,8 @@ asmlinkage long sys_io_uring_setup(u32 entries,
                                struct io_uring_params __user *p);
 asmlinkage long sys_io_uring_enter(unsigned int fd, u32 to_submit,
                                u32 min_complete, u32 flags);
+asmlinkage long sys_io_uring_register(unsigned int fd, unsigned op,
+                               void __user *arg);

Would it be possible to make this a typed pointer instead? If this needs to
be extended later to pass a different structure, a new system call may
be better for consistency than overloading the argument in various
ways.

As you can see from the later patch for registering files, it'll be used
for other structs too. Feels a little silly to add an extra system call
for that. I agree the void * isn't the prettiest thing in the world, but
at least it allows us to extend the API without having to add even more
system calls down the line.

With the __u64 changes, we end up with this:

struct io_uring_register_buffers {
        __u64 iovecs;           /* pointer to iovecs array */
        __u32 nr_iovecs;        /* number of iovecs in array */
        __u32 pad;
};

struct io_uring_register_files {
        __u64 fds;
        __u32 nr_fds;
        __u32 pad;
};

which are identical. So the question then becomes if I should just make
these opaque enough to be the same thing, ala:

struct io_uring_register_data {
        __u64 data;
        __u32 nr_elems;
        __u32 pad;
};

Right, that looks good in either form.

quoted

and then probably add a bit more reserved space so we have something
that can be extended...

Or maybe go the opposite way and pass the two members you have
directly to the system call:

int io_uring_register(unsigned int fd, unsigned int opcode, void
__user *, arg, unsigned count)
{
      ...
}

Where 'arg' now points to the array of iovecs or the the array of file
descriptors, or whatever else you need.

I kind of like that, that gets rid of having to wrap it in a struct. If
I later wanted to abuse it, arg could point to a struct...

I'll make this change.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>