Re: [PATCH RFC v3 08/17] fuse: {uring} Handle SQEs - register commands

[Jens Axboe <axboe@kernel.dk>]

On 9/1/24 7:37 AM, Bernd Schubert wrote:
> +/**
> + * Entry function from io_uring to handle the given passthrough command
> + * (op cocde IORING_OP_URING_CMD)
> + */
> +int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
> +{
> +	const struct fuse_uring_cmd_req *cmd_req = io_uring_sqe_cmd(cmd->sqe);
> +	struct fuse_dev *fud;
> +	struct fuse_conn *fc;
> +	struct fuse_ring *ring;
> +	struct fuse_ring_queue *queue;
> +	struct fuse_ring_ent *ring_ent = NULL;
> +	u32 cmd_op = cmd->cmd_op;
> +	int ret = 0;
> +
> +	ret = -ENODEV;
> +	fud = fuse_get_dev(cmd->file);
> +	if (!fud)
> +		goto out;
> +	fc = fud->fc;
> +
> +	ring = fc->ring;
> +	if (!ring)
> +		goto out;
> +
> +	queue = fud->ring_q;
> +	if (!queue)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	if (queue->qid != cmd_req->qid)
> +		goto out;
> +
> +	ret = -ERANGE;
> +	if (cmd_req->tag > ring->queue_depth)
> +		goto out;
> +
> +	ring_ent = &queue->ring_ent[cmd_req->tag];
> +
> +	pr_devel("%s:%d received: cmd op %d qid %d (%p) tag %d  (%p)\n",
> +		 __func__, __LINE__, cmd_op, cmd_req->qid, queue, cmd_req->tag,
> +		 ring_ent);
> +
> +	spin_lock(&queue->lock);
> +	ret = -ENOTCONN;
> +	if (unlikely(fc->aborted || queue->stopped))
> +		goto err_unlock;
> +
> +	switch (cmd_op) {
> +	case FUSE_URING_REQ_FETCH:
> +		ret = fuse_uring_fetch(ring_ent, cmd, issue_flags);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		pr_devel("Unknown uring command %d", cmd_op);
> +		goto err_unlock;
> +	}
> +out:
> +	pr_devel("uring cmd op=%d, qid=%d tag=%d ret=%d\n", cmd_op,
> +		 cmd_req->qid, cmd_req->tag, ret);
> +
> +	if (ret < 0) {
> +		if (ring_ent != NULL) {
> +			pr_info_ratelimited("error: uring cmd op=%d, qid=%d tag=%d ret=%d\n",
> +					    cmd_op, cmd_req->qid, cmd_req->tag,
> +					    ret);
> +
> +			/* must not change the entry state, as userspace
> +			 * might have sent random data, but valid requests
> +			 * might be registered already - don't confuse those.
> +			 */
> +		}
> +		io_uring_cmd_done(cmd, ret, 0, issue_flags);
> +	}
> +
> +	return -EIOCBQUEUED;
> +
> +err_unlock:
> +	spin_unlock(&queue->lock);
> +	goto out;
> +}

Just a minor thing, but you should be able to just return an error from
here, at least for commands where you don't yet have teardown associated
with it, rather than punting through task_work for that too. Doesn't
really matter and maybe it's cleaner to just keep it the same.

-- 
Jens Axboe