[PATCH V3 00/17] ublk: cleanup for supporting batch IO command

[PATCH V3 00/17] ublk: cleanup for supporting batch IO command

[Ming Lei]

Hi Jens,

The 1st 9 patches cleans ublk driver, and prepare for supporting
batch IO command which needs per-io lock.

The others are selftest cleanup, and prepare for supporting arbitrary
task context & ublk queue combination, which will be allowed with
batch IO feature.

ublk Batch IO feature introduction:

	- use per-queue multshot uring_cmd for fetching incoming io commands,
	and io command tag is saved to provided buffer

	- use per-queue uring_cmd for completing io command result, and io tag
	& result are filled in uring_cmd buffer

	- this way improves communication efficiency, also:

		- allows each queue to be handled in any pthread contexts, and each
		pthread context can handle any number of queues, and driver
		doesn't care ublk server context any more

		- help to apply blk-mq batch optimization in future

		- help to support io polling in future

	- github:

		https://github.com/ming1/linux/commits/ublk2-cmd-batch.v3/

Almost all feedback are from Caleb, and great thanks Caleb's review!

V3:	
	- add patch "ublk: validate ublk server pid"
	- clean "ublk: let ublk_fill_io_cmd() cover more things" by not setting
	  io->res in ublk_fill_io_cmd()
	- improve commit log
	- misc patch style fix
	- add reviewed-by tag

V2:
	- remove one unnecessary check (Caleb Sander Mateos)
	- add reviewed-by tag
	- rebase on latest for-6.17/block


Ming Lei (17):
  ublk: validate ublk server pid
  ublk: look up ublk task via its pid in timeout handler
  ublk: move fake timeout logic into __ublk_complete_rq()
  ublk: let ublk_fill_io_cmd() cover more things
  ublk: avoid to pass `struct ublksrv_io_cmd *` to
    ublk_commit_and_fetch()
  ublk: move auto buffer register handling into one dedicated helper
  ublk: store auto buffer register data into `struct ublk_io`
  ublk: add helper ublk_check_fetch_buf()
  ublk: remove ublk_commit_and_fetch()
  ublk: pass 'const struct ublk_io *' to ublk_[un]map_io()
  selftests: ublk: remove `tag` parameter of ->tgt_io_done()
  selftests: ublk: pass 'ublk_thread *' to ->queue_io() and
    ->tgt_io_done()
  selftests: ublk: pass 'ublk_thread *' to more common helpers
  selftests: ublk: remove ublk queue self-defined flags
  selftests: ublk: improve flags naming
  selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk
    command
  selftests: ublk: add utils.h

 drivers/block/ublk_drv.c                    | 254 ++++++++++++--------
 tools/testing/selftests/ublk/fault_inject.c |  15 +-
 tools/testing/selftests/ublk/file_backed.c  |  32 +--
 tools/testing/selftests/ublk/kublk.c        | 140 ++++++-----
 tools/testing/selftests/ublk/kublk.h        | 135 ++++-------
 tools/testing/selftests/ublk/null.c         |  32 +--
 tools/testing/selftests/ublk/stripe.c       |  33 +--
 tools/testing/selftests/ublk/utils.h        |  70 ++++++
 8 files changed, 391 insertions(+), 320 deletions(-)
 create mode 100644 tools/testing/selftests/ublk/utils.h

-- 
2.47.0

[PATCH V3 01/17] ublk: validate ublk server pid

[Ming Lei]

ublk server pid(the `tgid` of the process opening the ublk device) is stored
in `ublk_device->ublksrv_tgid`. This `tgid` is then checked against the
`ublksrv_pid` in `ublk_ctrl_start_dev` and `ublk_ctrl_end_recovery`.

This ensures that correct ublk server pid is stored in device info.

Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index a1a700c7e67a..2b894de29823 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -237,6 +237,7 @@ struct ublk_device {
 	unsigned int		nr_privileged_daemon;
 	struct mutex cancel_mutex;
 	bool canceling;
+	pid_t 	ublksrv_tgid;
 };
 
 /* header of ublk_params */
