On Wed, Mar 17, 2021 at 04:10:23PM -0600, Jens Axboe wrote:
> +/*
> + * Called by consumers of io_uring_cmd, if they originally returned
> + * -EIOCBQUEUED upon receiving the command.
> + */
> +void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret)
> +{
> + struct io_kiocb *req = container_of(cmd, struct io_kiocb, uring_cmd);
> +
> + if (ret < 0)
> + req_set_fail_links(req);
> + io_req_complete(req, ret);
> +}
> +EXPORT_SYMBOL(io_uring_cmd_done);
This really should be EXPORT_SYMBOL_GPL. But more importantly I'm not
sure it is an all that useful interface. All useful non-trivial ioctls
tend to access user memory, so something that queues up work in the task
context like in Joshis patch should really be part of the documented
interface.
> +
> +static int io_uring_cmd_prep(struct io_kiocb *req,
> + const struct io_uring_sqe *sqe)
> +{
> + const struct io_uring_cmd_sqe *csqe = (const void *) sqe;
We really should not need this casting. The struct io_uring_sqe
usage in io_uring.c needs to be replaced with a union or some other
properly type safe way to handle this.
> + ret = file->f_op->uring_cmd(&req->uring_cmd, issue_flags);
> + /* queued async, consumer will call io_uring_cmd_done() when complete */
> + if (ret == -EIOCBQUEUED)
> + return 0;
> + io_uring_cmd_done(&req->uring_cmd, ret);
> + return 0;
This can be simplified to:
if (ret != -EIOCBQUEUED)
io_uring_cmd_done(&req->uring_cmd, ret);
return 0;
> +/*
> + * Note that the first member here must be a struct file, as the
> + * io_uring command layout depends on that.
> + */
> +struct io_uring_cmd {
> + struct file *file;
> + __u16 op;
> + __u16 unused;
> + __u32 len;
> + __u64 pdu[5]; /* 40 bytes available inline for free use */
> +};
I am a little worried about exposting this internal structure to random
drivers. OTOH we need something that eventually allows a container_of
to io_kiocb for the completion, so I can't think of anything better
at the moment either.
