Re: [PATCH V4 2/2] io_uring: cancelable uring_cmd
From: Jens Axboe <axboe@kernel.dk>
Date: 2023-09-28 08:38:41
Also in:
io-uring
On 9/22/23 8:50 PM, Ming Lei wrote:
quoted hunk ↗ jump to hunk
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h index ae08d6f66e62..a0307289bdc7 100644 --- a/include/linux/io_uring.h +++ b/include/linux/io_uring.h@@ -20,9 +20,13 @@ enum io_uring_cmd_flags { IO_URING_F_SQE128 = (1 << 8), IO_URING_F_CQE32 = (1 << 9), IO_URING_F_IOPOLL = (1 << 10), + + /* set when uring wants to cancel one issued command */ + IO_URING_F_CANCEL = (1 << 11), };
I'd make that comment: /* set when uring wants to cancel a previously issued command */
quoted hunk ↗ jump to hunk
@@ -125,6 +132,15 @@ static inline int io_uring_cmd_sock(struct io_uring_cmd *cmd, { return -EOPNOTSUPP; } +static inline int io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd, + unsigned int issue_flags) +{ + return -EOPNOTSUPP; +}
Do we need this to return an error? Presumably this will never get called if IO_URING isn't defined, but if it does, it obviously doesn't need to do anything anyway. Seems like it should just be a void, and ditto for the enabled version which can't return an error anyway.
return ret;
}
+static bool io_uring_try_cancel_uring_cmd(struct io_ring_ctx *ctx,
+ struct task_struct *task, bool cancel_all)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ bool ret = false;
+
+ lockdep_assert_held(&ctx->uring_lock);
+
+ hlist_for_each_entry_safe(req, tmp, &ctx->cancelable_uring_cmd,
+ hash_node) {
+ struct io_uring_cmd *cmd = io_kiocb_to_cmd(req,
+ struct io_uring_cmd);
+ struct file *file = req->file;
+
+ if (WARN_ON_ONCE(!file->f_op->uring_cmd))
+ continue;That check belongs in the function that marks it cancelable and adds it to the list. Outside of those minor nits, looks fine to me, and patch 1 does too. -- Jens Axboe