* [PATCH 01/16] ublk: move fake timeout logic into __ublk_complete_rq()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-03 2:23 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 02/16] ublk: look up ublk task via its pid in timeout handler Ming Lei
` (14 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Almost every block driver deals with fake timeout logic around real
request completion code.
Also the existing way may cause request reference count leak, so move the
logic into __ublk_complete_rq(), then we can skip the completion in the
last step like other drivers.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index e52c2d1cb838..e2de76d78a08 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1152,7 +1152,7 @@ static inline void __ublk_complete_rq(struct request *req)
if (blk_update_request(req, BLK_STS_OK, io->res))
blk_mq_requeue_request(req, true);
- else
+ else if (likely(!blk_should_fake_timeout(req->q)))
__blk_mq_end_request(req, BLK_STS_OK);
return;
@@ -2206,9 +2206,6 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
if (req_op(req) == REQ_OP_ZONE_APPEND)
req->__sector = ub_cmd->zone_append_lba;
- if (unlikely(blk_should_fake_timeout(req->q)))
- return 0;
-
if (ublk_need_req_ref(ubq))
ublk_sub_req_ref(io, req);
else
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/16] ublk: look up ublk task via its pid in timeout handler
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
2025-07-02 4:03 ` [PATCH 01/16] ublk: move fake timeout logic into __ublk_complete_rq() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 03/16] ublk: let ublk_fill_io_cmd() cover more things Ming Lei
` (13 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Look up ublk process via its pid in timeout handler, so we can avoid to
touch io->task, because it is fragile to touch task structure.
It is fine to kill ublk server process and this way is simpler.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index e2de76d78a08..834d4db3b07e 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1365,14 +1365,19 @@ static void ublk_queue_cmd_list(struct ublk_io *io, struct rq_list *l)
static enum blk_eh_timer_return ublk_timeout(struct request *rq)
{
struct ublk_queue *ubq = rq->mq_hctx->driver_data;
- struct ublk_io *io = &ubq->ios[rq->tag];
-
- if (ubq->flags & UBLK_F_UNPRIVILEGED_DEV) {
- send_sig(SIGKILL, io->task, 0);
- return BLK_EH_DONE;
- }
-
- return BLK_EH_RESET_TIMER;
+ struct task_struct *p;
+ struct pid *pid;
+
+ if (!(ubq->flags & UBLK_F_UNPRIVILEGED_DEV))
+ return BLK_EH_RESET_TIMER;
+
+ rcu_read_lock();
+ pid = find_vpid(ubq->dev->dev_info.ublksrv_pid);
+ p = pid_task(pid, PIDTYPE_PID);
+ if (p)
+ send_sig(SIGKILL, p, 0);
+ rcu_read_unlock();
+ return BLK_EH_DONE;
}
static blk_status_t ublk_prep_req(struct ublk_queue *ubq, struct request *rq,
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/16] ublk: let ublk_fill_io_cmd() cover more things
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
2025-07-02 4:03 ` [PATCH 01/16] ublk: move fake timeout logic into __ublk_complete_rq() Ming Lei
2025-07-02 4:03 ` [PATCH 02/16] ublk: look up ublk task via its pid in timeout handler Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 04/16] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch() Ming Lei
` (12 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Let ublk_fill_io_cmd() cover more things:
- store io command result
- clear UBLK_IO_FLAG_OWNED_BY_SRV
It is fine to do above for ublk_fetch(), ublk_commit_and_fetch() and
handling UBLK_IO_NEED_GET_DATA.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 834d4db3b07e..2dc6962c804a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1984,11 +1984,16 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
}
static inline void ublk_fill_io_cmd(struct ublk_io *io,
- struct io_uring_cmd *cmd, unsigned long buf_addr)
+ struct io_uring_cmd *cmd, unsigned long buf_addr,
+ int result)
{
io->cmd = cmd;
io->flags |= UBLK_IO_FLAG_ACTIVE;
io->addr = buf_addr;
+ io->res = result;
+
+ /* now this cmd slot is owned by ublk driver */
+ io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
}
static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
@@ -2146,7 +2151,7 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
goto out;
}
- ublk_fill_io_cmd(io, cmd, buf_addr);
+ ublk_fill_io_cmd(io, cmd, buf_addr, 0);
WRITE_ONCE(io->task, get_task_struct(current));
ublk_mark_io_ready(ub, ubq);
out:
@@ -2202,11 +2207,7 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
return ret;
}
- ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
-
- /* now this cmd slot is owned by ublk driver */
- io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
- io->res = ub_cmd->result;
+ ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
if (req_op(req) == REQ_OP_ZONE_APPEND)
req->__sector = ub_cmd->zone_append_lba;
@@ -2326,8 +2327,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
* request
*/
req = io->req;
- ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
- io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+ ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
if (likely(ublk_get_data(ubq, io, req))) {
__ublk_prep_compl_io_cmd(io, req);
return UBLK_IO_RES_OK;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/16] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (2 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 03/16] ublk: let ublk_fill_io_cmd() cover more things Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-03 2:33 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper Ming Lei
` (11 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Refactor ublk_commit_and_fetch() in the following way for removing
parameter of `struct ublksrv_io_cmd *`:
- return `struct request *` from ublk_fill_io_cmd(), so that we can
use request reference reliably in this way cause both request and
io_uring_cmd reference share same storage
- move ublk_fill_io_cmd() before calling into ublk_commit_and_fetch(),
so that ublk_fill_io_cmd() could be run with per-io lock held for
supporting command batch.
- pass ->zone_append_lba to ublk_commit_and_fetch() directly
The main motivation is to reproduce ublk_commit_and_fetch() for fetching
io command batch with multishot uring_cmd.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 43 ++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2dc6962c804a..1780f9ce3a24 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1983,10 +1983,13 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
return 0;
}
-static inline void ublk_fill_io_cmd(struct ublk_io *io,
- struct io_uring_cmd *cmd, unsigned long buf_addr,
- int result)
+/* Once we return, `io->req` can't be used any more */
+static inline struct request *
+ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
+ unsigned long buf_addr, int result)
{
+ struct request *req = io->req;
+
io->cmd = cmd;
io->flags |= UBLK_IO_FLAG_ACTIVE;
io->addr = buf_addr;
@@ -1994,6 +1997,8 @@ static inline void ublk_fill_io_cmd(struct ublk_io *io,
/* now this cmd slot is owned by ublk driver */
io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+
+ return req;
}
static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
@@ -2159,10 +2164,8 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
return ret;
}
-static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
- struct ublk_io *io, struct io_uring_cmd *cmd,
- const struct ublksrv_io_cmd *ub_cmd,
- unsigned int issue_flags)
+static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
+ struct ublk_io *io, __u64 buf_addr)
{
struct request *req = io->req;
@@ -2171,10 +2174,10 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
* COMMIT_AND_FETCH_REQ has to provide IO buffer if
* NEED GET DATA is not enabled or it is Read IO.
*/
- if (!ub_cmd->addr && (!ublk_need_get_data(ubq) ||
+ if (!buf_addr && (!ublk_need_get_data(ubq) ||
req_op(req) == REQ_OP_READ))
return -EINVAL;
- } else if (req_op(req) != REQ_OP_ZONE_APPEND && ub_cmd->addr) {
+ } else if (req_op(req) != REQ_OP_ZONE_APPEND && buf_addr) {
/*
* User copy requires addr to be unset when command is
* not zone append
@@ -2182,6 +2185,14 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
return -EINVAL;
}
+ return 0;
+}
+
+static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
+ struct ublk_io *io, struct io_uring_cmd *cmd,
+ struct request *req, unsigned int issue_flags,
+ __u64 zone_append_lba)
+{
if (ublk_support_auto_buf_reg(ubq)) {
int ret;
@@ -2207,10 +2218,8 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
return ret;
}
- ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
-
if (req_op(req) == REQ_OP_ZONE_APPEND)
- req->__sector = ub_cmd->zone_append_lba;
+ req->__sector = zone_append_lba;
if (ublk_need_req_ref(ubq))
ublk_sub_req_ref(io, req);
@@ -2316,7 +2325,12 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
return ublk_daemon_register_io_buf(cmd, ubq, io, ub_cmd->addr,
issue_flags);
case UBLK_IO_COMMIT_AND_FETCH_REQ:
- ret = ublk_commit_and_fetch(ubq, io, cmd, ub_cmd, issue_flags);
+ ret = ublk_check_commit_and_fetch(ubq, io, ub_cmd->addr);
+ if (ret)
+ goto out;
+ req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
+ ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
+ ub_cmd->zone_append_lba);
if (ret)
goto out;
break;
@@ -2326,8 +2340,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
* uring_cmd active first and prepare for handling new requeued
* request
*/
- req = io->req;
- ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
+ req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
if (likely(ublk_get_data(ubq, io, req))) {
__ublk_prep_compl_io_cmd(io, req);
return UBLK_IO_RES_OK;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 04/16] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch()
2025-07-02 4:03 ` [PATCH 04/16] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch() Ming Lei
@ 2025-07-03 2:33 ` Caleb Sander Mateos
0 siblings, 0 replies; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-03 2:33 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Refactor ublk_commit_and_fetch() in the following way for removing
> parameter of `struct ublksrv_io_cmd *`:
>
> - return `struct request *` from ublk_fill_io_cmd(), so that we can
> use request reference reliably in this way cause both request and
> io_uring_cmd reference share same storage
>
> - move ublk_fill_io_cmd() before calling into ublk_commit_and_fetch(),
> so that ublk_fill_io_cmd() could be run with per-io lock held for
> supporting command batch.
>
> - pass ->zone_append_lba to ublk_commit_and_fetch() directly
>
> The main motivation is to reproduce ublk_commit_and_fetch() for fetching
> io command batch with multishot uring_cmd.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/block/ublk_drv.c | 43 ++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2dc6962c804a..1780f9ce3a24 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1983,10 +1983,13 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> return 0;
> }
>
> -static inline void ublk_fill_io_cmd(struct ublk_io *io,
> - struct io_uring_cmd *cmd, unsigned long buf_addr,
> - int result)
> +/* Once we return, `io->req` can't be used any more */
> +static inline struct request *
> +ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
> + unsigned long buf_addr, int result)
> {
> + struct request *req = io->req;
> +
> io->cmd = cmd;
> io->flags |= UBLK_IO_FLAG_ACTIVE;
> io->addr = buf_addr;
> @@ -1994,6 +1997,8 @@ static inline void ublk_fill_io_cmd(struct ublk_io *io,
>
> /* now this cmd slot is owned by ublk driver */
> io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
> +
> + return req;
This is nice symmetry with __ublk_prep_compl_io_cmd().
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> }
>
> static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
> @@ -2159,10 +2164,8 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> return ret;
> }
>
> -static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> - struct ublk_io *io, struct io_uring_cmd *cmd,
> - const struct ublksrv_io_cmd *ub_cmd,
> - unsigned int issue_flags)
> +static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
> + struct ublk_io *io, __u64 buf_addr)
> {
> struct request *req = io->req;
>
> @@ -2171,10 +2174,10 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> * COMMIT_AND_FETCH_REQ has to provide IO buffer if
> * NEED GET DATA is not enabled or it is Read IO.
> */
> - if (!ub_cmd->addr && (!ublk_need_get_data(ubq) ||
> + if (!buf_addr && (!ublk_need_get_data(ubq) ||
> req_op(req) == REQ_OP_READ))
> return -EINVAL;
> - } else if (req_op(req) != REQ_OP_ZONE_APPEND && ub_cmd->addr) {
> + } else if (req_op(req) != REQ_OP_ZONE_APPEND && buf_addr) {
> /*
> * User copy requires addr to be unset when command is
> * not zone append
> @@ -2182,6 +2185,14 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> return -EINVAL;
> }
>
> + return 0;
> +}
> +
> +static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> + struct ublk_io *io, struct io_uring_cmd *cmd,
> + struct request *req, unsigned int issue_flags,
> + __u64 zone_append_lba)
> +{
> if (ublk_support_auto_buf_reg(ubq)) {
> int ret;
>
> @@ -2207,10 +2218,8 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> return ret;
> }
>
> - ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
> -
> if (req_op(req) == REQ_OP_ZONE_APPEND)
> - req->__sector = ub_cmd->zone_append_lba;
> + req->__sector = zone_append_lba;
>
> if (ublk_need_req_ref(ubq))
> ublk_sub_req_ref(io, req);
> @@ -2316,7 +2325,12 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> return ublk_daemon_register_io_buf(cmd, ubq, io, ub_cmd->addr,
> issue_flags);
> case UBLK_IO_COMMIT_AND_FETCH_REQ:
> - ret = ublk_commit_and_fetch(ubq, io, cmd, ub_cmd, issue_flags);
> + ret = ublk_check_commit_and_fetch(ubq, io, ub_cmd->addr);
> + if (ret)
> + goto out;
> + req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
> + ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> + ub_cmd->zone_append_lba);
> if (ret)
> goto out;
> break;
> @@ -2326,8 +2340,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> * uring_cmd active first and prepare for handling new requeued
> * request
> */
> - req = io->req;
> - ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
> + req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
> if (likely(ublk_get_data(ubq, io, req))) {
> __ublk_prep_compl_io_cmd(io, req);
> return UBLK_IO_RES_OK;
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (3 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 04/16] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-03 20:19 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 06/16] ublk: store auto buffer register data into `struct ublk_io` Ming Lei
` (10 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Move check & clearing UBLK_IO_FLAG_AUTO_BUF_REG to
ublk_handle_auto_buf_reg(), also return buffer index from this helper.
Also move ublk_set_auto_buf_reg() to this single helper too.
Add ublk_config_io_buf() for setting up ublk io buffer, covers both
ublk buffer copy or auto buffer register.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 132 ++++++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 56 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 1780f9ce3a24..ba1b2672e7a8 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -48,6 +48,8 @@
#define UBLK_MINORS (1U << MINORBITS)
+#define UBLK_INVALID_BUF_IDX ((unsigned short)-1)
+
/* private ioctl command mirror */
#define UBLK_CMD_DEL_DEV_ASYNC _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
#define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
@@ -1983,16 +1985,53 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
return 0;
}
+static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
+{
+ struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
+
+ pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
+
+ if (pdu->buf.reserved0 || pdu->buf.reserved1)
+ return -EINVAL;
+
+ if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
+ return -EINVAL;
+ return 0;
+}
+
+static int ublk_handle_auto_buf_reg(struct ublk_io *io,
+ struct io_uring_cmd *cmd,
+ u16 *buf_idx)
+{
+ if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
+ io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
+
+ /*
+ * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
+ * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
+ * `io_ring_ctx`.
+ *
+ * If this uring_cmd's io_ring_ctx isn't same with the
+ * one for registering the buffer, it is ublk server's
+ * responsibility for unregistering the buffer, otherwise
+ * this ublk request gets stuck.
+ */
+ if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
+ buf_idx)
+ *buf_idx = io->buf_index;
+ }
+
+ return ublk_set_auto_buf_reg(cmd);
+}
+
/* Once we return, `io->req` can't be used any more */
static inline struct request *
-ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
- unsigned long buf_addr, int result)
+ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd, int result)
{
struct request *req = io->req;
io->cmd = cmd;
io->flags |= UBLK_IO_FLAG_ACTIVE;
- io->addr = buf_addr;
io->res = result;
/* now this cmd slot is owned by ublk driver */
@@ -2001,6 +2040,22 @@ ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
return req;
}
+static inline int
+ublk_config_io_buf(const struct ublk_queue *ubq, struct ublk_io *io,
+ struct io_uring_cmd *cmd, unsigned long buf_addr,
+ u16 *buf_idx)
+{
+ if (ublk_support_auto_buf_reg(ubq)) {
+ int ret = ublk_handle_auto_buf_reg(io, cmd, buf_idx);
+
+ if (ret)
+ return ret;
+ } else {
+ io->addr = buf_addr;
+ }
+ return 0;
+}
+
static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
unsigned int issue_flags,
struct ublk_queue *ubq, unsigned int tag)
@@ -2016,20 +2071,6 @@ static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
io_uring_cmd_mark_cancelable(cmd, issue_flags);
}
-static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
-{
- struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
-
- pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
-
- if (pdu->buf.reserved0 || pdu->buf.reserved1)
- return -EINVAL;
-
- if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
- return -EINVAL;
- return 0;
-}
-
static void ublk_io_release(void *priv)
{
struct request *rq = priv;
@@ -2150,13 +2191,11 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
goto out;
}
- if (ublk_support_auto_buf_reg(ubq)) {
- ret = ublk_set_auto_buf_reg(cmd);
- if (ret)
- goto out;
- }
+ ublk_fill_io_cmd(io, cmd, 0);
+ ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
+ if (ret)
+ goto out;
- ublk_fill_io_cmd(io, cmd, buf_addr, 0);
WRITE_ONCE(io->task, get_task_struct(current));
ublk_mark_io_ready(ub, ubq);
out:
@@ -2188,35 +2227,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
return 0;
}
-static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
- struct ublk_io *io, struct io_uring_cmd *cmd,
- struct request *req, unsigned int issue_flags,
- __u64 zone_append_lba)
+static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
+ struct ublk_io *io, struct io_uring_cmd *cmd,
+ struct request *req, unsigned int issue_flags,
+ __u64 zone_append_lba, u16 buf_idx)
{
- if (ublk_support_auto_buf_reg(ubq)) {
- int ret;
-
- /*
- * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
- * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
- * `io_ring_ctx`.
- *
- * If this uring_cmd's io_ring_ctx isn't same with the
- * one for registering the buffer, it is ublk server's
- * responsibility for unregistering the buffer, otherwise
- * this ublk request gets stuck.
- */
- if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
- if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd))
- io_buffer_unregister_bvec(cmd, io->buf_index,
- issue_flags);
- io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
- }
-
- ret = ublk_set_auto_buf_reg(cmd);
- if (ret)
- return ret;
- }
+ if (buf_idx != UBLK_INVALID_BUF_IDX)
+ io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
if (req_op(req) == REQ_OP_ZONE_APPEND)
req->__sector = zone_append_lba;
@@ -2225,7 +2242,6 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
ublk_sub_req_ref(io, req);
else
__ublk_complete_rq(req);
- return 0;
}
static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
@@ -2250,6 +2266,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
unsigned int issue_flags,
const struct ublksrv_io_cmd *ub_cmd)
{
+ u16 buf_idx = UBLK_INVALID_BUF_IDX;
struct ublk_device *ub = cmd->file->private_data;
struct ublk_queue *ubq;
struct ublk_io *io;
@@ -2328,9 +2345,10 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
ret = ublk_check_commit_and_fetch(ubq, io, ub_cmd->addr);
if (ret)
goto out;
- req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
- ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
- ub_cmd->zone_append_lba);
+ req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
+ ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
+ ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
+ ub_cmd->zone_append_lba, buf_idx);
if (ret)
goto out;
break;
@@ -2340,7 +2358,9 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
* uring_cmd active first and prepare for handling new requeued
* request
*/
- req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
+ req = ublk_fill_io_cmd(io, cmd, 0);
+ ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, NULL);
+ WARN_ON_ONCE(ret);
if (likely(ublk_get_data(ubq, io, req))) {
__ublk_prep_compl_io_cmd(io, req);
return UBLK_IO_RES_OK;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper
2025-07-02 4:03 ` [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper Ming Lei
@ 2025-07-03 20:19 ` Caleb Sander Mateos
2025-07-07 9:44 ` Ming Lei
0 siblings, 1 reply; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-03 20:19 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Move check & clearing UBLK_IO_FLAG_AUTO_BUF_REG to
> ublk_handle_auto_buf_reg(), also return buffer index from this helper.
>
> Also move ublk_set_auto_buf_reg() to this single helper too.
>
> Add ublk_config_io_buf() for setting up ublk io buffer, covers both
> ublk buffer copy or auto buffer register.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/block/ublk_drv.c | 132 ++++++++++++++++++++++-----------------
> 1 file changed, 76 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 1780f9ce3a24..ba1b2672e7a8 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -48,6 +48,8 @@
>
> #define UBLK_MINORS (1U << MINORBITS)
>
> +#define UBLK_INVALID_BUF_IDX ((unsigned short)-1)
u16?
> +
> /* private ioctl command mirror */
> #define UBLK_CMD_DEL_DEV_ASYNC _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
> #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> @@ -1983,16 +1985,53 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> return 0;
> }
>
> +static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
> +{
> + struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> +
> + pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
> +
> + if (pdu->buf.reserved0 || pdu->buf.reserved1)
> + return -EINVAL;
> +
> + if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static int ublk_handle_auto_buf_reg(struct ublk_io *io,
> + struct io_uring_cmd *cmd,
> + u16 *buf_idx)
> +{
> + if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
> + io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
> +
> + /*
> + * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
> + * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
> + * `io_ring_ctx`.
> + *
> + * If this uring_cmd's io_ring_ctx isn't same with the
> + * one for registering the buffer, it is ublk server's
> + * responsibility for unregistering the buffer, otherwise
> + * this ublk request gets stuck.
> + */
> + if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
> + buf_idx)
Not sure the buf_idx check here is necessary. This io->flags &
UBLK_IO_FLAG_AUTO_BUF_REG path should only be reachable from
ublk_commit_and_fetch(), not ublk_fetch() or UBLK_IO_NEED_GET_DATA.
> + *buf_idx = io->buf_index;
> + }
> +
> + return ublk_set_auto_buf_reg(cmd);
> +}
> +
> /* Once we return, `io->req` can't be used any more */
> static inline struct request *
> -ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
> - unsigned long buf_addr, int result)
> +ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd, int result)
> {
> struct request *req = io->req;
>
> io->cmd = cmd;
> io->flags |= UBLK_IO_FLAG_ACTIVE;
> - io->addr = buf_addr;
> io->res = result;
>
> /* now this cmd slot is owned by ublk driver */
> @@ -2001,6 +2040,22 @@ ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
> return req;
> }
>
> +static inline int
> +ublk_config_io_buf(const struct ublk_queue *ubq, struct ublk_io *io,
> + struct io_uring_cmd *cmd, unsigned long buf_addr,
> + u16 *buf_idx)
> +{
> + if (ublk_support_auto_buf_reg(ubq)) {
> + int ret = ublk_handle_auto_buf_reg(io, cmd, buf_idx);
> +
> + if (ret)
> + return ret;
Just return ublk_handle_auto_buf_reg(io, cmd, buf_idx)?
Other than that,
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> + } else {
> + io->addr = buf_addr;
> + }
> + return 0;
> +}
> +
> static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
> unsigned int issue_flags,
> struct ublk_queue *ubq, unsigned int tag)
> @@ -2016,20 +2071,6 @@ static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
> io_uring_cmd_mark_cancelable(cmd, issue_flags);
> }
>
> -static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
> -{
> - struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> -
> - pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
> -
> - if (pdu->buf.reserved0 || pdu->buf.reserved1)
> - return -EINVAL;
> -
> - if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> - return -EINVAL;
> - return 0;
> -}
> -
> static void ublk_io_release(void *priv)
> {
> struct request *rq = priv;
> @@ -2150,13 +2191,11 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> goto out;
> }
>
> - if (ublk_support_auto_buf_reg(ubq)) {
> - ret = ublk_set_auto_buf_reg(cmd);
> - if (ret)
> - goto out;
> - }
> + ublk_fill_io_cmd(io, cmd, 0);
> + ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
> + if (ret)
> + goto out;
>
> - ublk_fill_io_cmd(io, cmd, buf_addr, 0);
> WRITE_ONCE(io->task, get_task_struct(current));
> ublk_mark_io_ready(ub, ubq);
> out:
> @@ -2188,35 +2227,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
> return 0;
> }
>
> -static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> - struct ublk_io *io, struct io_uring_cmd *cmd,
> - struct request *req, unsigned int issue_flags,
> - __u64 zone_append_lba)
> +static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
> + struct ublk_io *io, struct io_uring_cmd *cmd,
> + struct request *req, unsigned int issue_flags,
> + __u64 zone_append_lba, u16 buf_idx)
> {
> - if (ublk_support_auto_buf_reg(ubq)) {
> - int ret;
> -
> - /*
> - * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
> - * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
> - * `io_ring_ctx`.
> - *
> - * If this uring_cmd's io_ring_ctx isn't same with the
> - * one for registering the buffer, it is ublk server's
> - * responsibility for unregistering the buffer, otherwise
> - * this ublk request gets stuck.
> - */
> - if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
> - if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd))
> - io_buffer_unregister_bvec(cmd, io->buf_index,
> - issue_flags);
> - io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
> - }
> -
> - ret = ublk_set_auto_buf_reg(cmd);
> - if (ret)
> - return ret;
> - }
> + if (buf_idx != UBLK_INVALID_BUF_IDX)
> + io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
>
> if (req_op(req) == REQ_OP_ZONE_APPEND)
> req->__sector = zone_append_lba;
> @@ -2225,7 +2242,6 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> ublk_sub_req_ref(io, req);
> else
> __ublk_complete_rq(req);
> - return 0;
> }
>
> static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
> @@ -2250,6 +2266,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> unsigned int issue_flags,
> const struct ublksrv_io_cmd *ub_cmd)
> {
> + u16 buf_idx = UBLK_INVALID_BUF_IDX;
> struct ublk_device *ub = cmd->file->private_data;
> struct ublk_queue *ubq;
> struct ublk_io *io;
> @@ -2328,9 +2345,10 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> ret = ublk_check_commit_and_fetch(ubq, io, ub_cmd->addr);
> if (ret)
> goto out;
> - req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
> - ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> - ub_cmd->zone_append_lba);
> + req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
> + ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
> + ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> + ub_cmd->zone_append_lba, buf_idx);
> if (ret)
> goto out;
> break;
> @@ -2340,7 +2358,9 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> * uring_cmd active first and prepare for handling new requeued
> * request
> */
> - req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
> + req = ublk_fill_io_cmd(io, cmd, 0);
> + ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, NULL);
> + WARN_ON_ONCE(ret);
> if (likely(ublk_get_data(ubq, io, req))) {
> __ublk_prep_compl_io_cmd(io, req);
> return UBLK_IO_RES_OK;
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper
2025-07-03 20:19 ` Caleb Sander Mateos
@ 2025-07-07 9:44 ` Ming Lei
0 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-07 9:44 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Jens Axboe, linux-block, Uday Shankar
On Thu, Jul 03, 2025 at 04:19:50PM -0400, Caleb Sander Mateos wrote:
> On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Move check & clearing UBLK_IO_FLAG_AUTO_BUF_REG to
> > ublk_handle_auto_buf_reg(), also return buffer index from this helper.
> >
> > Also move ublk_set_auto_buf_reg() to this single helper too.
> >
> > Add ublk_config_io_buf() for setting up ublk io buffer, covers both
> > ublk buffer copy or auto buffer register.
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > drivers/block/ublk_drv.c | 132 ++++++++++++++++++++++-----------------
> > 1 file changed, 76 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 1780f9ce3a24..ba1b2672e7a8 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -48,6 +48,8 @@
> >
> > #define UBLK_MINORS (1U << MINORBITS)
> >
> > +#define UBLK_INVALID_BUF_IDX ((unsigned short)-1)
>
> u16?
Yeah.
>
> > +
> > /* private ioctl command mirror */
> > #define UBLK_CMD_DEL_DEV_ASYNC _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
> > #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> > @@ -1983,16 +1985,53 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> > return 0;
> > }
> >
> > +static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
> > +{
> > + struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> > +
> > + pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
> > +
> > + if (pdu->buf.reserved0 || pdu->buf.reserved1)
> > + return -EINVAL;
> > +
> > + if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> > + return -EINVAL;
> > + return 0;
> > +}
> > +
> > +static int ublk_handle_auto_buf_reg(struct ublk_io *io,
> > + struct io_uring_cmd *cmd,
> > + u16 *buf_idx)
> > +{
> > + if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
> > + io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
> > +
> > + /*
> > + * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
> > + * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
> > + * `io_ring_ctx`.
> > + *
> > + * If this uring_cmd's io_ring_ctx isn't same with the
> > + * one for registering the buffer, it is ublk server's
> > + * responsibility for unregistering the buffer, otherwise
> > + * this ublk request gets stuck.
> > + */
> > + if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
> > + buf_idx)
>
> Not sure the buf_idx check here is necessary. This io->flags &
> UBLK_IO_FLAG_AUTO_BUF_REG path should only be reachable from
> ublk_commit_and_fetch(), not ublk_fetch() or UBLK_IO_NEED_GET_DATA.
Good catch, UBLK_IO_FLAG_AUTO_BUF_REG can only be set from
ublk_commit_and_fetch(), will fix it in next version.
Thanks,
Ming
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 06/16] ublk: store auto buffer register data into `struct ublk_io`
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (4 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 05/16] ublk: move auto buffer register handling into one dedicated helper Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-08 12:15 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 07/16] ublk: add helper ublk_check_fetch_buf() Ming Lei
` (9 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
We can share space of `io->addr` for storing auto buffer register data
and user space buffer address.
So store auto buffer register data into `struct ublk_io`.
Prepare for supporting batch IO in which many ublk IOs share single
uring_cmd, so we can't store auto buffer register data into uring_cmd
pdu.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index ba1b2672e7a8..6d3aa08eef22 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -105,8 +105,6 @@ struct ublk_uring_cmd_pdu {
*/
struct ublk_queue *ubq;
- struct ublk_auto_buf_reg buf;
-
u16 tag;
};
@@ -159,7 +157,10 @@ struct ublk_uring_cmd_pdu {
struct ublk_io {
/* userspace buffer address from io cmd */
- __u64 addr;
+ union {
+ __u64 addr;
+ struct ublk_auto_buf_reg buf;
+ };
unsigned int flags;
int res;
@@ -187,8 +188,6 @@ struct ublk_io {
/* Count of buffers registered on task and not yet unregistered */
unsigned task_registered_buffers;
- /* auto-registered buffer, valid if UBLK_IO_FLAG_AUTO_BUF_REG is set */
- u16 buf_index;
void *buf_ctx_handle;
} ____cacheline_aligned_in_smp;
@@ -1214,13 +1213,12 @@ ublk_auto_buf_reg_fallback(const struct ublk_queue *ubq, struct ublk_io *io)
static bool ublk_auto_buf_reg(const struct ublk_queue *ubq, struct request *req,
struct ublk_io *io, unsigned int issue_flags)
{
- struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(io->cmd);
int ret;
ret = io_buffer_register_bvec(io->cmd, req, ublk_io_release,
- pdu->buf.index, issue_flags);
+ io->buf.index, issue_flags);
if (ret) {
- if (pdu->buf.flags & UBLK_AUTO_BUF_REG_FALLBACK) {
+ if (io->buf.flags & UBLK_AUTO_BUF_REG_FALLBACK) {
ublk_auto_buf_reg_fallback(ubq, io);
return true;
}
@@ -1230,8 +1228,6 @@ static bool ublk_auto_buf_reg(const struct ublk_queue *ubq, struct request *req,
io->task_registered_buffers = 1;
io->buf_ctx_handle = io_uring_cmd_ctx_handle(io->cmd);
- /* store buffer index in request payload */
- io->buf_index = pdu->buf.index;
io->flags |= UBLK_IO_FLAG_AUTO_BUF_REG;
return true;
}
@@ -1985,16 +1981,14 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
return 0;
}
-static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
+static inline int ublk_set_auto_buf_reg(struct ublk_io *io, struct io_uring_cmd *cmd)
{
- struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
-
- pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
+ io->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
- if (pdu->buf.reserved0 || pdu->buf.reserved1)
+ if (io->buf.reserved0 || io->buf.reserved1)
return -EINVAL;
- if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
+ if (io->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
return -EINVAL;
return 0;
}
@@ -2018,10 +2012,10 @@ static int ublk_handle_auto_buf_reg(struct ublk_io *io,
*/
if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
buf_idx)
- *buf_idx = io->buf_index;
+ *buf_idx = io->buf.index;
}
- return ublk_set_auto_buf_reg(cmd);
+ return ublk_set_auto_buf_reg(io, cmd);
}
/* Once we return, `io->req` can't be used any more */
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 06/16] ublk: store auto buffer register data into `struct ublk_io`
2025-07-02 4:03 ` [PATCH 06/16] ublk: store auto buffer register data into `struct ublk_io` Ming Lei
@ 2025-07-08 12:15 ` Caleb Sander Mateos
0 siblings, 0 replies; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-08 12:15 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> We can share space of `io->addr` for storing auto buffer register data
> and user space buffer address.
>
> So store auto buffer register data into `struct ublk_io`.
>
> Prepare for supporting batch IO in which many ublk IOs share single
> uring_cmd, so we can't store auto buffer register data into uring_cmd
> pdu.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
> drivers/block/ublk_drv.c | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index ba1b2672e7a8..6d3aa08eef22 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -105,8 +105,6 @@ struct ublk_uring_cmd_pdu {
> */
> struct ublk_queue *ubq;
>
> - struct ublk_auto_buf_reg buf;
> -
> u16 tag;
> };
>
> @@ -159,7 +157,10 @@ struct ublk_uring_cmd_pdu {
>
> struct ublk_io {
> /* userspace buffer address from io cmd */
> - __u64 addr;
> + union {
> + __u64 addr;
> + struct ublk_auto_buf_reg buf;
> + };
> unsigned int flags;
> int res;
>
> @@ -187,8 +188,6 @@ struct ublk_io {
> /* Count of buffers registered on task and not yet unregistered */
> unsigned task_registered_buffers;
>
> - /* auto-registered buffer, valid if UBLK_IO_FLAG_AUTO_BUF_REG is set */
> - u16 buf_index;
> void *buf_ctx_handle;
> } ____cacheline_aligned_in_smp;
>
> @@ -1214,13 +1213,12 @@ ublk_auto_buf_reg_fallback(const struct ublk_queue *ubq, struct ublk_io *io)
> static bool ublk_auto_buf_reg(const struct ublk_queue *ubq, struct request *req,
> struct ublk_io *io, unsigned int issue_flags)
> {
> - struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(io->cmd);
> int ret;
>
> ret = io_buffer_register_bvec(io->cmd, req, ublk_io_release,
> - pdu->buf.index, issue_flags);
> + io->buf.index, issue_flags);
> if (ret) {
> - if (pdu->buf.flags & UBLK_AUTO_BUF_REG_FALLBACK) {
> + if (io->buf.flags & UBLK_AUTO_BUF_REG_FALLBACK) {
> ublk_auto_buf_reg_fallback(ubq, io);
> return true;
> }
> @@ -1230,8 +1228,6 @@ static bool ublk_auto_buf_reg(const struct ublk_queue *ubq, struct request *req,
>
> io->task_registered_buffers = 1;
> io->buf_ctx_handle = io_uring_cmd_ctx_handle(io->cmd);
> - /* store buffer index in request payload */
> - io->buf_index = pdu->buf.index;
> io->flags |= UBLK_IO_FLAG_AUTO_BUF_REG;
> return true;
> }
> @@ -1985,16 +1981,14 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> return 0;
> }
>
> -static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
> +static inline int ublk_set_auto_buf_reg(struct ublk_io *io, struct io_uring_cmd *cmd)
> {
> - struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> -
> - pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
> + io->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
>
> - if (pdu->buf.reserved0 || pdu->buf.reserved1)
> + if (io->buf.reserved0 || io->buf.reserved1)
> return -EINVAL;
>
> - if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> + if (io->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> return -EINVAL;
> return 0;
> }
> @@ -2018,10 +2012,10 @@ static int ublk_handle_auto_buf_reg(struct ublk_io *io,
> */
> if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
> buf_idx)
> - *buf_idx = io->buf_index;
> + *buf_idx = io->buf.index;
> }
>
> - return ublk_set_auto_buf_reg(cmd);
> + return ublk_set_auto_buf_reg(io, cmd);
> }
>
> /* Once we return, `io->req` can't be used any more */
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 07/16] ublk: add helper ublk_check_fetch_buf()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (5 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 06/16] ublk: store auto buffer register data into `struct ublk_io` Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-08 12:31 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 08/16] ublk: remove ublk_commit_and_fetch() Ming Lei
` (8 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Add helper ublk_check_fetch_buf() for checking buffer only.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 6d3aa08eef22..7fd2fa493d6a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2146,6 +2146,22 @@ static int ublk_unregister_io_buf(struct io_uring_cmd *cmd,
return io_buffer_unregister_bvec(cmd, index, issue_flags);
}
+static int ublk_check_fetch_buf(const struct ublk_queue *ubq, __u64 buf_addr)
+{
+ if (ublk_need_map_io(ubq)) {
+ /*
+ * FETCH_RQ has to provide IO buffer if NEED GET
+ * DATA is not enabled
+ */
+ if (!buf_addr && !ublk_need_get_data(ubq))
+ return -EINVAL;
+ } else if (buf_addr) {
+ /* User copy requires addr to be unset */
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
struct ublk_io *io, __u64 buf_addr)
{
@@ -2172,19 +2188,6 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
WARN_ON_ONCE(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV);
- if (ublk_need_map_io(ubq)) {
- /*
- * FETCH_RQ has to provide IO buffer if NEED GET
- * DATA is not enabled
- */
- if (!buf_addr && !ublk_need_get_data(ubq))
- goto out;
- } else if (buf_addr) {
- /* User copy requires addr to be unset */
- ret = -EINVAL;
- goto out;
- }
-
ublk_fill_io_cmd(io, cmd, 0);
ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
if (ret)
@@ -2297,6 +2300,9 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
io = &ubq->ios[tag];
/* UBLK_IO_FETCH_REQ can be handled on any task, which sets io->task */
if (unlikely(_IOC_NR(cmd_op) == UBLK_IO_FETCH_REQ)) {
+ ret = ublk_check_fetch_buf(ubq, ub_cmd->addr);
+ if (ret)
+ goto out;
ret = ublk_fetch(cmd, ubq, io, ub_cmd->addr);
if (ret)
goto out;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 07/16] ublk: add helper ublk_check_fetch_buf()
2025-07-02 4:03 ` [PATCH 07/16] ublk: add helper ublk_check_fetch_buf() Ming Lei
@ 2025-07-08 12:31 ` Caleb Sander Mateos
2025-07-13 14:13 ` Ming Lei
0 siblings, 1 reply; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-08 12:31 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Add helper ublk_check_fetch_buf() for checking buffer only.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
The commit message seems a bit sparse. How about something like:
Add a helper ublk_check_fetch_buf() to validate UBLK_IO_FETCH_REQ's
addr. This doesn't require access to the ublk_io, so it can be done
before taking the ublk_device mutex.
> ---
> drivers/block/ublk_drv.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 6d3aa08eef22..7fd2fa493d6a 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2146,6 +2146,22 @@ static int ublk_unregister_io_buf(struct io_uring_cmd *cmd,
> return io_buffer_unregister_bvec(cmd, index, issue_flags);
> }
>
> +static int ublk_check_fetch_buf(const struct ublk_queue *ubq, __u64 buf_addr)
> +{
> + if (ublk_need_map_io(ubq)) {
> + /*
> + * FETCH_RQ has to provide IO buffer if NEED GET
> + * DATA is not enabled
> + */
> + if (!buf_addr && !ublk_need_get_data(ubq))
> + return -EINVAL;
> + } else if (buf_addr) {
> + /* User copy requires addr to be unset */
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> struct ublk_io *io, __u64 buf_addr)
> {
> @@ -2172,19 +2188,6 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
>
> WARN_ON_ONCE(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV);
>
> - if (ublk_need_map_io(ubq)) {
> - /*
> - * FETCH_RQ has to provide IO buffer if NEED GET
> - * DATA is not enabled
> - */
> - if (!buf_addr && !ublk_need_get_data(ubq))
> - goto out;
Was it a bug that this didn't set ret = -EINVAL before jumping to out?
Looks like ublk_fetch() would incorrectly skip initializing the
ublk_io and return 0 in this case. So probably this needs a Fixes tag.
Looks like the bug was introduced by the code movement in b69b8edfb27d
("ublk: properly serialize all FETCH_REQs").
Other than that,
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> - } else if (buf_addr) {
> - /* User copy requires addr to be unset */
> - ret = -EINVAL;
> - goto out;
> - }
> -
> ublk_fill_io_cmd(io, cmd, 0);
> ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
> if (ret)
> @@ -2297,6 +2300,9 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> io = &ubq->ios[tag];
> /* UBLK_IO_FETCH_REQ can be handled on any task, which sets io->task */
> if (unlikely(_IOC_NR(cmd_op) == UBLK_IO_FETCH_REQ)) {
> + ret = ublk_check_fetch_buf(ubq, ub_cmd->addr);
> + if (ret)
> + goto out;
> ret = ublk_fetch(cmd, ubq, io, ub_cmd->addr);
> if (ret)
> goto out;
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 07/16] ublk: add helper ublk_check_fetch_buf()
2025-07-08 12:31 ` Caleb Sander Mateos
@ 2025-07-13 14:13 ` Ming Lei
0 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-13 14:13 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Jens Axboe, linux-block, Uday Shankar
On Tue, Jul 08, 2025 at 08:31:05AM -0400, Caleb Sander Mateos wrote:
> On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Add helper ublk_check_fetch_buf() for checking buffer only.
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
>
> The commit message seems a bit sparse. How about something like:
> Add a helper ublk_check_fetch_buf() to validate UBLK_IO_FETCH_REQ's
> addr. This doesn't require access to the ublk_io, so it can be done
> before taking the ublk_device mutex.
OK, looks much better.
>
> > ---
> > drivers/block/ublk_drv.c | 32 +++++++++++++++++++-------------
> > 1 file changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 6d3aa08eef22..7fd2fa493d6a 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -2146,6 +2146,22 @@ static int ublk_unregister_io_buf(struct io_uring_cmd *cmd,
> > return io_buffer_unregister_bvec(cmd, index, issue_flags);
> > }
> >
> > +static int ublk_check_fetch_buf(const struct ublk_queue *ubq, __u64 buf_addr)
> > +{
> > + if (ublk_need_map_io(ubq)) {
> > + /*
> > + * FETCH_RQ has to provide IO buffer if NEED GET
> > + * DATA is not enabled
> > + */
> > + if (!buf_addr && !ublk_need_get_data(ubq))
> > + return -EINVAL;
> > + } else if (buf_addr) {
> > + /* User copy requires addr to be unset */
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> > struct ublk_io *io, __u64 buf_addr)
> > {
> > @@ -2172,19 +2188,6 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> >
> > WARN_ON_ONCE(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV);
> >
> > - if (ublk_need_map_io(ubq)) {
> > - /*
> > - * FETCH_RQ has to provide IO buffer if NEED GET
> > - * DATA is not enabled
> > - */
> > - if (!buf_addr && !ublk_need_get_data(ubq))
> > - goto out;
>
> Was it a bug that this didn't set ret = -EINVAL before jumping to out?
> Looks like ublk_fetch() would incorrectly skip initializing the
> ublk_io and return 0 in this case. So probably this needs a Fixes tag.
> Looks like the bug was introduced by the code movement in b69b8edfb27d
> ("ublk: properly serialize all FETCH_REQs").
Good catch!
Thanks,
Ming
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 08/16] ublk: remove ublk_commit_and_fetch()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (6 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 07/16] ublk: add helper ublk_check_fetch_buf() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-08 13:27 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 09/16] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io() Ming Lei
` (7 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Remove ublk_commit_and_fetch() and open code request completion.
Now request reference is stored in 'ublk_io', which becomes one global
variable, the motivation is to centralize access 'struct ublk_io' reference,
then we can introduce lock to protect `ublk_io` in future for supporting
io batch.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 7fd2fa493d6a..13c6b1e0e1ef 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -711,13 +711,12 @@ static inline void ublk_put_req_ref(struct ublk_io *io, struct request *req)
__ublk_complete_rq(req);
}
-static inline void ublk_sub_req_ref(struct ublk_io *io, struct request *req)
+static inline bool ublk_sub_req_ref(struct ublk_io *io, struct request *req)
{
unsigned sub_refs = UBLK_REFCOUNT_INIT - io->task_registered_buffers;
io->task_registered_buffers = 0;
- if (refcount_sub_and_test(sub_refs, &io->ref))
- __ublk_complete_rq(req);
+ return refcount_sub_and_test(sub_refs, &io->ref);
}
static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
@@ -2224,21 +2223,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
return 0;
}
-static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
- struct ublk_io *io, struct io_uring_cmd *cmd,
- struct request *req, unsigned int issue_flags,
- __u64 zone_append_lba, u16 buf_idx)
+static bool ublk_need_complete_req(const struct ublk_queue *ubq,
+ struct ublk_io *io,
+ struct request *req)
{
- if (buf_idx != UBLK_INVALID_BUF_IDX)
- io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
-
- if (req_op(req) == REQ_OP_ZONE_APPEND)
- req->__sector = zone_append_lba;
-
if (ublk_need_req_ref(ubq))
- ublk_sub_req_ref(io, req);
- else
- __ublk_complete_rq(req);
+ return ublk_sub_req_ref(io, req);
+ return true;
}
static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
@@ -2271,6 +2262,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
unsigned tag = ub_cmd->tag;
struct request *req;
int ret;
+ bool compl;
pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
__func__, cmd->cmd_op, ub_cmd->q_id, tag,
@@ -2347,8 +2339,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
goto out;
req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
- ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
- ub_cmd->zone_append_lba, buf_idx);
+ compl = ublk_need_complete_req(ubq, io, req);
+
+ /* can't touch 'ublk_io' any more */
+ if (buf_idx != UBLK_INVALID_BUF_IDX)
+ io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
+ if (req_op(req) == REQ_OP_ZONE_APPEND)
+ req->__sector = ub_cmd->zone_append_lba;
+ if (compl)
+ __ublk_complete_rq(req);
+
if (ret)
goto out;
break;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 08/16] ublk: remove ublk_commit_and_fetch()
2025-07-02 4:03 ` [PATCH 08/16] ublk: remove ublk_commit_and_fetch() Ming Lei
@ 2025-07-08 13:27 ` Caleb Sander Mateos
2025-07-09 11:56 ` Ming Lei
0 siblings, 1 reply; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-08 13:27 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Remove ublk_commit_and_fetch() and open code request completion.
>
> Now request reference is stored in 'ublk_io', which becomes one global
> variable, the motivation is to centralize access 'struct ublk_io' reference,
> then we can introduce lock to protect `ublk_io` in future for supporting
> io batch.
I didn't follow this. What do you mean by "global variable"?
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/block/ublk_drv.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 7fd2fa493d6a..13c6b1e0e1ef 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -711,13 +711,12 @@ static inline void ublk_put_req_ref(struct ublk_io *io, struct request *req)
> __ublk_complete_rq(req);
> }
>
> -static inline void ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> +static inline bool ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> {
> unsigned sub_refs = UBLK_REFCOUNT_INIT - io->task_registered_buffers;
>
> io->task_registered_buffers = 0;
> - if (refcount_sub_and_test(sub_refs, &io->ref))
> - __ublk_complete_rq(req);
> + return refcount_sub_and_test(sub_refs, &io->ref);
The struct request *req parameter can be removed now. Looks like it
can be removed from ublk_need_complete_req() too.
> }
>
> static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
> @@ -2224,21 +2223,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
> return 0;
> }
>
> -static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
> - struct ublk_io *io, struct io_uring_cmd *cmd,
> - struct request *req, unsigned int issue_flags,
> - __u64 zone_append_lba, u16 buf_idx)
> +static bool ublk_need_complete_req(const struct ublk_queue *ubq,
> + struct ublk_io *io,
> + struct request *req)
> {
> - if (buf_idx != UBLK_INVALID_BUF_IDX)
> - io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> -
> - if (req_op(req) == REQ_OP_ZONE_APPEND)
> - req->__sector = zone_append_lba;
> -
> if (ublk_need_req_ref(ubq))
> - ublk_sub_req_ref(io, req);
> - else
> - __ublk_complete_rq(req);
> + return ublk_sub_req_ref(io, req);
> + return true;
> }
>
> static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
> @@ -2271,6 +2262,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> unsigned tag = ub_cmd->tag;
> struct request *req;
> int ret;
> + bool compl;
>
> pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
> __func__, cmd->cmd_op, ub_cmd->q_id, tag,
> @@ -2347,8 +2339,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> goto out;
> req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
> ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
> - ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> - ub_cmd->zone_append_lba, buf_idx);
> + compl = ublk_need_complete_req(ubq, io, req);
> +
> + /* can't touch 'ublk_io' any more */
> + if (buf_idx != UBLK_INVALID_BUF_IDX)
> + io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> + if (req_op(req) == REQ_OP_ZONE_APPEND)
> + req->__sector = ub_cmd->zone_append_lba;
> + if (compl)
> + __ublk_complete_rq(req);
What is the benefit of separating the reference count decrement from
the call to __ublk_complete_rq()? I can understand if you want to keep
the code manipulating ublk_io separate from the code dispatching the
completed request. But it seems like this could be written more simply
as
if (ublk_need_complete_req(ubq, io, req))
__ublk_complete_rq(req);
Best,
Caleb
> +
> if (ret)
> goto out;
> break;
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/16] ublk: remove ublk_commit_and_fetch()
2025-07-08 13:27 ` Caleb Sander Mateos
@ 2025-07-09 11:56 ` Ming Lei
2025-07-11 14:05 ` Caleb Sander Mateos
0 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-09 11:56 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Jens Axboe, linux-block, Uday Shankar
On Tue, Jul 08, 2025 at 09:27:57AM -0400, Caleb Sander Mateos wrote:
> On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Remove ublk_commit_and_fetch() and open code request completion.
> >
> > Now request reference is stored in 'ublk_io', which becomes one global
> > variable, the motivation is to centralize access 'struct ublk_io' reference,
> > then we can introduce lock to protect `ublk_io` in future for supporting
> > io batch.
>
> I didn't follow this. What do you mean by "global variable"?
ublk server can send anything to driver with specified tag if batch io
extension is added and per-io task is relaxed, then 'ublk_io' instance can be
visible to any userpsace command, which needs protection, looks like one
global variable.
If reference is stored in request pdu, things becomes more like local
variable, since the early ublk_io flag check guarantees that concurrent
access can't reach 'request'.
>
> >
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > drivers/block/ublk_drv.c | 36 ++++++++++++++++++------------------
> > 1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 7fd2fa493d6a..13c6b1e0e1ef 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -711,13 +711,12 @@ static inline void ublk_put_req_ref(struct ublk_io *io, struct request *req)
> > __ublk_complete_rq(req);
> > }
> >
> > -static inline void ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> > +static inline bool ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> > {
> > unsigned sub_refs = UBLK_REFCOUNT_INIT - io->task_registered_buffers;
> >
> > io->task_registered_buffers = 0;
> > - if (refcount_sub_and_test(sub_refs, &io->ref))
> > - __ublk_complete_rq(req);
> > + return refcount_sub_and_test(sub_refs, &io->ref);
>
> The struct request *req parameter can be removed now. Looks like it
> can be removed from ublk_need_complete_req() too.
Good catch!
>
> > }
> >
> > static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
> > @@ -2224,21 +2223,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
> > return 0;
> > }
> >
> > -static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
> > - struct ublk_io *io, struct io_uring_cmd *cmd,
> > - struct request *req, unsigned int issue_flags,
> > - __u64 zone_append_lba, u16 buf_idx)
> > +static bool ublk_need_complete_req(const struct ublk_queue *ubq,
> > + struct ublk_io *io,
> > + struct request *req)
> > {
> > - if (buf_idx != UBLK_INVALID_BUF_IDX)
> > - io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> > -
> > - if (req_op(req) == REQ_OP_ZONE_APPEND)
> > - req->__sector = zone_append_lba;
> > -
> > if (ublk_need_req_ref(ubq))
> > - ublk_sub_req_ref(io, req);
> > - else
> > - __ublk_complete_rq(req);
> > + return ublk_sub_req_ref(io, req);
> > + return true;
> > }
> >
> > static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
> > @@ -2271,6 +2262,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> > unsigned tag = ub_cmd->tag;
> > struct request *req;
> > int ret;
> > + bool compl;
> >
> > pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
> > __func__, cmd->cmd_op, ub_cmd->q_id, tag,
> > @@ -2347,8 +2339,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> > goto out;
> > req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
> > ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
> > - ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> > - ub_cmd->zone_append_lba, buf_idx);
> > + compl = ublk_need_complete_req(ubq, io, req);
> > +
> > + /* can't touch 'ublk_io' any more */
> > + if (buf_idx != UBLK_INVALID_BUF_IDX)
> > + io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> > + if (req_op(req) == REQ_OP_ZONE_APPEND)
> > + req->__sector = ub_cmd->zone_append_lba;
> > + if (compl)
> > + __ublk_complete_rq(req);
>
> What is the benefit of separating the reference count decrement from
> the call to __ublk_complete_rq()? I can understand if you want to keep
> the code manipulating ublk_io separate from the code dispatching the
> completed request. But it seems like this could be written more simply
> as
>
> if (ublk_need_complete_req(ubq, io, req))
> __ublk_complete_rq(req);
ublk_need_complete_req() has to be protected, but the following code
(buffer unregister, complete req needn't and can't be covered with spin lock).
It is for putting anything needing protection in future together.
Thanks,
Ming
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/16] ublk: remove ublk_commit_and_fetch()
2025-07-09 11:56 ` Ming Lei
@ 2025-07-11 14:05 ` Caleb Sander Mateos
2025-07-13 14:14 ` Ming Lei
0 siblings, 1 reply; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-11 14:05 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 9, 2025 at 7:57 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Tue, Jul 08, 2025 at 09:27:57AM -0400, Caleb Sander Mateos wrote:
> > On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
> > >
> > > Remove ublk_commit_and_fetch() and open code request completion.
> > >
> > > Now request reference is stored in 'ublk_io', which becomes one global
> > > variable, the motivation is to centralize access 'struct ublk_io' reference,
> > > then we can introduce lock to protect `ublk_io` in future for supporting
> > > io batch.
> >
> > I didn't follow this. What do you mean by "global variable"?
>
> ublk server can send anything to driver with specified tag if batch io
> extension is added and per-io task is relaxed, then 'ublk_io' instance can be
> visible to any userpsace command, which needs protection, looks like one
> global variable.
>
> If reference is stored in request pdu, things becomes more like local
> variable, since the early ublk_io flag check guarantees that concurrent
> access can't reach 'request'.
"global variable" means something specific in C, so I would avoid
using it here to refer to something else. How about something like the
following?
Consolidate accesses to struct ublk_io in
UBLK_IO_COMMIT_AND_FETCH_REQ. When the ublk_io daemon task restriction
is relaxed in the future, ublk_io will need to be protected by a lock.
Unregister the auto-registered buffer and complete the request last,
as these don't need to happen under the lock.
Best,
Caleb
>
> >
> > >
> > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > ---
> > > drivers/block/ublk_drv.c | 36 ++++++++++++++++++------------------
> > > 1 file changed, 18 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index 7fd2fa493d6a..13c6b1e0e1ef 100644
> > > --- a/drivers/block/ublk_drv.c
> > > +++ b/drivers/block/ublk_drv.c
> > > @@ -711,13 +711,12 @@ static inline void ublk_put_req_ref(struct ublk_io *io, struct request *req)
> > > __ublk_complete_rq(req);
> > > }
> > >
> > > -static inline void ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> > > +static inline bool ublk_sub_req_ref(struct ublk_io *io, struct request *req)
> > > {
> > > unsigned sub_refs = UBLK_REFCOUNT_INIT - io->task_registered_buffers;
> > >
> > > io->task_registered_buffers = 0;
> > > - if (refcount_sub_and_test(sub_refs, &io->ref))
> > > - __ublk_complete_rq(req);
> > > + return refcount_sub_and_test(sub_refs, &io->ref);
> >
> > The struct request *req parameter can be removed now. Looks like it
> > can be removed from ublk_need_complete_req() too.
>
> Good catch!
>
> >
> > > }
> > >
> > > static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
> > > @@ -2224,21 +2223,13 @@ static int ublk_check_commit_and_fetch(const struct ublk_queue *ubq,
> > > return 0;
> > > }
> > >
> > > -static void ublk_commit_and_fetch(const struct ublk_queue *ubq,
> > > - struct ublk_io *io, struct io_uring_cmd *cmd,
> > > - struct request *req, unsigned int issue_flags,
> > > - __u64 zone_append_lba, u16 buf_idx)
> > > +static bool ublk_need_complete_req(const struct ublk_queue *ubq,
> > > + struct ublk_io *io,
> > > + struct request *req)
> > > {
> > > - if (buf_idx != UBLK_INVALID_BUF_IDX)
> > > - io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> > > -
> > > - if (req_op(req) == REQ_OP_ZONE_APPEND)
> > > - req->__sector = zone_append_lba;
> > > -
> > > if (ublk_need_req_ref(ubq))
> > > - ublk_sub_req_ref(io, req);
> > > - else
> > > - __ublk_complete_rq(req);
> > > + return ublk_sub_req_ref(io, req);
> > > + return true;
> > > }
> > >
> > > static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
> > > @@ -2271,6 +2262,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> > > unsigned tag = ub_cmd->tag;
> > > struct request *req;
> > > int ret;
> > > + bool compl;
> > >
> > > pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
> > > __func__, cmd->cmd_op, ub_cmd->q_id, tag,
> > > @@ -2347,8 +2339,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> > > goto out;
> > > req = ublk_fill_io_cmd(io, cmd, ub_cmd->result);
> > > ret = ublk_config_io_buf(ubq, io, cmd, ub_cmd->addr, &buf_idx);
> > > - ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
> > > - ub_cmd->zone_append_lba, buf_idx);
> > > + compl = ublk_need_complete_req(ubq, io, req);
> > > +
> > > + /* can't touch 'ublk_io' any more */
> > > + if (buf_idx != UBLK_INVALID_BUF_IDX)
> > > + io_buffer_unregister_bvec(cmd, buf_idx, issue_flags);
> > > + if (req_op(req) == REQ_OP_ZONE_APPEND)
> > > + req->__sector = ub_cmd->zone_append_lba;
> > > + if (compl)
> > > + __ublk_complete_rq(req);
> >
> > What is the benefit of separating the reference count decrement from
> > the call to __ublk_complete_rq()? I can understand if you want to keep
> > the code manipulating ublk_io separate from the code dispatching the
> > completed request. But it seems like this could be written more simply
> > as
> >
> > if (ublk_need_complete_req(ubq, io, req))
> > __ublk_complete_rq(req);
>
> ublk_need_complete_req() has to be protected, but the following code
> (buffer unregister, complete req needn't and can't be covered with spin lock).
>
> It is for putting anything needing protection in future together.
>
>
> Thanks,
> Ming
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/16] ublk: remove ublk_commit_and_fetch()
2025-07-11 14:05 ` Caleb Sander Mateos
@ 2025-07-13 14:14 ` Ming Lei
0 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-13 14:14 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Jens Axboe, linux-block, Uday Shankar
On Fri, Jul 11, 2025 at 10:05:13AM -0400, Caleb Sander Mateos wrote:
> On Wed, Jul 9, 2025 at 7:57 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Tue, Jul 08, 2025 at 09:27:57AM -0400, Caleb Sander Mateos wrote:
> > > On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
> > > >
> > > > Remove ublk_commit_and_fetch() and open code request completion.
> > > >
> > > > Now request reference is stored in 'ublk_io', which becomes one global
> > > > variable, the motivation is to centralize access 'struct ublk_io' reference,
> > > > then we can introduce lock to protect `ublk_io` in future for supporting
> > > > io batch.
> > >
> > > I didn't follow this. What do you mean by "global variable"?
> >
> > ublk server can send anything to driver with specified tag if batch io
> > extension is added and per-io task is relaxed, then 'ublk_io' instance can be
> > visible to any userpsace command, which needs protection, looks like one
> > global variable.
> >
> > If reference is stored in request pdu, things becomes more like local
> > variable, since the early ublk_io flag check guarantees that concurrent
> > access can't reach 'request'.
>
> "global variable" means something specific in C, so I would avoid
> using it here to refer to something else. How about something like the
> following?
>
> Consolidate accesses to struct ublk_io in
> UBLK_IO_COMMIT_AND_FETCH_REQ. When the ublk_io daemon task restriction
> is relaxed in the future, ublk_io will need to be protected by a lock.
> Unregister the auto-registered buffer and complete the request last,
> as these don't need to happen under the lock.
OK.
Thanks,
Ming
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 09/16] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (7 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 08/16] ublk: remove ublk_commit_and_fetch() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-08 13:29 ` Caleb Sander Mateos
2025-07-02 4:03 ` [PATCH 10/16] selftests: ublk: remove `tag` parameter of ->tgt_io_done() Ming Lei
` (6 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Pass 'const struct ublk_io *' to ublk_[un]map_io() since just io->addr
and io->res are read in the two helpers.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 13c6b1e0e1ef..3934254f7b99 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -993,7 +993,7 @@ static inline bool ublk_need_unmap_req(const struct request *req)
}
static int ublk_map_io(const struct ublk_queue *ubq, const struct request *req,
- struct ublk_io *io)
+ const struct ublk_io *io)
{
const unsigned int rq_bytes = blk_rq_bytes(req);
@@ -1017,7 +1017,7 @@ static int ublk_map_io(const struct ublk_queue *ubq, const struct request *req,
static int ublk_unmap_io(const struct ublk_queue *ubq,
const struct request *req,
- struct ublk_io *io)
+ const struct ublk_io *io)
{
const unsigned int rq_bytes = blk_rq_bytes(req);
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 09/16] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io()
2025-07-02 4:03 ` [PATCH 09/16] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io() Ming Lei
@ 2025-07-08 13:29 ` Caleb Sander Mateos
0 siblings, 0 replies; 29+ messages in thread
From: Caleb Sander Mateos @ 2025-07-08 13:29 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Uday Shankar
On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Pass 'const struct ublk_io *' to ublk_[un]map_io() since just io->addr
> and io->res are read in the two helpers.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
> drivers/block/ublk_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 13c6b1e0e1ef..3934254f7b99 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -993,7 +993,7 @@ static inline bool ublk_need_unmap_req(const struct request *req)
> }
>
> static int ublk_map_io(const struct ublk_queue *ubq, const struct request *req,
> - struct ublk_io *io)
> + const struct ublk_io *io)
> {
> const unsigned int rq_bytes = blk_rq_bytes(req);
>
> @@ -1017,7 +1017,7 @@ static int ublk_map_io(const struct ublk_queue *ubq, const struct request *req,
>
> static int ublk_unmap_io(const struct ublk_queue *ubq,
> const struct request *req,
> - struct ublk_io *io)
> + const struct ublk_io *io)
> {
> const unsigned int rq_bytes = blk_rq_bytes(req);
>
> --
> 2.47.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 10/16] selftests: ublk: remove `tag` parameter of ->tgt_io_done()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (8 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 09/16] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 11/16] selftests: ublk: pass 'ublk_thread *' to ->queue_io() and ->tgt_io_done() Ming Lei
` (5 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
The `tag` parameter can be figured out from cqe->user_data, and that is
also the only way to get the info, so remove `tag` parameter, and
let target code retrieve it from cqe->user_data.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/fault_inject.c | 3 ++-
tools/testing/selftests/ublk/file_backed.c | 3 ++-
tools/testing/selftests/ublk/kublk.c | 4 +---
tools/testing/selftests/ublk/kublk.h | 3 +--
tools/testing/selftests/ublk/null.c | 3 ++-
tools/testing/selftests/ublk/stripe.c | 3 ++-
6 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index 6e60f7d97125..c980958ec045 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -55,9 +55,10 @@ static int ublk_fault_inject_queue_io(struct ublk_queue *q, int tag)
return 0;
}
-static void ublk_fault_inject_tgt_io_done(struct ublk_queue *q, int tag,
+static void ublk_fault_inject_tgt_io_done(struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
+ unsigned tag = user_data_to_tag(cqe->user_data);
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
if (cqe->res != -ETIME)
diff --git a/tools/testing/selftests/ublk/file_backed.c b/tools/testing/selftests/ublk/file_backed.c
index cfa59b631693..02fb8a411d3b 100644
--- a/tools/testing/selftests/ublk/file_backed.c
+++ b/tools/testing/selftests/ublk/file_backed.c
@@ -108,9 +108,10 @@ static int ublk_loop_queue_io(struct ublk_queue *q, int tag)
return 0;
}
-static void ublk_loop_io_done(struct ublk_queue *q, int tag,
+static void ublk_loop_io_done(struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
+ unsigned tag = user_data_to_tag(cqe->user_data);
unsigned op = user_data_to_op(cqe->user_data);
struct ublk_io *io = ublk_get_io(q, tag);
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index e2d2042810d4..fba4e80e9bab 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -717,8 +717,6 @@ static int ublk_thread_is_done(struct ublk_thread *t)
static inline void ublksrv_handle_tgt_cqe(struct ublk_queue *q,
struct io_uring_cqe *cqe)
{
- unsigned tag = user_data_to_tag(cqe->user_data);
-
if (cqe->res < 0 && cqe->res != -EAGAIN)
ublk_err("%s: failed tgt io: res %d qid %u tag %u, cmd_op %u\n",
__func__, cqe->res, q->q_id,
@@ -726,7 +724,7 @@ static inline void ublksrv_handle_tgt_cqe(struct ublk_queue *q,
user_data_to_op(cqe->user_data));
if (q->tgt_ops->tgt_io_done)
- q->tgt_ops->tgt_io_done(q, tag, cqe);
+ q->tgt_ops->tgt_io_done(q, cqe);
}
static void ublk_handle_cqe(struct ublk_thread *t,
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 6be601536b3d..d7b711e63822 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -145,8 +145,7 @@ struct ublk_tgt_ops {
void (*deinit_tgt)(struct ublk_dev *);
int (*queue_io)(struct ublk_queue *, int tag);
- void (*tgt_io_done)(struct ublk_queue *,
- int tag, const struct io_uring_cqe *);
+ void (*tgt_io_done)(struct ublk_queue *, const struct io_uring_cqe *);
/*
* Target specific command line handling
diff --git a/tools/testing/selftests/ublk/null.c b/tools/testing/selftests/ublk/null.c
index afe0b99d77ee..ea3da53437e9 100644
--- a/tools/testing/selftests/ublk/null.c
+++ b/tools/testing/selftests/ublk/null.c
@@ -87,9 +87,10 @@ static int null_queue_auto_zc_io(struct ublk_queue *q, int tag)
return 1;
}
-static void ublk_null_io_done(struct ublk_queue *q, int tag,
+static void ublk_null_io_done(struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
+ unsigned tag = user_data_to_tag(cqe->user_data);
unsigned op = user_data_to_op(cqe->user_data);
struct ublk_io *io = ublk_get_io(q, tag);
diff --git a/tools/testing/selftests/ublk/stripe.c b/tools/testing/selftests/ublk/stripe.c
index 37d50bbf5f5e..53cefffaf32e 100644
--- a/tools/testing/selftests/ublk/stripe.c
+++ b/tools/testing/selftests/ublk/stripe.c
@@ -226,9 +226,10 @@ static int ublk_stripe_queue_io(struct ublk_queue *q, int tag)
return 0;
}
-static void ublk_stripe_io_done(struct ublk_queue *q, int tag,
+static void ublk_stripe_io_done(struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
+ unsigned tag = user_data_to_tag(cqe->user_data);
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
unsigned op = user_data_to_op(cqe->user_data);
struct ublk_io *io = ublk_get_io(q, tag);
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/16] selftests: ublk: pass 'ublk_thread *' to ->queue_io() and ->tgt_io_done()
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (9 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 10/16] selftests: ublk: remove `tag` parameter of ->tgt_io_done() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 12/16] selftests: ublk: pass 'ublk_thread *' to more common helpers Ming Lei
` (4 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
'struct thread' is task local structure, and the related code will become
more readable if we pass it via parameter.
Meantime pass 'ublk_thread *' to ublk_io_alloc_sqes(), and this way is
natural since we use per-thread io_uring for handling IO.
More importantly it helps much for removing the current ubq_daemon or
per-io-task limit.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/fault_inject.c | 8 ++++---
tools/testing/selftests/ublk/file_backed.c | 25 +++++++++++---------
tools/testing/selftests/ublk/kublk.c | 13 ++++++-----
tools/testing/selftests/ublk/kublk.h | 9 +++----
tools/testing/selftests/ublk/null.c | 21 ++++++++++-------
tools/testing/selftests/ublk/stripe.c | 26 ++++++++++++---------
6 files changed, 58 insertions(+), 44 deletions(-)
diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index c980958ec045..b4e9355e0eab 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -38,7 +38,8 @@ static int ublk_fault_inject_tgt_init(const struct dev_ctx *ctx,
return 0;
}
-static int ublk_fault_inject_queue_io(struct ublk_queue *q, int tag)
+static int ublk_fault_inject_queue_io(struct ublk_thread *t,
+ struct ublk_queue *q, int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
struct io_uring_sqe *sqe;
@@ -46,7 +47,7 @@ static int ublk_fault_inject_queue_io(struct ublk_queue *q, int tag)
.tv_nsec = (long long)q->dev->private_data,
};
- ublk_io_alloc_sqes(ublk_get_io(q, tag), &sqe, 1);
+ ublk_io_alloc_sqes(t, &sqe, 1);
io_uring_prep_timeout(sqe, &ts, 1, 0);
sqe->user_data = build_user_data(tag, ublksrv_get_op(iod), 0, q->q_id, 1);
@@ -55,7 +56,8 @@ static int ublk_fault_inject_queue_io(struct ublk_queue *q, int tag)
return 0;
}
-static void ublk_fault_inject_tgt_io_done(struct ublk_queue *q,
+static void ublk_fault_inject_tgt_io_done(struct ublk_thread *t,
+ struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
unsigned tag = user_data_to_tag(cqe->user_data);
diff --git a/tools/testing/selftests/ublk/file_backed.c b/tools/testing/selftests/ublk/file_backed.c
index 02fb8a411d3b..eeac7af2230f 100644
--- a/tools/testing/selftests/ublk/file_backed.c
+++ b/tools/testing/selftests/ublk/file_backed.c
@@ -13,12 +13,13 @@ static enum io_uring_op ublk_to_uring_op(const struct ublksrv_io_desc *iod, int
assert(0);
}
-static int loop_queue_flush_io(struct ublk_queue *q, const struct ublksrv_io_desc *iod, int tag)
+static int loop_queue_flush_io(struct ublk_thread *t, struct ublk_queue *q,
+ const struct ublksrv_io_desc *iod, int tag)
{
unsigned ublk_op = ublksrv_get_op(iod);
struct io_uring_sqe *sqe[1];
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, 1);
+ ublk_io_alloc_sqes(t, sqe, 1);
io_uring_prep_fsync(sqe[0], 1 /*fds[1]*/, IORING_FSYNC_DATASYNC);
io_uring_sqe_set_flags(sqe[0], IOSQE_FIXED_FILE);
/* bit63 marks us as tgt io */
@@ -26,7 +27,8 @@ static int loop_queue_flush_io(struct ublk_queue *q, const struct ublksrv_io_des
return 1;
}
-static int loop_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_desc *iod, int tag)
+static int loop_queue_tgt_rw_io(struct ublk_thread *t, struct ublk_queue *q,
+ const struct ublksrv_io_desc *iod, int tag)
{
unsigned ublk_op = ublksrv_get_op(iod);
unsigned zc = ublk_queue_use_zc(q);
@@ -36,7 +38,7 @@ static int loop_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_de
void *addr = (zc | auto_zc) ? NULL : (void *)iod->addr;
if (!zc || auto_zc) {
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, 1);
+ ublk_io_alloc_sqes(t, sqe, 1);
if (!sqe[0])
return -ENOMEM;
@@ -52,7 +54,7 @@ static int loop_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_de
return 1;
}
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, 3);
+ ublk_io_alloc_sqes(t, sqe, 3);
io_uring_prep_buf_register(sqe[0], 0, tag, q->q_id, ublk_get_io(q, tag)->buf_index);
sqe[0]->flags |= IOSQE_CQE_SKIP_SUCCESS | IOSQE_IO_HARDLINK;
@@ -72,7 +74,7 @@ static int loop_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_de
return 2;
}
-static int loop_queue_tgt_io(struct ublk_queue *q, int tag)
+static int loop_queue_tgt_io(struct ublk_thread *t, struct ublk_queue *q, int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
unsigned ublk_op = ublksrv_get_op(iod);
@@ -80,7 +82,7 @@ static int loop_queue_tgt_io(struct ublk_queue *q, int tag)
switch (ublk_op) {
case UBLK_IO_OP_FLUSH:
- ret = loop_queue_flush_io(q, iod, tag);
+ ret = loop_queue_flush_io(t, q, iod, tag);
break;
case UBLK_IO_OP_WRITE_ZEROES:
case UBLK_IO_OP_DISCARD:
@@ -88,7 +90,7 @@ static int loop_queue_tgt_io(struct ublk_queue *q, int tag)
break;
case UBLK_IO_OP_READ:
case UBLK_IO_OP_WRITE:
- ret = loop_queue_tgt_rw_io(q, iod, tag);
+ ret = loop_queue_tgt_rw_io(t, q, iod, tag);
break;
default:
ret = -EINVAL;
@@ -100,15 +102,16 @@ static int loop_queue_tgt_io(struct ublk_queue *q, int tag)
return ret;
}
-static int ublk_loop_queue_io(struct ublk_queue *q, int tag)
+static int ublk_loop_queue_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
- int queued = loop_queue_tgt_io(q, tag);
+ int queued = loop_queue_tgt_io(t, q, tag);
ublk_queued_tgt_io(q, tag, queued);
return 0;
}
-static void ublk_loop_io_done(struct ublk_queue *q,
+static void ublk_loop_io_done(struct ublk_thread *t, struct ublk_queue *q,
const struct io_uring_cqe *cqe)
{
unsigned tag = user_data_to_tag(cqe->user_data);
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index fba4e80e9bab..180b6da08eab 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -620,7 +620,7 @@ int ublk_queue_io_cmd(struct ublk_io *io)
if (io_uring_sq_space_left(&t->ring) < 1)
io_uring_submit(&t->ring);
- ublk_io_alloc_sqes(io, sqe, 1);
+ ublk_io_alloc_sqes(t, sqe, 1);
if (!sqe[0]) {
ublk_err("%s: run out of sqe. thread %u, tag %d\n",
__func__, t->idx, io->tag);
@@ -714,8 +714,9 @@ static int ublk_thread_is_done(struct ublk_thread *t)
return (t->state & UBLKSRV_THREAD_STOPPING) && ublk_thread_is_idle(t);
}
-static inline void ublksrv_handle_tgt_cqe(struct ublk_queue *q,
- struct io_uring_cqe *cqe)
+static inline void ublksrv_handle_tgt_cqe(struct ublk_thread *t,
+ struct ublk_queue *q,
+ struct io_uring_cqe *cqe)
{
if (cqe->res < 0 && cqe->res != -EAGAIN)
ublk_err("%s: failed tgt io: res %d qid %u tag %u, cmd_op %u\n",
@@ -724,7 +725,7 @@ static inline void ublksrv_handle_tgt_cqe(struct ublk_queue *q,
user_data_to_op(cqe->user_data));
if (q->tgt_ops->tgt_io_done)
- q->tgt_ops->tgt_io_done(q, cqe);
+ q->tgt_ops->tgt_io_done(t, q, cqe);
}
static void ublk_handle_cqe(struct ublk_thread *t,
@@ -751,7 +752,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
/* Don't retrieve io in case of target io */
if (is_target_io(cqe->user_data)) {
- ublksrv_handle_tgt_cqe(q, cqe);
+ ublksrv_handle_tgt_cqe(t, q, cqe);
return;
}
@@ -766,7 +767,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
if (cqe->res == UBLK_IO_RES_OK) {
assert(tag < q->q_depth);
if (q->tgt_ops->queue_io)
- q->tgt_ops->queue_io(q, tag);
+ q->tgt_ops->queue_io(t, q, tag);
} else if (cqe->res == UBLK_IO_RES_NEED_GET_DATA) {
io->flags |= UBLKSRV_NEED_GET_DATA | UBLKSRV_IO_FREE;
ublk_queue_io_cmd(io);
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index d7b711e63822..91e0286c5ca6 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -144,8 +144,9 @@ struct ublk_tgt_ops {
int (*init_tgt)(const struct dev_ctx *ctx, struct ublk_dev *);
void (*deinit_tgt)(struct ublk_dev *);
- int (*queue_io)(struct ublk_queue *, int tag);
- void (*tgt_io_done)(struct ublk_queue *, const struct io_uring_cqe *);
+ int (*queue_io)(struct ublk_thread *, struct ublk_queue *, int tag);
+ void (*tgt_io_done)(struct ublk_thread *, struct ublk_queue *,
+ const struct io_uring_cqe *);
/*
* Target specific command line handling
@@ -313,10 +314,10 @@ static inline struct ublk_queue *ublk_io_to_queue(const struct ublk_io *io)
return container_of(io, struct ublk_queue, ios[io->tag]);
}
-static inline int ublk_io_alloc_sqes(struct ublk_io *io,
+static inline int ublk_io_alloc_sqes(struct ublk_thread *t,
struct io_uring_sqe *sqes[], int nr_sqes)
{
- struct io_uring *ring = &io->t->ring;
+ struct io_uring *ring = &t->ring;
unsigned left = io_uring_sq_space_left(ring);
int i;
diff --git a/tools/testing/selftests/ublk/null.c b/tools/testing/selftests/ublk/null.c
index ea3da53437e9..e29a005fc1cc 100644
--- a/tools/testing/selftests/ublk/null.c
+++ b/tools/testing/selftests/ublk/null.c
@@ -55,12 +55,13 @@ static void __setup_nop_io(int tag, const struct ublksrv_io_desc *iod,
sqe->user_data = build_user_data(tag, ublk_op, 0, q_id, 1);
}
-static int null_queue_zc_io(struct ublk_queue *q, int tag)
+static int null_queue_zc_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
struct io_uring_sqe *sqe[3];
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, 3);
+ ublk_io_alloc_sqes(t, sqe, 3);
io_uring_prep_buf_register(sqe[0], 0, tag, q->q_id, ublk_get_io(q, tag)->buf_index);
sqe[0]->user_data = build_user_data(tag,
@@ -77,18 +78,19 @@ static int null_queue_zc_io(struct ublk_queue *q, int tag)
return 2;
}
-static int null_queue_auto_zc_io(struct ublk_queue *q, int tag)
+static int null_queue_auto_zc_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
struct io_uring_sqe *sqe[1];
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, 1);
+ ublk_io_alloc_sqes(t, sqe, 1);
__setup_nop_io(tag, iod, sqe[0], q->q_id);
return 1;
}
-static void ublk_null_io_done(struct ublk_queue *q,
- const struct io_uring_cqe *cqe)
+static void ublk_null_io_done(struct ublk_thread *t, struct ublk_queue *q,
+ const struct io_uring_cqe *cqe)
{
unsigned tag = user_data_to_tag(cqe->user_data);
unsigned op = user_data_to_op(cqe->user_data);
@@ -110,7 +112,8 @@ static void ublk_null_io_done(struct ublk_queue *q,
ublk_complete_io(q, tag, io->result);
}
-static int ublk_null_queue_io(struct ublk_queue *q, int tag)
+static int ublk_null_queue_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
unsigned auto_zc = ublk_queue_use_auto_zc(q);
@@ -118,9 +121,9 @@ static int ublk_null_queue_io(struct ublk_queue *q, int tag)
int queued;
if (auto_zc && !ublk_io_auto_zc_fallback(iod))
- queued = null_queue_auto_zc_io(q, tag);
+ queued = null_queue_auto_zc_io(t, q, tag);
else if (zc)
- queued = null_queue_zc_io(q, tag);
+ queued = null_queue_zc_io(t, q, tag);
else {
ublk_complete_io(q, tag, iod->nr_sectors << 9);
return 0;
diff --git a/tools/testing/selftests/ublk/stripe.c b/tools/testing/selftests/ublk/stripe.c
index 53cefffaf32e..462fab7492ce 100644
--- a/tools/testing/selftests/ublk/stripe.c
+++ b/tools/testing/selftests/ublk/stripe.c
@@ -123,7 +123,8 @@ static inline enum io_uring_op stripe_to_uring_op(
assert(0);
}
-static int stripe_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_desc *iod, int tag)
+static int stripe_queue_tgt_rw_io(struct ublk_thread *t, struct ublk_queue *q,
+ const struct ublksrv_io_desc *iod, int tag)
{
const struct stripe_conf *conf = get_chunk_shift(q);
unsigned auto_zc = (ublk_queue_use_auto_zc(q) != 0);
@@ -138,7 +139,7 @@ static int stripe_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_
io->private_data = s;
calculate_stripe_array(conf, iod, s, base);
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, s->nr + extra);
+ ublk_io_alloc_sqes(t, sqe, s->nr + extra);
if (zc) {
io_uring_prep_buf_register(sqe[0], 0, tag, q->q_id, io->buf_index);
@@ -176,13 +177,14 @@ static int stripe_queue_tgt_rw_io(struct ublk_queue *q, const struct ublksrv_io_
return s->nr + zc;
}
-static int handle_flush(struct ublk_queue *q, const struct ublksrv_io_desc *iod, int tag)
+static int handle_flush(struct ublk_thread *t, struct ublk_queue *q,
+ const struct ublksrv_io_desc *iod, int tag)
{
const struct stripe_conf *conf = get_chunk_shift(q);
struct io_uring_sqe *sqe[NR_STRIPE];
int i;
- ublk_io_alloc_sqes(ublk_get_io(q, tag), sqe, conf->nr_files);
+ ublk_io_alloc_sqes(t, sqe, conf->nr_files);
for (i = 0; i < conf->nr_files; i++) {
io_uring_prep_fsync(sqe[i], i + 1, IORING_FSYNC_DATASYNC);
io_uring_sqe_set_flags(sqe[i], IOSQE_FIXED_FILE);
@@ -191,7 +193,8 @@ static int handle_flush(struct ublk_queue *q, const struct ublksrv_io_desc *iod,
return conf->nr_files;
}
-static int stripe_queue_tgt_io(struct ublk_queue *q, int tag)
+static int stripe_queue_tgt_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
unsigned ublk_op = ublksrv_get_op(iod);
@@ -199,7 +202,7 @@ static int stripe_queue_tgt_io(struct ublk_queue *q, int tag)
switch (ublk_op) {
case UBLK_IO_OP_FLUSH:
- ret = handle_flush(q, iod, tag);
+ ret = handle_flush(t, q, iod, tag);
break;
case UBLK_IO_OP_WRITE_ZEROES:
case UBLK_IO_OP_DISCARD:
@@ -207,7 +210,7 @@ static int stripe_queue_tgt_io(struct ublk_queue *q, int tag)
break;
case UBLK_IO_OP_READ:
case UBLK_IO_OP_WRITE:
- ret = stripe_queue_tgt_rw_io(q, iod, tag);
+ ret = stripe_queue_tgt_rw_io(t, q, iod, tag);
break;
default:
ret = -EINVAL;
@@ -218,16 +221,17 @@ static int stripe_queue_tgt_io(struct ublk_queue *q, int tag)
return ret;
}
-static int ublk_stripe_queue_io(struct ublk_queue *q, int tag)
+static int ublk_stripe_queue_io(struct ublk_thread *t, struct ublk_queue *q,
+ int tag)
{
- int queued = stripe_queue_tgt_io(q, tag);
+ int queued = stripe_queue_tgt_io(t, q, tag);
ublk_queued_tgt_io(q, tag, queued);
return 0;
}
-static void ublk_stripe_io_done(struct ublk_queue *q,
- const struct io_uring_cqe *cqe)
+static void ublk_stripe_io_done(struct ublk_thread *t, struct ublk_queue *q,
+ const struct io_uring_cqe *cqe)
{
unsigned tag = user_data_to_tag(cqe->user_data);
const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/16] selftests: ublk: pass 'ublk_thread *' to more common helpers
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (10 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 11/16] selftests: ublk: pass 'ublk_thread *' to ->queue_io() and ->tgt_io_done() Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 13/16] selftests: ublk: remove ublk queue self-defined flags Ming Lei
` (3 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Pass 'ublk_thread *' to more common helpers, then we can avoid to store
this reference into 'struct ublk_io'.
Prepare for supporting to handle IO via different task context.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/fault_inject.c | 6 +++---
tools/testing/selftests/ublk/file_backed.c | 6 +++---
tools/testing/selftests/ublk/kublk.c | 11 ++++-------
tools/testing/selftests/ublk/kublk.h | 20 +++++++++++---------
tools/testing/selftests/ublk/null.c | 8 ++++----
tools/testing/selftests/ublk/stripe.c | 6 +++---
6 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index b4e9355e0eab..b227bd78b252 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -51,7 +51,7 @@ static int ublk_fault_inject_queue_io(struct ublk_thread *t,
io_uring_prep_timeout(sqe, &ts, 1, 0);
sqe->user_data = build_user_data(tag, ublksrv_get_op(iod), 0, q->q_id, 1);
- ublk_queued_tgt_io(q, tag, 1);
+ ublk_queued_tgt_io(t, q, tag, 1);
return 0;
}
@@ -66,8 +66,8 @@ static void ublk_fault_inject_tgt_io_done(struct ublk_thread *t,
if (cqe->res != -ETIME)
ublk_err("%s: unexpected cqe res %d\n", __func__, cqe->res);
- if (ublk_completed_tgt_io(q, tag))
- ublk_complete_io(q, tag, iod->nr_sectors << 9);
+ if (ublk_completed_tgt_io(t, q, tag))
+ ublk_complete_io(t, q, tag, iod->nr_sectors << 9);
else
ublk_err("%s: io not complete after 1 cqe\n", __func__);
}
diff --git a/tools/testing/selftests/ublk/file_backed.c b/tools/testing/selftests/ublk/file_backed.c
index eeac7af2230f..2d93ac860bd5 100644
--- a/tools/testing/selftests/ublk/file_backed.c
+++ b/tools/testing/selftests/ublk/file_backed.c
@@ -107,7 +107,7 @@ static int ublk_loop_queue_io(struct ublk_thread *t, struct ublk_queue *q,
{
int queued = loop_queue_tgt_io(t, q, tag);
- ublk_queued_tgt_io(q, tag, queued);
+ ublk_queued_tgt_io(t, q, tag, queued);
return 0;
}
@@ -130,8 +130,8 @@ static void ublk_loop_io_done(struct ublk_thread *t, struct ublk_queue *q,
if (op == ublk_cmd_op_nr(UBLK_U_IO_REGISTER_IO_BUF))
io->tgt_ios += 1;
- if (ublk_completed_tgt_io(q, tag))
- ublk_complete_io(q, tag, io->result);
+ if (ublk_completed_tgt_io(t, q, tag))
+ ublk_complete_io(t, q, tag, io->result);
}
static int ublk_loop_tgt_init(const struct dev_ctx *ctx, struct ublk_dev *dev)
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 180b6da08eab..944e0806ba05 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -589,9 +589,8 @@ static void ublk_set_auto_buf_reg(const struct ublk_queue *q,
sqe->addr = ublk_auto_buf_reg_to_sqe_addr(&buf);
}
-int ublk_queue_io_cmd(struct ublk_io *io)
+int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io)
{
- struct ublk_thread *t = io->t;
struct ublk_queue *q = ublk_io_to_queue(io);
struct ublksrv_io_cmd *cmd;
struct io_uring_sqe *sqe[1];
@@ -685,9 +684,8 @@ static void ublk_submit_fetch_commands(struct ublk_thread *t)
int tag = i % dinfo->queue_depth;
q = &t->dev->q[q_id];
io = &q->ios[tag];
- io->t = t;
io->buf_index = j++;
- ublk_queue_io_cmd(io);
+ ublk_queue_io_cmd(t, io);
}
} else {
/*
@@ -697,9 +695,8 @@ static void ublk_submit_fetch_commands(struct ublk_thread *t)
struct ublk_queue *q = &t->dev->q[t->idx];
for (i = 0; i < q->q_depth; i++) {
io = &q->ios[i];
- io->t = t;
io->buf_index = i;
- ublk_queue_io_cmd(io);
+ ublk_queue_io_cmd(t, io);
}
}
}
@@ -770,7 +767,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
q->tgt_ops->queue_io(t, q, tag);
} else if (cqe->res == UBLK_IO_RES_NEED_GET_DATA) {
io->flags |= UBLKSRV_NEED_GET_DATA | UBLKSRV_IO_FREE;
- ublk_queue_io_cmd(io);
+ ublk_queue_io_cmd(t, io);
} else {
/*
* COMMIT_REQ will be completed immediately since no fetching
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 91e0286c5ca6..a4049984b055 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -136,7 +136,6 @@ struct ublk_io {
unsigned short buf_index;
unsigned short tgt_ios;
void *private_data;
- struct ublk_thread *t;
};
struct ublk_tgt_ops {
@@ -232,7 +231,7 @@ struct ublk_dev {
extern unsigned int ublk_dbg_mask;
-extern int ublk_queue_io_cmd(struct ublk_io *io);
+extern int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io);
static inline int ublk_io_auto_zc_fallback(const struct ublksrv_io_desc *iod)
@@ -402,33 +401,36 @@ static inline struct ublk_io *ublk_get_io(struct ublk_queue *q, unsigned tag)
return &q->ios[tag];
}
-static inline int ublk_complete_io(struct ublk_queue *q, unsigned tag, int res)
+static inline int ublk_complete_io(struct ublk_thread *t, struct ublk_queue *q,
+ unsigned tag, int res)
{
struct ublk_io *io = &q->ios[tag];
ublk_mark_io_done(io, res);
- return ublk_queue_io_cmd(io);
+ return ublk_queue_io_cmd(t, io);
}
-static inline void ublk_queued_tgt_io(struct ublk_queue *q, unsigned tag, int queued)
+static inline void ublk_queued_tgt_io(struct ublk_thread *t, struct ublk_queue *q,
+ unsigned tag, int queued)
{
if (queued < 0)
- ublk_complete_io(q, tag, queued);
+ ublk_complete_io(t, q, tag, queued);
else {
struct ublk_io *io = ublk_get_io(q, tag);
- io->t->io_inflight += queued;
+ t->io_inflight += queued;
io->tgt_ios = queued;
io->result = 0;
}
}
-static inline int ublk_completed_tgt_io(struct ublk_queue *q, unsigned tag)
+static inline int ublk_completed_tgt_io(struct ublk_thread *t,
+ struct ublk_queue *q, unsigned tag)
{
struct ublk_io *io = ublk_get_io(q, tag);
- io->t->io_inflight--;
+ t->io_inflight--;
return --io->tgt_ios == 0;
}
diff --git a/tools/testing/selftests/ublk/null.c b/tools/testing/selftests/ublk/null.c
index e29a005fc1cc..452dcc369c8b 100644
--- a/tools/testing/selftests/ublk/null.c
+++ b/tools/testing/selftests/ublk/null.c
@@ -108,8 +108,8 @@ static void ublk_null_io_done(struct ublk_thread *t, struct ublk_queue *q,
if (op == ublk_cmd_op_nr(UBLK_U_IO_REGISTER_IO_BUF))
io->tgt_ios += 1;
- if (ublk_completed_tgt_io(q, tag))
- ublk_complete_io(q, tag, io->result);
+ if (ublk_completed_tgt_io(t, q, tag))
+ ublk_complete_io(t, q, tag, io->result);
}
static int ublk_null_queue_io(struct ublk_thread *t, struct ublk_queue *q,
@@ -125,10 +125,10 @@ static int ublk_null_queue_io(struct ublk_thread *t, struct ublk_queue *q,
else if (zc)
queued = null_queue_zc_io(t, q, tag);
else {
- ublk_complete_io(q, tag, iod->nr_sectors << 9);
+ ublk_complete_io(t, q, tag, iod->nr_sectors << 9);
return 0;
}
- ublk_queued_tgt_io(q, tag, queued);
+ ublk_queued_tgt_io(t, q, tag, queued);
return 0;
}
diff --git a/tools/testing/selftests/ublk/stripe.c b/tools/testing/selftests/ublk/stripe.c
index 462fab7492ce..1fb9b7cc281b 100644
--- a/tools/testing/selftests/ublk/stripe.c
+++ b/tools/testing/selftests/ublk/stripe.c
@@ -226,7 +226,7 @@ static int ublk_stripe_queue_io(struct ublk_thread *t, struct ublk_queue *q,
{
int queued = stripe_queue_tgt_io(t, q, tag);
- ublk_queued_tgt_io(q, tag, queued);
+ ublk_queued_tgt_io(t, q, tag, queued);
return 0;
}
@@ -262,13 +262,13 @@ static void ublk_stripe_io_done(struct ublk_thread *t, struct ublk_queue *q,
}
}
- if (ublk_completed_tgt_io(q, tag)) {
+ if (ublk_completed_tgt_io(t, q, tag)) {
int res = io->result;
if (!res)
res = iod->nr_sectors << 9;
- ublk_complete_io(q, tag, res);
+ ublk_complete_io(t, q, tag, res);
free_stripe_array(io->private_data);
io->private_data = NULL;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 13/16] selftests: ublk: remove ublk queue self-defined flags
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (11 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 12/16] selftests: ublk: pass 'ublk_thread *' to more common helpers Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 14/16] selftests: ublk: improve flags naming Ming Lei
` (2 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Remove ublk queue self-defined flags, and use the uapi flags directly.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/kublk.c | 25 +++++++++----------------
tools/testing/selftests/ublk/kublk.h | 22 +++++++++++++++-------
tools/testing/selftests/ublk/null.c | 2 +-
3 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 944e0806ba05..e90260468652 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -441,17 +441,10 @@ static int ublk_queue_init(struct ublk_queue *q, unsigned extra_flags)
unsigned long off;
q->tgt_ops = dev->tgt.ops;
- q->state = 0;
+ q->flags = 0;
q->q_depth = depth;
-
- if (dev->dev_info.flags & (UBLK_F_SUPPORT_ZERO_COPY | UBLK_F_AUTO_BUF_REG)) {
- q->state |= UBLKSRV_NO_BUF;
- if (dev->dev_info.flags & UBLK_F_SUPPORT_ZERO_COPY)
- q->state |= UBLKSRV_ZC;
- if (dev->dev_info.flags & UBLK_F_AUTO_BUF_REG)
- q->state |= UBLKSRV_AUTO_BUF_REG;
- }
- q->state |= extra_flags;
+ q->flags = dev->dev_info.flags;
+ q->flags |= extra_flags;
cmd_buf_size = ublk_queue_cmd_buf_sz(q);
off = UBLKSRV_CMD_BUF_OFFSET + q->q_id * ublk_queue_max_cmd_buf_sz();
@@ -469,7 +462,7 @@ static int ublk_queue_init(struct ublk_queue *q, unsigned extra_flags)
q->ios[i].flags = UBLKSRV_NEED_FETCH_RQ | UBLKSRV_IO_FREE;
q->ios[i].tag = i;
- if (q->state & UBLKSRV_NO_BUF)
+ if (ublk_queue_no_buf(q))
continue;
if (posix_memalign((void **)&q->ios[i].buf_addr,
@@ -583,7 +576,7 @@ static void ublk_set_auto_buf_reg(const struct ublk_queue *q,
else
buf.index = q->ios[tag].buf_index;
- if (q->state & UBLKSRV_AUTO_BUF_REG_FALLBACK)
+ if (ublk_queue_auto_zc_fallback(q))
buf.flags = UBLK_AUTO_BUF_REG_FALLBACK;
sqe->addr = ublk_auto_buf_reg_to_sqe_addr(&buf);
@@ -639,12 +632,12 @@ int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io)
sqe[0]->rw_flags = 0;
cmd->tag = io->tag;
cmd->q_id = q->q_id;
- if (!(q->state & UBLKSRV_NO_BUF))
+ if (!ublk_queue_no_buf(q))
cmd->addr = (__u64) (uintptr_t) io->buf_addr;
else
cmd->addr = 0;
- if (q->state & UBLKSRV_AUTO_BUF_REG)
+ if (ublk_queue_use_auto_zc(q))
ublk_set_auto_buf_reg(q, sqe[0], io->tag);
user_data = build_user_data(io->tag, _IOC_NR(cmd_op), 0, q->q_id, 0);
@@ -739,7 +732,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
if (cqe->res < 0 && cqe->res != -ENODEV)
ublk_err("%s: res %d userdata %llx queue state %x\n", __func__,
- cqe->res, cqe->user_data, q->state);
+ cqe->res, cqe->user_data, q->flags);
ublk_dbg(UBLK_DBG_IO_CMD, "%s: res %d (qid %d tag %u cmd_op %u target %d/%d) stopping %d\n",
__func__, cqe->res, q->q_id, tag, cmd_op,
@@ -911,7 +904,7 @@ static int ublk_start_daemon(const struct dev_ctx *ctx, struct ublk_dev *dev)
{
const struct ublksrv_ctrl_dev_info *dinfo = &dev->dev_info;
struct ublk_thread_info *tinfo;
- unsigned extra_flags = 0;
+ unsigned long long extra_flags = 0;
cpu_set_t *affinity_buf;
void *thread_ret;
sem_t ready;
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index a4049984b055..9ecb63bc930e 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -178,12 +178,10 @@ struct ublk_queue {
const struct ublk_tgt_ops *tgt_ops;
struct ublksrv_io_desc *io_cmd_buf;
+/* borrow one bit of ublk uapi flags, which may never be used */
+#define UBLKSRV_AUTO_BUF_REG_FALLBACK (1ULL << 63)
+ __u64 flags;
struct ublk_io ios[UBLK_QUEUE_DEPTH];
-#define UBLKSRV_NO_BUF (1U << 2)
-#define UBLKSRV_ZC (1U << 3)
-#define UBLKSRV_AUTO_BUF_REG (1U << 4)
-#define UBLKSRV_AUTO_BUF_REG_FALLBACK (1U << 5)
- unsigned state;
};
struct ublk_thread {
@@ -437,12 +435,22 @@ static inline int ublk_completed_tgt_io(struct ublk_thread *t,
static inline int ublk_queue_use_zc(const struct ublk_queue *q)
{
- return q->state & UBLKSRV_ZC;
+ return q->flags & UBLK_F_SUPPORT_ZERO_COPY;
}
static inline int ublk_queue_use_auto_zc(const struct ublk_queue *q)
{
- return q->state & UBLKSRV_AUTO_BUF_REG;
+ return q->flags & UBLK_F_AUTO_BUF_REG;
+}
+
+static inline int ublk_queue_auto_zc_fallback(const struct ublk_queue *q)
+{
+ return q->flags & UBLKSRV_AUTO_BUF_REG_FALLBACK;
+}
+
+static inline int ublk_queue_no_buf(const struct ublk_queue *q)
+{
+ return ublk_queue_use_zc(q) || ublk_queue_use_auto_zc(q);
}
extern const struct ublk_tgt_ops null_tgt_ops;
diff --git a/tools/testing/selftests/ublk/null.c b/tools/testing/selftests/ublk/null.c
index 452dcc369c8b..f0e0003a4860 100644
--- a/tools/testing/selftests/ublk/null.c
+++ b/tools/testing/selftests/ublk/null.c
@@ -138,7 +138,7 @@ static int ublk_null_queue_io(struct ublk_thread *t, struct ublk_queue *q,
*/
static unsigned short ublk_null_buf_index(const struct ublk_queue *q, int tag)
{
- if (q->state & UBLKSRV_AUTO_BUF_REG_FALLBACK)
+ if (ublk_queue_auto_zc_fallback(q))
return (unsigned short)-1;
return q->ios[tag].buf_index;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 14/16] selftests: ublk: improve flags naming
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (12 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 13/16] selftests: ublk: remove ublk queue self-defined flags Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 15/16] selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk command Ming Lei
2025-07-02 4:03 ` [PATCH 16/16] selftests: ublk: add utils.h Ming Lei
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Improve all kinds of flags naming by adding its host structure suffix for
making code more readable.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/kublk.c | 38 ++++++++++++++--------------
tools/testing/selftests/ublk/kublk.h | 23 ++++++++---------
2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index e90260468652..84307bc12f37 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -459,7 +459,7 @@ static int ublk_queue_init(struct ublk_queue *q, unsigned extra_flags)
io_buf_size = dev->dev_info.max_io_buf_bytes;
for (i = 0; i < q->q_depth; i++) {
q->ios[i].buf_addr = NULL;
- q->ios[i].flags = UBLKSRV_NEED_FETCH_RQ | UBLKSRV_IO_FREE;
+ q->ios[i].flags = UBLKS_IO_NEED_FETCH_RQ | UBLKS_IO_FREE;
q->ios[i].tag = i;
if (ublk_queue_no_buf(q))
@@ -591,7 +591,7 @@ int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io)
__u64 user_data;
/* only freed io can be issued */
- if (!(io->flags & UBLKSRV_IO_FREE))
+ if (!(io->flags & UBLKS_IO_FREE))
return 0;
/*
@@ -599,14 +599,14 @@ int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io)
* getting data
*/
if (!(io->flags &
- (UBLKSRV_NEED_FETCH_RQ | UBLKSRV_NEED_COMMIT_RQ_COMP | UBLKSRV_NEED_GET_DATA)))
+ (UBLKS_IO_NEED_FETCH_RQ | UBLKS_IO_NEED_COMMIT_RQ_COMP | UBLKS_IO_NEED_GET_DATA)))
return 0;
- if (io->flags & UBLKSRV_NEED_GET_DATA)
+ if (io->flags & UBLKS_IO_NEED_GET_DATA)
cmd_op = UBLK_U_IO_NEED_GET_DATA;
- else if (io->flags & UBLKSRV_NEED_COMMIT_RQ_COMP)
+ else if (io->flags & UBLKS_IO_NEED_COMMIT_RQ_COMP)
cmd_op = UBLK_U_IO_COMMIT_AND_FETCH_REQ;
- else if (io->flags & UBLKSRV_NEED_FETCH_RQ)
+ else if (io->flags & UBLKS_IO_NEED_FETCH_RQ)
cmd_op = UBLK_U_IO_FETCH_REQ;
if (io_uring_sq_space_left(&t->ring) < 1)
@@ -649,7 +649,7 @@ int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io)
ublk_dbg(UBLK_DBG_IO_CMD, "%s: (thread %u qid %d tag %u cmd_op %u) iof %x stopping %d\n",
__func__, t->idx, q->q_id, io->tag, cmd_op,
- io->flags, !!(t->state & UBLKSRV_THREAD_STOPPING));
+ io->flags, !!(t->state & UBLKS_T_STOPPING));
return 1;
}
@@ -701,7 +701,7 @@ static int ublk_thread_is_idle(struct ublk_thread *t)
static int ublk_thread_is_done(struct ublk_thread *t)
{
- return (t->state & UBLKSRV_THREAD_STOPPING) && ublk_thread_is_idle(t);
+ return (t->state & UBLKS_T_STOPPING) && ublk_thread_is_idle(t);
}
static inline void ublksrv_handle_tgt_cqe(struct ublk_thread *t,
@@ -727,7 +727,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
unsigned tag = user_data_to_tag(cqe->user_data);
unsigned cmd_op = user_data_to_op(cqe->user_data);
int fetch = (cqe->res != UBLK_IO_RES_ABORT) &&
- !(t->state & UBLKSRV_THREAD_STOPPING);
+ !(t->state & UBLKS_T_STOPPING);
struct ublk_io *io;
if (cqe->res < 0 && cqe->res != -ENODEV)
@@ -738,7 +738,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
__func__, cqe->res, q->q_id, tag, cmd_op,
is_target_io(cqe->user_data),
user_data_to_tgt_data(cqe->user_data),
- (t->state & UBLKSRV_THREAD_STOPPING));
+ (t->state & UBLKS_T_STOPPING));
/* Don't retrieve io in case of target io */
if (is_target_io(cqe->user_data)) {
@@ -750,8 +750,8 @@ static void ublk_handle_cqe(struct ublk_thread *t,
t->cmd_inflight--;
if (!fetch) {
- t->state |= UBLKSRV_THREAD_STOPPING;
- io->flags &= ~UBLKSRV_NEED_FETCH_RQ;
+ t->state |= UBLKS_T_STOPPING;
+ io->flags &= ~UBLKS_IO_NEED_FETCH_RQ;
}
if (cqe->res == UBLK_IO_RES_OK) {
@@ -759,7 +759,7 @@ static void ublk_handle_cqe(struct ublk_thread *t,
if (q->tgt_ops->queue_io)
q->tgt_ops->queue_io(t, q, tag);
} else if (cqe->res == UBLK_IO_RES_NEED_GET_DATA) {
- io->flags |= UBLKSRV_NEED_GET_DATA | UBLKSRV_IO_FREE;
+ io->flags |= UBLKS_IO_NEED_GET_DATA | UBLKS_IO_FREE;
ublk_queue_io_cmd(t, io);
} else {
/*
@@ -767,10 +767,10 @@ static void ublk_handle_cqe(struct ublk_thread *t,
* piggyback is required.
*
* Marking IO_FREE only, then this io won't be issued since
- * we only issue io with (UBLKSRV_IO_FREE | UBLKSRV_NEED_*)
+ * we only issue io with (UBLKS_IO_FREE | UBLKSRV_NEED_*)
*
* */
- io->flags = UBLKSRV_IO_FREE;
+ io->flags = UBLKS_IO_FREE;
}
}
@@ -797,7 +797,7 @@ static int ublk_process_io(struct ublk_thread *t)
t->dev->dev_info.dev_id,
t->idx, io_uring_sq_ready(&t->ring),
t->cmd_inflight,
- (t->state & UBLKSRV_THREAD_STOPPING));
+ (t->state & UBLKS_T_STOPPING));
if (ublk_thread_is_done(t))
return -ENODEV;
@@ -806,8 +806,8 @@ static int ublk_process_io(struct ublk_thread *t)
reapped = ublk_reap_events_uring(t);
ublk_dbg(UBLK_DBG_THREAD, "submit result %d, reapped %d stop %d idle %d\n",
- ret, reapped, (t->state & UBLKSRV_THREAD_STOPPING),
- (t->state & UBLKSRV_THREAD_IDLE));
+ ret, reapped, (t->state & UBLKS_T_STOPPING),
+ (t->state & UBLKS_T_IDLE));
return reapped;
}
@@ -926,7 +926,7 @@ static int ublk_start_daemon(const struct dev_ctx *ctx, struct ublk_dev *dev)
return ret;
if (ctx->auto_zc_fallback)
- extra_flags = UBLKSRV_AUTO_BUF_REG_FALLBACK;
+ extra_flags = UBLKS_Q_AUTO_BUF_REG_FALLBACK;
for (i = 0; i < dinfo->nr_hw_queues; i++) {
dev->q[i].dev = dev;
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 9ecb63bc930e..c668472097ff 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -45,9 +45,6 @@
#define UBLK_CTRL_RING_DEPTH 32
#define ERROR_EVTFD_DEVID -2
-/* queue idle timeout */
-#define UBLKSRV_IO_IDLE_SECS 20
-
#define UBLK_IO_MAX_BYTES (1 << 20)
#define UBLK_MAX_QUEUES_SHIFT 5
#define UBLK_MAX_QUEUES (1 << UBLK_MAX_QUEUES_SHIFT)
@@ -121,11 +118,11 @@ struct ublk_ctrl_cmd_data {
struct ublk_io {
char *buf_addr;
-#define UBLKSRV_NEED_FETCH_RQ (1UL << 0)
-#define UBLKSRV_NEED_COMMIT_RQ_COMP (1UL << 1)
-#define UBLKSRV_IO_FREE (1UL << 2)
-#define UBLKSRV_NEED_GET_DATA (1UL << 3)
-#define UBLKSRV_NEED_REG_BUF (1UL << 4)
+#define UBLKS_IO_NEED_FETCH_RQ (1UL << 0)
+#define UBLKS_IO_NEED_COMMIT_RQ_COMP (1UL << 1)
+#define UBLKS_IO_FREE (1UL << 2)
+#define UBLKS_IO_NEED_GET_DATA (1UL << 3)
+#define UBLKS_IO_NEED_REG_BUF (1UL << 4)
unsigned short flags;
unsigned short refs; /* used by target code only */
@@ -179,7 +176,7 @@ struct ublk_queue {
struct ublksrv_io_desc *io_cmd_buf;
/* borrow one bit of ublk uapi flags, which may never be used */
-#define UBLKSRV_AUTO_BUF_REG_FALLBACK (1ULL << 63)
+#define UBLKS_Q_AUTO_BUF_REG_FALLBACK (1ULL << 63)
__u64 flags;
struct ublk_io ios[UBLK_QUEUE_DEPTH];
};
@@ -193,8 +190,8 @@ struct ublk_thread {
pthread_t thread;
unsigned idx;
-#define UBLKSRV_THREAD_STOPPING (1U << 0)
-#define UBLKSRV_THREAD_IDLE (1U << 1)
+#define UBLKS_T_STOPPING (1U << 0)
+#define UBLKS_T_IDLE (1U << 1)
unsigned state;
};
@@ -377,7 +374,7 @@ static inline int ublk_get_io_res(const struct ublk_queue *q, unsigned tag)
static inline void ublk_mark_io_done(struct ublk_io *io, int res)
{
- io->flags |= (UBLKSRV_NEED_COMMIT_RQ_COMP | UBLKSRV_IO_FREE);
+ io->flags |= (UBLKS_IO_NEED_COMMIT_RQ_COMP | UBLKS_IO_FREE);
io->result = res;
}
@@ -445,7 +442,7 @@ static inline int ublk_queue_use_auto_zc(const struct ublk_queue *q)
static inline int ublk_queue_auto_zc_fallback(const struct ublk_queue *q)
{
- return q->flags & UBLKSRV_AUTO_BUF_REG_FALLBACK;
+ return q->flags & UBLKS_Q_AUTO_BUF_REG_FALLBACK;
}
static inline int ublk_queue_no_buf(const struct ublk_queue *q)
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 15/16] selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk command
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (13 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 14/16] selftests: ublk: improve flags naming Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
2025-07-02 4:03 ` [PATCH 16/16] selftests: ublk: add utils.h Ming Lei
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Add helper ublk_handle_uring_cmd() for handling ublk command, and make
ublk_handle_cqe() more readable.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/kublk.c | 61 ++++++++++++++++------------
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 84307bc12f37..95188065b2e9 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -718,36 +718,14 @@ static inline void ublksrv_handle_tgt_cqe(struct ublk_thread *t,
q->tgt_ops->tgt_io_done(t, q, cqe);
}
-static void ublk_handle_cqe(struct ublk_thread *t,
- struct io_uring_cqe *cqe, void *data)
+static void ublk_handle_uring_cmd(struct ublk_thread *t,
+ struct ublk_queue *q,
+ const struct io_uring_cqe *cqe)
{
- struct ublk_dev *dev = t->dev;
- unsigned q_id = user_data_to_q_id(cqe->user_data);
- struct ublk_queue *q = &dev->q[q_id];
- unsigned tag = user_data_to_tag(cqe->user_data);
- unsigned cmd_op = user_data_to_op(cqe->user_data);
int fetch = (cqe->res != UBLK_IO_RES_ABORT) &&
!(t->state & UBLKS_T_STOPPING);
- struct ublk_io *io;
-
- if (cqe->res < 0 && cqe->res != -ENODEV)
- ublk_err("%s: res %d userdata %llx queue state %x\n", __func__,
- cqe->res, cqe->user_data, q->flags);
-
- ublk_dbg(UBLK_DBG_IO_CMD, "%s: res %d (qid %d tag %u cmd_op %u target %d/%d) stopping %d\n",
- __func__, cqe->res, q->q_id, tag, cmd_op,
- is_target_io(cqe->user_data),
- user_data_to_tgt_data(cqe->user_data),
- (t->state & UBLKS_T_STOPPING));
-
- /* Don't retrieve io in case of target io */
- if (is_target_io(cqe->user_data)) {
- ublksrv_handle_tgt_cqe(t, q, cqe);
- return;
- }
-
- io = &q->ios[tag];
- t->cmd_inflight--;
+ unsigned tag = user_data_to_tag(cqe->user_data);
+ struct ublk_io *io = &q->ios[tag];
if (!fetch) {
t->state |= UBLKS_T_STOPPING;
@@ -774,6 +752,35 @@ static void ublk_handle_cqe(struct ublk_thread *t,
}
}
+static void ublk_handle_cqe(struct ublk_thread *t,
+ struct io_uring_cqe *cqe, void *data)
+{
+ struct ublk_dev *dev = t->dev;
+ unsigned q_id = user_data_to_q_id(cqe->user_data);
+ struct ublk_queue *q = &dev->q[q_id];
+ unsigned cmd_op = user_data_to_op(cqe->user_data);
+
+ if (cqe->res < 0 && cqe->res != -ENODEV)
+ ublk_err("%s: res %d userdata %llx queue state %x\n", __func__,
+ cqe->res, cqe->user_data, q->flags);
+
+ ublk_dbg(UBLK_DBG_IO_CMD, "%s: res %d (qid %d tag %u cmd_op %u target %d/%d) stopping %d\n",
+ __func__, cqe->res, q->q_id, user_data_to_tag(cqe->user_data),
+ cmd_op, is_target_io(cqe->user_data),
+ user_data_to_tgt_data(cqe->user_data),
+ (t->state & UBLKS_T_STOPPING));
+
+ /* Don't retrieve io in case of target io */
+ if (is_target_io(cqe->user_data)) {
+ ublksrv_handle_tgt_cqe(t, q, cqe);
+ return;
+ }
+
+ t->cmd_inflight--;
+
+ ublk_handle_uring_cmd(t, q, cqe);
+}
+
static int ublk_reap_events_uring(struct ublk_thread *t)
{
struct io_uring_cqe *cqe;
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 16/16] selftests: ublk: add utils.h
2025-07-02 4:03 [PATCH 00/16] ublk: cleanup for supporting batch IO command Ming Lei
` (14 preceding siblings ...)
2025-07-02 4:03 ` [PATCH 15/16] selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk command Ming Lei
@ 2025-07-02 4:03 ` Ming Lei
15 siblings, 0 replies; 29+ messages in thread
From: Ming Lei @ 2025-07-02 4:03 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Uday Shankar, Caleb Sander Mateos, Ming Lei
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
tools/testing/selftests/ublk/kublk.h | 64 +------------------------
tools/testing/selftests/ublk/utils.h | 70 ++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 62 deletions(-)
create mode 100644 tools/testing/selftests/ublk/utils.h
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index c668472097ff..219233f8a053 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -29,13 +29,9 @@
#include "ublk_dep.h"
#include <linux/ublk_cmd.h>
-#define __maybe_unused __attribute__((unused))
-#define MAX_BACK_FILES 4
-#ifndef min
-#define min(a, b) ((a) < (b) ? (a) : (b))
-#endif
+#include "utils.h"
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
+#define MAX_BACK_FILES 4
/****************** part 1: libublk ********************/
@@ -52,13 +48,6 @@
#define UBLK_MAX_THREADS (1 << UBLK_MAX_THREADS_SHIFT)
#define UBLK_QUEUE_DEPTH 1024
-#define UBLK_DBG_DEV (1U << 0)
-#define UBLK_DBG_THREAD (1U << 1)
-#define UBLK_DBG_IO_CMD (1U << 2)
-#define UBLK_DBG_IO (1U << 3)
-#define UBLK_DBG_CTRL_CMD (1U << 4)
-#define UBLK_LOG (1U << 5)
-
struct ublk_dev;
struct ublk_queue;
struct ublk_thread;
@@ -211,21 +200,6 @@ struct ublk_dev {
void *private_data;
};
-#ifndef offsetof
-#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
-#endif
-
-#ifndef container_of
-#define container_of(ptr, type, member) ({ \
- unsigned long __mptr = (unsigned long)(ptr); \
- ((type *)(__mptr - offsetof(type, member))); })
-#endif
-
-#define round_up(val, rnd) \
- (((val) + ((rnd) - 1)) & ~((rnd) - 1))
-
-
-extern unsigned int ublk_dbg_mask;
extern int ublk_queue_io_cmd(struct ublk_thread *t, struct ublk_io *io);
@@ -275,34 +249,6 @@ static inline unsigned short ublk_cmd_op_nr(unsigned int op)
return _IOC_NR(op);
}
-static inline void ublk_err(const char *fmt, ...)
-{
- va_list ap;
-
- va_start(ap, fmt);
- vfprintf(stderr, fmt, ap);
-}
-
-static inline void ublk_log(const char *fmt, ...)
-{
- if (ublk_dbg_mask & UBLK_LOG) {
- va_list ap;
-
- va_start(ap, fmt);
- vfprintf(stdout, fmt, ap);
- }
-}
-
-static inline void ublk_dbg(int level, const char *fmt, ...)
-{
- if (level & ublk_dbg_mask) {
- va_list ap;
-
- va_start(ap, fmt);
- vfprintf(stdout, fmt, ap);
- }
-}
-
static inline struct ublk_queue *ublk_io_to_queue(const struct ublk_io *io)
{
return container_of(io, struct ublk_queue, ios[io->tag]);
@@ -458,10 +404,4 @@ extern const struct ublk_tgt_ops fault_inject_tgt_ops;
void backing_file_tgt_deinit(struct ublk_dev *dev);
int backing_file_tgt_init(struct ublk_dev *dev);
-static inline unsigned int ilog2(unsigned int x)
-{
- if (x == 0)
- return 0;
- return (sizeof(x) * 8 - 1) - __builtin_clz(x);
-}
#endif
diff --git a/tools/testing/selftests/ublk/utils.h b/tools/testing/selftests/ublk/utils.h
new file mode 100644
index 000000000000..36545d1567f1
--- /dev/null
+++ b/tools/testing/selftests/ublk/utils.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef KUBLK_UTILS_H
+#define KUBLK_UTILS_H
+
+#define __maybe_unused __attribute__((unused))
+
+#ifndef min
+#define min(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
+
+#ifndef offsetof
+#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
+#endif
+
+#ifndef container_of
+#define container_of(ptr, type, member) ({ \
+ unsigned long __mptr = (unsigned long)(ptr); \
+ ((type *)(__mptr - offsetof(type, member))); })
+#endif
+
+#define round_up(val, rnd) \
+ (((val) + ((rnd) - 1)) & ~((rnd) - 1))
+
+static inline unsigned int ilog2(unsigned int x)
+{
+ if (x == 0)
+ return 0;
+ return (sizeof(x) * 8 - 1) - __builtin_clz(x);
+}
+
+#define UBLK_DBG_DEV (1U << 0)
+#define UBLK_DBG_THREAD (1U << 1)
+#define UBLK_DBG_IO_CMD (1U << 2)
+#define UBLK_DBG_IO (1U << 3)
+#define UBLK_DBG_CTRL_CMD (1U << 4)
+#define UBLK_LOG (1U << 5)
+
+extern unsigned int ublk_dbg_mask;
+
+static inline void ublk_err(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(stderr, fmt, ap);
+}
+
+static inline void ublk_log(const char *fmt, ...)
+{
+ if (ublk_dbg_mask & UBLK_LOG) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(stdout, fmt, ap);
+ }
+}
+
+static inline void ublk_dbg(int level, const char *fmt, ...)
+{
+ if (level & ublk_dbg_mask) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(stdout, fmt, ap);
+ }
+}
+
+#endif
--
2.47.0
^ permalink raw reply related [flat|nested] 29+ messages in thread