| Message ID | 20250225212456.2902549-1-csander@purestorage.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ublk: complete command synchronously on error | expand |
On Tue, Feb 25, 2025 at 02:24:55PM -0700, Caleb Sander Mateos wrote: > In case of an error, ublk's ->uring_cmd() functions currently return > -EIOCBQUEUED and immediately call io_uring_cmd_done(). -EIOCBQUEUED and > io_uring_cmd_done() are intended for asynchronous completions. For > synchronous completions, the ->uring_cmd() function can just return the > negative return code directly. This skips io_uring_cmd_del_cancelable(), > and deferring the completion to task work. So return the error code > directly from __ublk_ch_uring_cmd() and ublk_ctrl_uring_cmd(). > > Update ublk_ch_uring_cmd_cb(), which currently ignores the return value > from __ublk_ch_uring_cmd(), to call io_uring_cmd_done() for synchronous > completions. Makes sense, and looks good. Reviewed-by: Keith Busch <kbusch@kernel.org>
On Tue, Feb 25, 2025 at 02:24:55PM -0700, Caleb Sander Mateos wrote: > In case of an error, ublk's ->uring_cmd() functions currently return > -EIOCBQUEUED and immediately call io_uring_cmd_done(). -EIOCBQUEUED and > io_uring_cmd_done() are intended for asynchronous completions. For > synchronous completions, the ->uring_cmd() function can just return the > negative return code directly. This skips io_uring_cmd_del_cancelable(), > and deferring the completion to task work. So return the error code > directly from __ublk_ch_uring_cmd() and ublk_ctrl_uring_cmd(). > > Update ublk_ch_uring_cmd_cb(), which currently ignores the return value > from __ublk_ch_uring_cmd(), to call io_uring_cmd_done() for synchronous > completions. > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming
On Tue, 25 Feb 2025 14:24:55 -0700, Caleb Sander Mateos wrote: > In case of an error, ublk's ->uring_cmd() functions currently return > -EIOCBQUEUED and immediately call io_uring_cmd_done(). -EIOCBQUEUED and > io_uring_cmd_done() are intended for asynchronous completions. For > synchronous completions, the ->uring_cmd() function can just return the > negative return code directly. This skips io_uring_cmd_del_cancelable(), > and deferring the completion to task work. So return the error code > directly from __ublk_ch_uring_cmd() and ublk_ctrl_uring_cmd(). > > [...] Applied, thanks! [1/1] ublk: complete command synchronously on error commit: 6376ef2b6af3bbcb7c50dc657bdfb83aba467aef Best regards,
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 529085181f35..ff648c6839c1 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -1864,14 +1864,13 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd, } ublk_prep_cancel(cmd, issue_flags, ubq, tag); return -EIOCBQUEUED; out: - io_uring_cmd_done(cmd, ret, 0, issue_flags); pr_devel("%s: complete: cmd op %d, tag %d ret %x io_flags %x\n", __func__, cmd_op, tag, ret, io->flags); - return -EIOCBQUEUED; + return ret; } static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub, struct ublk_queue *ubq, int tag, size_t offset) { @@ -1923,11 +1922,14 @@ static inline int ublk_ch_uring_cmd_local(struct io_uring_cmd *cmd, } static void ublk_ch_uring_cmd_cb(struct io_uring_cmd *cmd, unsigned int issue_flags) { - ublk_ch_uring_cmd_local(cmd, issue_flags); + int ret = ublk_ch_uring_cmd_local(cmd, issue_flags); + + if (ret != -EIOCBQUEUED) + io_uring_cmd_done(cmd, ret, 0, issue_flags); } static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags) { if (unlikely(issue_flags & IO_URING_F_CANCEL)) { @@ -3054,14 +3056,13 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, put_dev: if (ub) ublk_put_device(ub); out: - io_uring_cmd_done(cmd, ret, 0, issue_flags); pr_devel("%s: cmd done ret %d cmd_op %x, dev id %d qid %d\n", __func__, ret, cmd->cmd_op, header->dev_id, header->queue_id); - return -EIOCBQUEUED; + return ret; } static const struct file_operations ublk_ctl_fops = { .open = nonseekable_open, .uring_cmd = ublk_ctrl_uring_cmd,
In case of an error, ublk's ->uring_cmd() functions currently return -EIOCBQUEUED and immediately call io_uring_cmd_done(). -EIOCBQUEUED and io_uring_cmd_done() are intended for asynchronous completions. For synchronous completions, the ->uring_cmd() function can just return the negative return code directly. This skips io_uring_cmd_del_cancelable(), and deferring the completion to task work. So return the error code directly from __ublk_ch_uring_cmd() and ublk_ctrl_uring_cmd(). Update ublk_ch_uring_cmd_cb(), which currently ignores the return value from __ublk_ch_uring_cmd(), to call io_uring_cmd_done() for synchronous completions. Signed-off-by: Caleb Sander Mateos <csander@purestorage.com> --- drivers/block/ublk_drv.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)