@@ -1528,6 +1529,7 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
 	if (test_and_set_bit(UB_STATE_OPEN, &ub->state))
 		return -EBUSY;
 	filp->private_data = ub;
+	ub->ublksrv_tgid = current->tgid;
 	return 0;
 }
 
@@ -1542,6 +1544,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
 	ub->mm = NULL;
 	ub->nr_queues_ready = 0;
 	ub->nr_privileged_daemon = 0;
+	ub->ublksrv_tgid = -1;
 }
 
 static struct gendisk *ublk_get_disk(struct ublk_device *ub)
@@ -2820,6 +2823,9 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
 	if (wait_for_completion_interruptible(&ub->completion) != 0)
 		return -EINTR;
 
+	if (ub->ublksrv_tgid != ublksrv_pid)
+		return -EINVAL;
+
 	mutex_lock(&ub->mutex);
 	if (ub->dev_info.state == UBLK_S_DEV_LIVE ||
 	    test_bit(UB_STATE_USED, &ub->state)) {
@@ -3321,6 +3327,9 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 	pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
 		 header->dev_id);
 
+	if (ub->ublksrv_tgid != ublksrv_pid)
+		return -EINVAL;
+
 	mutex_lock(&ub->mutex);
 	if (ublk_nosrv_should_stop_dev(ub))
 		goto out_unlock;
-- 
2.47.0

[PATCH V3 02/17] ublk: look up ublk task via its pid in timeout handler

[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 | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2b894de29823..7d1d8bd979c5 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1368,14 +1368,23 @@ 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;
+	pid_t tgid = ubq->dev->ublksrv_tgid;
+	struct task_struct *p;
+	struct pid *pid;
+
+	if (!(ubq->flags & UBLK_F_UNPRIVILEGED_DEV))
+		return BLK_EH_RESET_TIMER;
+
+	if (unlikely(!tgid))
+		return BLK_EH_RESET_TIMER;
+
+	rcu_read_lock();
+	pid = find_vpid(tgid);
+	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

[PATCH V3 03/17] ublk: move fake timeout logic into __ublk_complete_rq()

[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.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
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 7d1d8bd979c5..73c6c8d3b117 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1155,7 +1155,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;
@@ -2237,9 +2237,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

[PATCH V3 04/17] ublk: let ublk_fill_io_cmd() cover more things

[Ming Lei]

Let ublk_fill_io_cmd() clear UBLK_IO_FLAG_OWNED_BY_SRV too.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 73c6c8d3b117..f251517baea3 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2014,6 +2014,8 @@ static inline void ublk_fill_io_cmd(struct ublk_io *io,
 {
 	io->cmd = cmd;
 	io->flags |= UBLK_IO_FLAG_ACTIVE;
+	/* now this cmd slot is owned by ublk driver */
+	io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
 	io->addr = buf_addr;
 }
 
@@ -2229,9 +2231,6 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
 	}
 
 	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;
 
 	if (req_op(req) == REQ_OP_ZONE_APPEND)
@@ -2353,7 +2352,6 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
 		 */
 		req = io->req;
 		ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
-		io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
 		if (likely(ublk_get_data(ubq, io, req))) {
 			__ublk_prep_compl_io_cmd(io, req);
 			return UBLK_IO_RES_OK;
-- 
2.47.0

[PATCH V3 05/17] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch()

[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.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 44 ++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index f251517baea3..62393c64f17d 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2009,14 +2009,20 @@ 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)
+/* 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)
 {
+	struct request *req = io->req;
+
 	io->cmd = cmd;
 	io->flags |= UBLK_IO_FLAG_ACTIVE;
 	/* now this cmd slot is owned by ublk driver */
 	io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
 	io->addr = buf_addr;
+
+	return req;
 }
 
 static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
@@ -2182,10 +2188,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;
 
@@ -2194,10 +2198,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
@@ -2205,6 +2209,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;
 
@@ -2230,11 +2242,8 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
 			return ret;
 	}
 
-	ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
-	io->res = 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);
@@ -2340,7 +2349,13 @@ 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;
+		io->res = ub_cmd->result;
+		req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
+		ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
+					    ub_cmd->zone_append_lba);
 		if (ret)
 			goto out;
 		break;
@@ -2350,8 +2365,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);
+		req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
 		if (likely(ublk_get_data(ubq, io, req))) {
 			__ublk_prep_compl_io_cmd(io, req);
 			return UBLK_IO_RES_OK;
-- 
2.47.0

[PATCH V3 06/17] ublk: move auto buffer register handling into one dedicated helper

[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.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 127 ++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 56 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 62393c64f17d..f70fab36fbc7 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 	((u16)-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)
@@ -2009,10 +2011,47 @@ 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 = 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)
+ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd)
 {
 	struct request *req = io->req;
 
@@ -2020,11 +2059,22 @@ ublk_fill_io_cmd(struct ublk_io *io, struct io_uring_cmd *cmd,
 	io->flags |= UBLK_IO_FLAG_ACTIVE;
 	/* now this cmd slot is owned by ublk driver */
 	io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
-	io->addr = buf_addr;
 
 	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))
+		return ublk_handle_auto_buf_reg(io, cmd, buf_idx);
+
+	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)
@@ -2040,20 +2090,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;
@@ -2174,13 +2210,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);
+	ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
+	if (ret)
+		goto out;
 
-	ublk_fill_io_cmd(io, cmd, buf_addr);
 	WRITE_ONCE(io->task, get_task_struct(current));
 	ublk_mark_io_ready(ub, ubq);
 out:
@@ -2212,35 +2246,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;
@@ -2249,7 +2261,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,
@@ -2274,6 +2285,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;
@@ -2353,9 +2365,10 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
 		if (ret)
 			goto out;
 		io->res = ub_cmd->result;
-		req = ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
-		ret = ublk_commit_and_fetch(ubq, io, cmd, req, issue_flags,
-					    ub_cmd->zone_append_lba);
+		req = ublk_fill_io_cmd(io, cmd);
+		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;
@@ -2365,7 +2378,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);
+		req = ublk_fill_io_cmd(io, cmd);
+		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

[PATCH V3 07/17] ublk: store auto buffer register data into `struct ublk_io`

[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.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
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 f70fab36fbc7..c69d4fafc6cc 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;
 
@@ -1217,13 +1216,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;
 		}
@@ -1233,8 +1231,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;
 }
@@ -2011,16 +2007,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;
 }
@@ -2043,10 +2037,10 @@ static int ublk_handle_auto_buf_reg(struct ublk_io *io,
 		 * this ublk request gets stuck.
 		 */
 		if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd))
-			*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

[PATCH V3 08/17] ublk: add helper ublk_check_fetch_buf()

[Ming Lei]

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.

This way also fixes one missing return value of -EINVAL in case of early
failure from ublk_fetch().

Fixes: b69b8edfb27d ("ublk: properly serialize all FETCH_REQs")
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
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 c69d4fafc6cc..1b22fd5f5610 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2165,6 +2165,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)
 {
@@ -2191,19 +2207,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);
 	ret = ublk_config_io_buf(ubq, io, cmd, buf_addr, NULL);
 	if (ret)
@@ -2316,6 +2319,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

[PATCH V3 09/17] ublk: remove ublk_commit_and_fetch()

[Ming Lei]

Remove ublk_commit_and_fetch() and open code request completion.

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.

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 1b22fd5f5610..252cae345b3a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -714,13 +714,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)
@@ -2243,21 +2242,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,
@@ -2290,6 +2281,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,
@@ -2367,8 +2359,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
 		io->res = ub_cmd->result;
 		req = ublk_fill_io_cmd(io, cmd);
 		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

[PATCH V3 10/17] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io()

[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.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
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 252cae345b3a..d8b378ad6872 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -996,7 +996,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);
 
@@ -1020,7 +1020,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

[PATCH V3 11/17] selftests: ublk: remove `tag` parameter of ->tgt_io_done()

[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

[PATCH V3 12/17] selftests: ublk: pass 'ublk_thread *' to ->queue_io() and ->tgt_io_done()

[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

[PATCH V3 13/17] selftests: ublk: pass 'ublk_thread *' to more common helpers

[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

[PATCH V3 14/17] selftests: ublk: remove ublk queue self-defined flags

[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

[PATCH V3 15/17] selftests: ublk: improve flags naming

[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

[PATCH V3 16/17] selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk command

[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

[PATCH V3 17/17] selftests: ublk: add utils.h

[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

Re: [PATCH V3 00/17] ublk: cleanup for supporting batch IO command

[Jens Axboe]

On Sun, 13 Jul 2025 22:33:55 +0800, Ming Lei wrote:
> The 1st 9 patches cleans ublk driver, and prepare for supporting
> batch IO command which needs per-io lock.
> 
> The others are selftest cleanup, and prepare for supporting arbitrary
> task context & ublk queue combination, which will be allowed with
> batch IO feature.
> 
> [...]

Applied, thanks!

[01/17] ublk: validate ublk server pid
        commit: c2c8089f325ed703fd5123b39e2dece1dd605904
[02/17] ublk: look up ublk task via its pid in timeout handler
        commit: dd7a8507319e22141fa2e107d81cba18a4007d92
[03/17] ublk: move fake timeout logic into __ublk_complete_rq()
        commit: 7074feeca41d09713d70e619a34d9e7b4e219f8c
[04/17] ublk: let ublk_fill_io_cmd() cover more things
        commit: 07bc706431799e7cf5209e8afdd8071d400266e7
[05/17] ublk: avoid to pass `struct ublksrv_io_cmd *` to ublk_commit_and_fetch()
        commit: 7ebdba87cf2a248aad10aff6b5fb1ca7c6b0add7
[06/17] ublk: move auto buffer register handling into one dedicated helper
        commit: 52460dda3a775a73f226312b43c0a0211e8665ea
[07/17] ublk: store auto buffer register data into `struct ublk_io`
        commit: 21bb9facb1e78c50cf8bd5a51571fb8cbec3fb9d
[08/17] ublk: add helper ublk_check_fetch_buf()
        commit: 3446583f81fc3b98dddaac15b37d9c4ff2b569af
[09/17] ublk: remove ublk_commit_and_fetch()
        commit: b749965edda8fcf0fd3e188c56845e991eaa63c9
[10/17] ublk: pass 'const struct ublk_io *' to ublk_[un]map_io()
        commit: ef92541d99c1c1319e5254d5f5380959962abb87
[11/17] selftests: ublk: remove `tag` parameter of ->tgt_io_done()
        commit: b36c73251aaec6c9941b5493637a9007d0a56616
[12/17] selftests: ublk: pass 'ublk_thread *' to ->queue_io() and ->tgt_io_done()
        commit: e0054835bf6850245e17417fdbe80e232737e537
[13/17] selftests: ublk: pass 'ublk_thread *' to more common helpers
        commit: 92dda98424feebb5f9b9135e30219342b80791b3
[14/17] selftests: ublk: remove ublk queue self-defined flags
        commit: c3a6d48f86da1df277ef4f95f147a7f7f5cd6f44
[15/17] selftests: ublk: improve flags naming
        commit: a66f89017673881e85e65a460cfdec35da4f07f2
[16/17] selftests: ublk: add helper ublk_handle_uring_cmd() for handle ublk command
        commit: c1dc9b0d9e48380c868acd338c8912ebb7d75f0a
[17/17] selftests: ublk: add utils.h
        commit: e56828f4df139b49cb837198ef8f3d2f13db65cb

Best regards,
-- 
Jens Axboe

Re: [PATCH V3 01/17] ublk: validate ublk server pid

[Caleb Sander Mateos]

On Sun, Jul 13, 2025 at 10:34 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> ublk server pid(the `tgid` of the process opening the ublk device) is stored
> in `ublk_device->ublksrv_tgid`. This `tgid` is then checked against the
> `ublksrv_pid` in `ublk_ctrl_start_dev` and `ublk_ctrl_end_recovery`.
>
> This ensures that correct ublk server pid is stored in device info.
>
> Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/block/ublk_drv.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index a1a700c7e67a..2b894de29823 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -237,6 +237,7 @@ struct ublk_device {
>         unsigned int            nr_privileged_daemon;
>         struct mutex cancel_mutex;
>         bool canceling;
> +       pid_t   ublksrv_tgid;
>  };
>
>  /* header of ublk_params */
> @@ -1528,6 +1529,7 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
>         if (test_and_set_bit(UB_STATE_OPEN, &ub->state))
>                 return -EBUSY;
>         filp->private_data = ub;
> +       ub->ublksrv_tgid = current->tgid;
>         return 0;
>  }
>
> @@ -1542,6 +1544,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
>         ub->mm = NULL;
>         ub->nr_queues_ready = 0;
>         ub->nr_privileged_daemon = 0;
> +       ub->ublksrv_tgid = -1;

Should this be reset to 0? The next patch checks whether ublksrv_tgid
is 0 in ublk_timeout(). Also, the accesses to it should probably be
using {READ,WRITE}_ONCE() since ublk server open/close can happen
concurrently with ublk I/O timeout handling.

Best,
Caleb

>  }
>
>  static struct gendisk *ublk_get_disk(struct ublk_device *ub)
> @@ -2820,6 +2823,9 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>         if (wait_for_completion_interruptible(&ub->completion) != 0)
>                 return -EINTR;
>
> +       if (ub->ublksrv_tgid != ublksrv_pid)
> +               return -EINVAL;
> +
>         mutex_lock(&ub->mutex);
>         if (ub->dev_info.state == UBLK_S_DEV_LIVE ||
>             test_bit(UB_STATE_USED, &ub->state)) {
> @@ -3321,6 +3327,9 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
>         pr_devel("%s: All FETCH_REQs received, dev id %d\n", __func__,
>                  header->dev_id);
>
> +       if (ub->ublksrv_tgid != ublksrv_pid)
> +               return -EINVAL;
> +
>         mutex_lock(&ub->mutex);
>         if (ublk_nosrv_should_stop_dev(ub))
>                 goto out_unlock;
> --
> 2.47.0
>

Re: [PATCH V3 04/17] ublk: let ublk_fill_io_cmd() cover more things

[Caleb Sander Mateos]

On Sun, Jul 13, 2025 at 10:34 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Let ublk_fill_io_cmd() clear UBLK_IO_FLAG_OWNED_BY_SRV too.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>

> ---
>  drivers/block/ublk_drv.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 73c6c8d3b117..f251517baea3 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2014,6 +2014,8 @@ static inline void ublk_fill_io_cmd(struct ublk_io *io,
>  {
>         io->cmd = cmd;
>         io->flags |= UBLK_IO_FLAG_ACTIVE;
> +       /* now this cmd slot is owned by ublk driver */
> +       io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
>         io->addr = buf_addr;
>  }
>
> @@ -2229,9 +2231,6 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
>         }
>
>         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;
>
>         if (req_op(req) == REQ_OP_ZONE_APPEND)
> @@ -2353,7 +2352,6 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
>                  */
>                 req = io->req;
>                 ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
> -               io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
>                 if (likely(ublk_get_data(ubq, io, req))) {
>                         __ublk_prep_compl_io_cmd(io, req);
>                         return UBLK_IO_RES_OK;
> --
> 2.47.0
>

Re: [PATCH V3 09/17] ublk: remove ublk_commit_and_fetch()

[Caleb Sander Mateos]

On Sun, Jul 13, 2025 at 10:35 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Remove ublk_commit_and_fetch() and open code request completion.
>
> 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.
>
> 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 1b22fd5f5610..252cae345b3a 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -714,13 +714,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);

I mentioned before that the struct request *req parameter can now be
removed from ublk_sub_req_ref() and ublk_need_complete_req().

Other than that,
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>

>  }
>
>  static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
> @@ -2243,21 +2242,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,
> @@ -2290,6 +2281,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,
> @@ -2367,8 +2359,16 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
>                 io->res = ub_cmd->result;
>                 req = ublk_fill_io_cmd(io, cmd);
>                 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
>

Re: [PATCH V3 01/17] ublk: validate ublk server pid

[Ming Lei]

On Tue, Jul 15, 2025 at 10:50:39AM -0400, Caleb Sander Mateos wrote:
> On Sun, Jul 13, 2025 at 10:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > ublk server pid(the `tgid` of the process opening the ublk device) is stored
> > in `ublk_device->ublksrv_tgid`. This `tgid` is then checked against the
> > `ublksrv_pid` in `ublk_ctrl_start_dev` and `ublk_ctrl_end_recovery`.
> >
> > This ensures that correct ublk server pid is stored in device info.
> >
> > Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/block/ublk_drv.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index a1a700c7e67a..2b894de29823 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -237,6 +237,7 @@ struct ublk_device {
> >         unsigned int            nr_privileged_daemon;
> >         struct mutex cancel_mutex;
> >         bool canceling;
> > +       pid_t   ublksrv_tgid;
> >  };
> >
> >  /* header of ublk_params */
> > @@ -1528,6 +1529,7 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
> >         if (test_and_set_bit(UB_STATE_OPEN, &ub->state))
> >                 return -EBUSY;
> >         filp->private_data = ub;
> > +       ub->ublksrv_tgid = current->tgid;
> >         return 0;
> >  }
> >
> > @@ -1542,6 +1544,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
> >         ub->mm = NULL;
> >         ub->nr_queues_ready = 0;
> >         ub->nr_privileged_daemon = 0;
> > +       ub->ublksrv_tgid = -1;
> 
> Should this be reset to 0? The next patch checks whether ublksrv_tgid
> is 0 in ublk_timeout().

No, swapper pid is 0.

The check in next patch just tries to double check if ublk char device
is opened.

> Also, the accesses to it should probably be
> using {READ,WRITE}_ONCE() since ublk server open/close can happen
> concurrently with ublk I/O timeout handling.

ublk_abort_queue() is called in ublk_ch_release(), and any inflight request
is either requeued or failed, so ublk I/O timeout handling won't happen
concurrently with ublk char open()/close().


Thanks, 
Ming

Re: [PATCH V3 01/17] ublk: validate ublk server pid

[Caleb Sander Mateos]

On Tue, Jul 15, 2025 at 11:42 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Tue, Jul 15, 2025 at 10:50:39AM -0400, Caleb Sander Mateos wrote:
> > On Sun, Jul 13, 2025 at 10:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> > >
> > > ublk server pid(the `tgid` of the process opening the ublk device) is stored
> > > in `ublk_device->ublksrv_tgid`. This `tgid` is then checked against the
> > > `ublksrv_pid` in `ublk_ctrl_start_dev` and `ublk_ctrl_end_recovery`.
> > >
> > > This ensures that correct ublk server pid is stored in device info.
> > >
> > > Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > ---
> > >  drivers/block/ublk_drv.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index a1a700c7e67a..2b894de29823 100644
> > > --- a/drivers/block/ublk_drv.c
> > > +++ b/drivers/block/ublk_drv.c
> > > @@ -237,6 +237,7 @@ struct ublk_device {
> > >         unsigned int            nr_privileged_daemon;
> > >         struct mutex cancel_mutex;
> > >         bool canceling;
> > > +       pid_t   ublksrv_tgid;
> > >  };
> > >
> > >  /* header of ublk_params */
> > > @@ -1528,6 +1529,7 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
> > >         if (test_and_set_bit(UB_STATE_OPEN, &ub->state))
> > >                 return -EBUSY;
> > >         filp->private_data = ub;
> > > +       ub->ublksrv_tgid = current->tgid;
> > >         return 0;
> > >  }
> > >
> > > @@ -1542,6 +1544,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
> > >         ub->mm = NULL;
> > >         ub->nr_queues_ready = 0;
> > >         ub->nr_privileged_daemon = 0;
> > > +       ub->ublksrv_tgid = -1;
> >
> > Should this be reset to 0? The next patch checks whether ublksrv_tgid
> > is 0 in ublk_timeout().
>
> No, swapper pid is 0.
>
> The check in next patch just tries to double check if ublk char device
> is opened.
>
> > Also, the accesses to it should probably be
> > using {READ,WRITE}_ONCE() since ublk server open/close can happen
> > concurrently with ublk I/O timeout handling.
>
> ublk_abort_queue() is called in ublk_ch_release(), and any inflight request
> is either requeued or failed, so ublk I/O timeout handling won't happen
> concurrently with ublk char open()/close().

Thanks for explaining. If the ublk server closing the char device
ensures there are no in-flight requests, does that make the
ublksrv_tgid check in ublk_timeout() unnecessary?

Best,
Caleb

Re: [PATCH V3 01/17] ublk: validate ublk server pid

[Ming Lei]

On Tue, Jul 15, 2025 at 11:48:12AM -0400, Caleb Sander Mateos wrote:
> On Tue, Jul 15, 2025 at 11:42 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Tue, Jul 15, 2025 at 10:50:39AM -0400, Caleb Sander Mateos wrote:
> > > On Sun, Jul 13, 2025 at 10:34 AM Ming Lei <ming.lei@redhat.com> wrote:
> > > >
> > > > ublk server pid(the `tgid` of the process opening the ublk device) is stored
> > > > in `ublk_device->ublksrv_tgid`. This `tgid` is then checked against the
> > > > `ublksrv_pid` in `ublk_ctrl_start_dev` and `ublk_ctrl_end_recovery`.
> > > >
> > > > This ensures that correct ublk server pid is stored in device info.
> > > >
> > > > Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> > > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > > ---
> > > >  drivers/block/ublk_drv.c | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > > index a1a700c7e67a..2b894de29823 100644
> > > > --- a/drivers/block/ublk_drv.c
> > > > +++ b/drivers/block/ublk_drv.c
> > > > @@ -237,6 +237,7 @@ struct ublk_device {
> > > >         unsigned int            nr_privileged_daemon;
> > > >         struct mutex cancel_mutex;
> > > >         bool canceling;
> > > > +       pid_t   ublksrv_tgid;
> > > >  };
> > > >
> > > >  /* header of ublk_params */
> > > > @@ -1528,6 +1529,7 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
> > > >         if (test_and_set_bit(UB_STATE_OPEN, &ub->state))
> > > >                 return -EBUSY;
> > > >         filp->private_data = ub;
> > > > +       ub->ublksrv_tgid = current->tgid;
> > > >         return 0;
> > > >  }
> > > >
> > > > @@ -1542,6 +1544,7 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
> > > >         ub->mm = NULL;
> > > >         ub->nr_queues_ready = 0;
> > > >         ub->nr_privileged_daemon = 0;
> > > > +       ub->ublksrv_tgid = -1;
> > >
> > > Should this be reset to 0? The next patch checks whether ublksrv_tgid
> > > is 0 in ublk_timeout().
> >
> > No, swapper pid is 0.
> >
> > The check in next patch just tries to double check if ublk char device
> > is opened.
> >
> > > Also, the accesses to it should probably be
> > > using {READ,WRITE}_ONCE() since ublk server open/close can happen
> > > concurrently with ublk I/O timeout handling.
> >
> > ublk_abort_queue() is called in ublk_ch_release(), and any inflight request
> > is either requeued or failed, so ublk I/O timeout handling won't happen
> > concurrently with ublk char open()/close().
> 
> Thanks for explaining. If the ublk server closing the char device
> ensures there are no in-flight requests, does that make the
> ublksrv_tgid check in ublk_timeout() unnecessary?

Yes, the check should have been WARN_ON_ONCE().

Thanks,
Ming