Re: [PATCH blktests v3 1/2] src: add mini ublk source code

[Ming Lei <ming.lei@redhat.com>]

Hi Chaitanya,

Thanks for the review!

On Tue, Feb 21, 2023 at 07:58:48PM +0000, Chaitanya Kulkarni wrote:
> On 2/19/2023 7:46 PM, Ming Lei wrote:
> > Prepare for adding ublk related test:
> > 
> > 1) ublk delete is sync removal, this way is convenient to
> >     blkg/queue/disk instance leak issue
> > 
> > 2) mini ublk has two builtin target(null, loop), and loop IO is
> > handled by io_uring, so we can use ublk to cover part of io_uring
> > workloads
> > 
> > 3) not like loop/nbd, ublk won't pre-allocate/add disk, and always
> > add/delete disk dynamically, this way may cover disk plug & unplug
> > tests
> > 
> > 4) ublk specific test given people starts to use it, so better to
> > let blktest cover ublk related tests
> > 
> > Add mini ublk source for test purpose only, which is easy to use:
> > 
> > ./miniublk add -t {null|loop} [-q nr_queues] [-d depth] [-n dev_id]
> > 	 default: nr_queues=2(max 4), depth=128(max 128), dev_id=-1(auto allocation)
> > 	 -t loop -f backing_file
> > 	 -t null
> > ./miniublk del [-n dev_id] -a
> > 	 -a delete all devices, -n delete specified device
> > ./miniublk list [-n dev_id] -a
> > 	 -a list all devices, -n list specified device, default -a
> > 
> > miniublk depends on liburing 2.2, adds HAVE_LIBURING for checking if
> > liburing 2.2 exists; also add HAVE_UBLK_HEADER for checking ublk kernel
> > UAPI header exists. If either of two dependencies can't be met, simply
> > ignore miniublk target.
> > 
> > Also v6.0 is the 1st linux kernel release with ublk.
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >   src/.gitignore |    1 +
> >   src/Makefile   |   18 +
> >   src/miniublk.c | 1376 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 1395 insertions(+)
> >   create mode 100644 src/miniublk.c
> > 
> > diff --git a/src/.gitignore b/src/.gitignore
> > index 355bed3..df7aff5 100644
> > --- a/src/.gitignore
> > +++ b/src/.gitignore
> > @@ -8,3 +8,4 @@
> >   /sg/dxfer-from-dev
> >   /sg/syzkaller1
> >   /zbdioctl
> > +/miniublk
> > diff --git a/src/Makefile b/src/Makefile
> > index 3b587f6..81c6541 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -2,6 +2,10 @@ HAVE_C_HEADER = $(shell if echo "\#include <$(1)>" |		\
> >   		$(CC) -E - > /dev/null 2>&1; then echo "$(2)";	\
> >   		else echo "$(3)"; fi)
> >   
> > +HAVE_C_MACRO = $(shell if echo "#include <$(1)>" |	\
> > +		$(CC) -E - 2>&1 /dev/null | grep $(2) > /dev/null 2>&1; \
> > +		then echo 1;else echo 0; fi)
> > +
> >   C_TARGETS := \
> >   	loblksize \
> >   	loop_change_fd \
> > @@ -13,16 +17,27 @@ C_TARGETS := \
> >   	sg/syzkaller1 \
> >   	zbdioctl
> >   
> > +C_MINIUBLK := miniublk
> > +
> > +HAVE_LIBURING := $(call HAVE_C_MACRO,liburing.h,IORING_OP_URING_CMD)
> > +HAVE_UBLK_HEADER := $(call HAVE_C_HEADER,linux/ublk_cmd.h,1)
> > +
> >   CXX_TARGETS := \
> >   	discontiguous-io
> >   
> > +ifeq ($(HAVE_LIBURING)$(HAVE_UBLK_HEADER), 11)
> > +TARGETS := $(C_TARGETS) $(CXX_TARGETS) $(C_MINIUBLK)
> > +else
> > +$(info Skip $(C_MINIUBLK) build due to missing kernel header(v6.0+) or liburing(2.2+))
> >   TARGETS := $(C_TARGETS) $(CXX_TARGETS)
> > +endif
> >   
> >   CONFIG_DEFS := $(call HAVE_C_HEADER,linux/blkzoned.h,-DHAVE_LINUX_BLKZONED_H)
> >   
> >   override CFLAGS   := -O2 -Wall -Wshadow $(CFLAGS) $(CONFIG_DEFS)
> >   override CXXFLAGS := -O2 -std=c++11 -Wall -Wextra -Wshadow -Wno-sign-compare \
> >   		     -Werror $(CXXFLAGS) $(CONFIG_DEFS)
> > +MINIUBLK_FLAGS :=  -D_GNU_SOURCE -lpthread -luring
> >   
> >   all: $(TARGETS)
> >   
> > @@ -39,4 +54,7 @@ $(C_TARGETS): %: %.c
> >   $(CXX_TARGETS): %: %.cpp
> >   	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $^
> >   
> > +$(C_MINIUBLK): %: miniublk.c
> > +	$(CC) $(CFLAGS) $(MINIUBLK_FLAGS) -o $@ miniublk.c
> > +
> >   .PHONY: all clean install
> > diff --git a/src/miniublk.c b/src/miniublk.c
> > new file mode 100644
> > index 0000000..e84ba41
> > --- /dev/null
> > +++ b/src/miniublk.c
> > @@ -0,0 +1,1376 @@
> > +// SPDX-License-Identifier: GPL-3.0+
> > +// Copyright (C) 2023 Ming Lei
> > +
> > +/*
> > + * io_uring based mini ublk implementation with null/loop target,
> > + * for test purpose only.
> > + *
> > + * So please keep it clean & simple & reliable.
> > + */
> > +
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <assert.h>
> > +#include <stdio.h>
> > +#include <stdarg.h>
> > +#include <string.h>
> > +#include <pthread.h>
> > +#include <getopt.h>
> > +#include <limits.h>
> > +#include <sys/syscall.h>
> > +#include <sys/mman.h>
> > +#include <sys/ioctl.h>
> > +#include <liburing.h>
> > +#include <linux/ublk_cmd.h>
> > +
> > +#define CTRL_DEV		"/dev/ublk-control"
> > +#define UBLKC_DEV		"/dev/ublkc"
> > +#define UBLK_CTRL_RING_DEPTH            32
> > +
> > +/* queue idle timeout */
> > +#define UBLKSRV_IO_IDLE_SECS		20
> > +
> > +#define UBLK_IO_MAX_BYTES               65536
> > +#define UBLK_MAX_QUEUES                 4
> > +#define UBLK_QUEUE_DEPTH                128
> > +
> > +#define UBLK_DBG_DEV            (1U << 0)
> > +#define UBLK_DBG_QUEUE          (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_ctrl_cmd_data {
> > +	unsigned short cmd_op;
> 
> perhaps use enum type to avoid any type mismatach errors in future..

Sounds good.

> 
> > +#define CTRL_CMD_HAS_DATA	1
> > +#define CTRL_CMD_HAS_BUF	2
> > +	unsigned short flags;
> 
> I'd keep falgs u64 or u32 just incase..

Fine.

> 
> > +
> > +	__u64 data[2];
> > +	__u64 addr;
> > +	__u32 len;
> > +};
> > +
> > +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)
> > +	unsigned int flags;
> > +
> > +	unsigned int result;
> > +};
> > +
> > +struct ublk_tgt_ops {
> > +	const char *name;
> > +	int (*init_tgt)(struct ublk_dev *);
> > +	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 *);
> > +};
> > +
> > +struct ublk_tgt {
> > +	unsigned long dev_size;
> > +	const struct ublk_tgt_ops *ops;
> > +	int argc;
> > +	char **argv;
> > +	struct ublk_params params;
> > +};
> > +
> > +struct ublk_queue {
> > +	int q_id;
> > +	int q_depth;
> 
> is there a reason to not use unsigned int for qid and qd above ?

Because int is enough for holding nr_queues and depth.

> 
> > +	unsigned int cmd_inflight;
> > +	unsigned int io_inflight;
> > +	struct ublk_dev *dev;
> > +	const struct ublk_tgt_ops *tgt_ops;
> > +	char *io_cmd_buf;
> > +	struct io_uring ring;
> > +	struct ublk_io ios[UBLK_QUEUE_DEPTH];
> > +#define UBLKSRV_QUEUE_STOPPING	(1U << 0)
> > +#define UBLKSRV_QUEUE_IDLE	(1U << 1)
> > +	unsigned state;
> > +	int tid;
> 
> instead of int pid_t  ?

OK.

> 
> > +	pthread_t thread;
> > +};
> > +
> > +struct ublk_dev {
> > +	struct ublk_tgt tgt;
> > +	struct ublksrv_ctrl_dev_info  dev_info;
> > +	struct ublk_queue q[UBLK_MAX_QUEUES];
> > +
> > +	int fds[2];	/* fds[0] points to /dev/ublkcN */
> > +	int nr_fds;
> > +	int ctrl_fd;
> > +	struct io_uring ring;
> > +};
> > +
> > +#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))
> > +
> > +#define ublk_assert(x)  do { \
> > +	if (!(x)) {     \
> > +		ublk_err("%s %d: assert!\n", __func__, __LINE__); \
> > +		assert(x);      \
> > +	}       \
> > +} while (0)
> > +
> > +static const struct ublk_tgt_ops *ublk_find_tgt(const char *name);
> > +
> > +static unsigned int ublk_dbg_mask = UBLK_LOG;
> > +
> > +static inline unsigned ilog2(unsigned x)
> > +{
> > +    return sizeof(unsigned) * 8 - 1 - __builtin_clz(x);
> > +}
> > +
> > +static inline int is_target_io(__u64 user_data)
> > +{
> > +	return (user_data & (1ULL << 63)) != 0;
> > +}
> > +
> > +static inline __u64 build_user_data(unsigned tag, unsigned op,
> > +		unsigned tgt_data, unsigned is_target_io)
> > +{
> > +	assert(!(tag >> 16) && !(op >> 8) && !(tgt_data >> 16));
> > +
> > +	return tag | (op << 16) | (tgt_data << 24) | (__u64)is_target_io << 63;
> > +}
> > +
> > +static inline unsigned int user_data_to_tag(__u64 user_data)
> > +{
> > +	return user_data & 0xffff;
> > +}
> > +
> > +static inline unsigned int user_data_to_op(__u64 user_data)
> > +{
> > +	return (user_data >> 16) & 0xff;
> > +}
> > +
> > +static void ublk_err(const char *fmt, ...)
> > +{
> > +	va_list ap;
> > +
> > +	va_start(ap, fmt);
> > +	vfprintf(stderr, fmt, ap);
> > +}
> > +
> > +static void ublk_log(const char *fmt, ...)
> > +{
> > +	if (ublk_dbg_mask & UBLK_LOG) {
> > +		va_list ap;
> > +
> > +		va_start(ap, fmt);
> > +		vfprintf(stdout, fmt, ap);
> > +	}
> > +}
> > +
> > +static 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 void *ublk_get_sqe_cmd(const struct io_uring_sqe *sqe)
> > +{
> > +	return (void *)&sqe->addr3;
> > +}
> > +
> > +static inline void ublk_mark_io_done(struct ublk_io *io, int res)
> > +{
> > +	io->flags |= (UBLKSRV_NEED_COMMIT_RQ_COMP | UBLKSRV_IO_FREE);
> > +	io->result = res;
> > +}
> > +
> > +static inline const struct ublksrv_io_desc *ublk_get_iod(
> > +                const struct ublk_queue *q, int tag)
> > +{
> > +        return (struct ublksrv_io_desc *)
> > +                &(q->io_cmd_buf[tag * sizeof(struct ublksrv_io_desc)]);
> > +}
> > +
> > +static inline void ublk_set_sqe_cmd_op(struct io_uring_sqe *sqe,
> > +		__u32 cmd_op)
> > +{
> > +        __u32 *addr = (__u32 *)&sqe->off;
> > +
> > +        addr[0] = cmd_op;
> > +        addr[1] = 0;
> > +}
> > +
> > +static inline int ublk_setup_ring(struct io_uring *r, int depth,
> > +		int cq_depth, unsigned flags)
> > +{
> > +	struct io_uring_params p;
> > +
> > +	memset(&p, 0, sizeof(p));
> > +	p.flags = flags | IORING_SETUP_CQSIZE;
> > +	p.cq_entries = cq_depth;
> > +
> > +	return io_uring_queue_init_params(depth, r, &p);
> > +}
> > +
> > +static inline void ublk_ctrl_init_cmd(struct ublk_dev *dev,
> > +		struct io_uring_sqe *sqe,
> > +		struct ublk_ctrl_cmd_data *data)
> > +{
> > +	struct ublksrv_ctrl_dev_info *info = &dev->dev_info;
> > +	struct ublksrv_ctrl_cmd *cmd = (struct ublksrv_ctrl_cmd *)ublk_get_sqe_cmd(sqe);
> > +
> > +	sqe->fd = dev->ctrl_fd;
> > +	sqe->opcode = IORING_OP_URING_CMD;
> > +	sqe->ioprio = 0;
> > +
> > +	if (data->flags & CTRL_CMD_HAS_BUF) {
> > +		cmd->addr = data->addr;
> > +		cmd->len = data->len;
> > +	}
> > +
> > +	if (data->flags & CTRL_CMD_HAS_DATA)
> > +		cmd->data[0] = data->data[0];
> > +
> > +	cmd->dev_id = info->dev_id;
> > +	cmd->queue_id = -1;
> > +
> > +	ublk_set_sqe_cmd_op(sqe, data->cmd_op);
> > +
> > +	io_uring_sqe_set_data(sqe, cmd);
> > +}
> > +
> > +static int __ublk_ctrl_cmd(struct ublk_dev *dev,
> > +		struct ublk_ctrl_cmd_data *data)
> > +{
> > +	struct io_uring_sqe *sqe;
> > +	struct io_uring_cqe *cqe;
> > +	int ret = -EINVAL;
> > +
> > +	sqe = io_uring_get_sqe(&dev->ring);
> > +	if (!sqe) {
> > +		ublk_err("%s: can't get sqe ret %d\n", __func__, ret);
> > +		return ret;
> > +	}
> > +
> > +	ublk_ctrl_init_cmd(dev, sqe, data);
> > +
> > +	ret = io_uring_submit(&dev->ring);
> > +	if (ret < 0) {
> > +		ublk_err("uring submit ret %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = io_uring_wait_cqe(&dev->ring, &cqe);
> > +	if (ret < 0) {
> > +		ublk_err("wait cqe: %s\n", strerror(-ret));
> > +		return ret;
> > +	}
> > +	io_uring_cqe_seen(&dev->ring, cqe);
> > +
> > +	return cqe->res;
> > +}
> > +
> > +int ublk_ctrl_stop_dev(struct ublk_dev *dev)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_STOP_DEV,
> > +	};
> > +	int ret;
> > +
> > +	ret = __ublk_ctrl_cmd(dev, &data);
> > +	return ret;
> 
> you can just return directly from here and remove local var ret..

OK.

> 
> > +}
> > +
> > +int ublk_ctrl_start_dev(struct ublk_dev *dev,
> > +		int daemon_pid)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_START_DEV,
> > +		.flags	= CTRL_CMD_HAS_DATA,
> > +	};
> > +	int ret;
> > +
> > +	dev->dev_info.ublksrv_pid = data.data[0] = daemon_pid;
> > +
> > +	ret = __ublk_ctrl_cmd(dev, &data);
> > +
> > +	return ret;
> 
> same here as previous comment ...
> 
> > +}
> > +
> > +int ublk_ctrl_add_dev(struct ublk_dev *dev)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_ADD_DEV,
> > +		.flags	= CTRL_CMD_HAS_BUF,
> > +		.addr = (__u64)&dev->dev_info,
> > +		.len = sizeof(struct ublksrv_ctrl_dev_info),
> > +	};
> > +
> > +	return __ublk_ctrl_cmd(dev, &data);
> > +}
> > +
> > +int ublk_ctrl_del_dev(struct ublk_dev *dev)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op = UBLK_CMD_DEL_DEV,
> > +		.flags = 0,
> > +	};
> > +
> > +	return __ublk_ctrl_cmd(dev, &data);
> > +}
> > +
> > +int ublk_ctrl_get_info(struct ublk_dev *dev)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_GET_DEV_INFO,
> > +		.flags	= CTRL_CMD_HAS_BUF,
> > +		.addr = (__u64)&dev->dev_info,
> > +		.len = sizeof(struct ublksrv_ctrl_dev_info),
> > +	};
> > +
> > +	return __ublk_ctrl_cmd(dev, &data);
> > +}
> > +
> > +int ublk_ctrl_set_params(struct ublk_dev *dev,
> > +		struct ublk_params *params)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_SET_PARAMS,
> > +		.flags	= CTRL_CMD_HAS_BUF,
> > +		.addr = (__u64)params,
> > +		.len = sizeof(*params),
> > +	};
> > +	params->len = sizeof(*params);
> > +	return __ublk_ctrl_cmd(dev, &data);
> > +}
> > +
> > +static int ublk_ctrl_get_params(struct ublk_dev *dev,
> > +		struct ublk_params *params)
> > +{
> > +	struct ublk_ctrl_cmd_data data = {
> > +		.cmd_op	= UBLK_CMD_GET_PARAMS,
> > +		.flags	= CTRL_CMD_HAS_BUF,
> > +		.addr = (__u64)params,
> > +		.len = sizeof(*params),
> > +	};
> > +	int ret;
> > +
> > +	params->len = sizeof(*params);
> > +
> > +	ret = __ublk_ctrl_cmd(dev, &data);
> > +	return ret;
> 
> same here as previous comment ...
> 
> > +}
> > +
> > +static const char *ublk_dev_state_desc(struct ublk_dev *dev)
> > +{
> > +	switch (dev->dev_info.state) {
> > +	case UBLK_S_DEV_DEAD:
> > +		return "DEAD";
> > +	case UBLK_S_DEV_LIVE:
> > +		return "LIVE";
> > +	default:
> > +		return "UNKNOWN";
> > +	};
> > +}
> > +
> > +static void ublk_ctrl_dump(struct ublk_dev *dev, bool show_queue)
> > +{
> > +	struct ublksrv_ctrl_dev_info *info = &dev->dev_info;
> > +	int ret;
> > +	struct ublk_params p;
> > +
> > +	ret = ublk_ctrl_get_params(dev, &p);
> > +	if (ret < 0) {
> > +		ublk_err("failed to get params %m\n");
> > +		return;
> > +	}
> > +
> > +	ublk_log("dev id %d: nr_hw_queues %d queue_depth %d block size %d dev_capacity %lld\n",
> > +			info->dev_id,
> > +                        info->nr_hw_queues, info->queue_depth,
> > +                        1 << p.basic.logical_bs_shift, p.basic.dev_sectors);
> > +	ublk_log("\tmax rq size %d daemon pid %d flags 0x%llx state %s\n",
> > +                        info->max_io_buf_bytes,
> > +			info->ublksrv_pid, info->flags,
> > +			ublk_dev_state_desc(dev));
> > +	if (show_queue) {
> > +		int i;
> > +
> > +		for (i = 0; i < dev->dev_info.nr_hw_queues; i++)
> > +			ublk_log("\tqueue 0 tid: %d\n", dev->q[i].tid);
> > +	}
> > +	fflush(stdout);
> > +}
> > +
> > +static void ublk_ctrl_deinit(struct ublk_dev *dev)
> > +{
> > +	close(dev->ctrl_fd);
> > +	free(dev);
> > +}
> > +
> > +static struct ublk_dev *ublk_ctrl_init()
> 
> perhaps specify void -> ublk_ctrl_init(void)
> 
> > +{
> > +	struct ublk_dev *dev = (struct ublk_dev *)calloc(1, sizeof(*dev));
> > +	struct ublksrv_ctrl_dev_info *info = &dev->dev_info;
> > +	int ret;
> > +
> > +	dev->ctrl_fd = open(CTRL_DEV, O_RDWR);
> > +	if (dev->ctrl_fd < 0) {
> > +		ublk_err("control dev %s can't be opened: %m\n", CTRL_DEV);
> 
> perhaps also print the errno that is set by the open system call unless
> ublk_err is doing that implicitly which I'm not sure.
> 
> also I didn't understand two format specifiers for above call and only
> one argument its a bit confusing to read the code...

%m is supposed to print message of errno, but it is fine to add the
errno too.

> 
> > +		exit(dev->ctrl_fd);
> > +	}
> > +	info->max_io_buf_bytes = UBLK_IO_MAX_BYTES;
> > +
> > +	ret = ublk_setup_ring(&dev->ring, UBLK_CTRL_RING_DEPTH,
> > +			UBLK_CTRL_RING_DEPTH, IORING_SETUP_SQE128);
> > +	if (ret < 0) {
> > +		ublk_err("queue_init: %s\n", strerror(-ret));
> > +		free(dev);
> > +		return NULL;
> > +	}
> > +	dev->nr_fds = 1;
> > +
> > +	return dev;
> > +}
> > +
> > +static int ublk_queue_cmd_buf_sz(struct ublk_queue *q)
> > +{
> > +	int size =  q->q_depth * sizeof(struct ublksrv_io_desc);
> > +	unsigned int page_sz = getpagesize();
> > +
> > +	return round_up(size, page_sz);
> > +}
> > +
> > +static void ublk_queue_deinit(struct ublk_queue *q)
> > +{
> > +	int i;
> > +	int nr_ios = q->q_depth;
> > +
> > +	io_uring_unregister_ring_fd(&q->ring);
> > +
> > +	if (q->ring.ring_fd > 0) {
> > +		io_uring_unregister_files(&q->ring);
> > +		close(q->ring.ring_fd);
> > +		q->ring.ring_fd = -1;
> > +	}
> > +
> > +	if (q->io_cmd_buf)
> > +		munmap(q->io_cmd_buf, ublk_queue_cmd_buf_sz(q));
> > +
> > +	for (i = 0; i < nr_ios; i++)
> > +		free(q->ios[i].buf_addr);
> > +}
> > +
> > +static int ublk_queue_init(struct ublk_queue *q)
> > +{
> > +	struct ublk_dev *dev = q->dev;
> > +	int depth = dev->dev_info.queue_depth;
> > +	int i, ret = -1;
> > +	int cmd_buf_size, io_buf_size;
> > +	unsigned long off;
> > +	int ring_depth = depth, cq_depth = depth;
> > +
> > +	q->tgt_ops = dev->tgt.ops;
> > +	q->state = 0;
> > +	q->q_depth = depth;
> > +	q->cmd_inflight = 0;
> > +	q->tid = gettid();
> > +
> > +	cmd_buf_size = ublk_queue_cmd_buf_sz(q);
> > +	off = UBLKSRV_CMD_BUF_OFFSET +
> > +		q->q_id * (UBLK_MAX_QUEUE_DEPTH * sizeof(struct ublksrv_io_desc));
> > +	q->io_cmd_buf = (char *)mmap(0, cmd_buf_size, PROT_READ,
> > +			MAP_SHARED | MAP_POPULATE, dev->fds[0], off);
> > +	if (q->io_cmd_buf == MAP_FAILED) {
> > +		ublk_err("ublk dev %d queue %d map io_cmd_buf failed\n",
> > +				q->dev->dev_info.dev_id, q->q_id);
> 
> also add strerror(errno) to message ?

%m could be easier.

> 
> > +		goto fail;
> > +	}
> > +
> > +	io_buf_size = dev->dev_info.max_io_buf_bytes;
> > +	for (i = 0; i < q->q_depth; i++) {
> > +		q->ios[i].buf_addr = NULL;
> > +
> > +		if (posix_memalign((void **)&q->ios[i].buf_addr,
> > +					getpagesize(), io_buf_size)) {
> > +			ublk_err("ublk dev %d queue %d io %d posix_memalign failed\n",
> > +					dev->dev_info.dev_id, q->q_id, i);
> > +			goto fail;
> > +		}
> 
> same as previous comment ..

OK.

> 
> > +		q->ios[i].flags = UBLKSRV_NEED_FETCH_RQ | UBLKSRV_IO_FREE;
> > +	}
> > +
> > +	ret = ublk_setup_ring(&q->ring, ring_depth, cq_depth,
> > +			IORING_SETUP_SQE128 | IORING_SETUP_COOP_TASKRUN);
> > +	if (ret < 0) {
> > +		ublk_err("ublk dev %d queue %d setup io_uring failed %d\n",
> > +				q->dev->dev_info.dev_id, q->q_id, ret);
> > +		goto fail;
> > +	}
> > +
> > +	io_uring_register_ring_fd(&q->ring);
> > +
> > +	ret = io_uring_register_files(&q->ring, dev->fds, dev->nr_fds);
> > +	if (ret) {
> > +		ublk_err("ublk dev %d queue %d register files failed %d\n",
> > +				q->dev->dev_info.dev_id, q->q_id, ret);
> > +		goto fail;
> > +	}
> > +
> > +	return 0;
> > + fail:
> > +	ublk_queue_deinit(q);
> > +	ublk_err("ublk dev %d queue %d failed\n",
> > +			dev->dev_info.dev_id, q->q_id);
> > +	return -ENOMEM;
> > +}
> > +
> > +static int ublk_dev_prep(struct ublk_dev *dev)
> > +{
> > +	int dev_id = dev->dev_info.dev_id;
> > +	char buf[64];
> > +	int ret;
> > +
> > +	snprintf(buf, 64, "%s%d", UBLKC_DEV, dev_id);
> > +	dev->fds[0] = open(buf, O_RDWR);
> > +	if (dev->fds[0] < 0) {
> > +		ret = -EBADF;
> > +		ublk_err("can't open %s, ret %d\n", buf, dev->fds[0]);
> > +		goto fail;
> > +	}
> > +
> > +	if (dev->tgt.ops->init_tgt)
> > +		ret = dev->tgt.ops->init_tgt(dev);
> > +	else
> > +		ret = 0;
> > +
> 
> if ret is initialize to 0 at the time of declaration in above function
> we can safely get rid of else above ?

OK.

> 
> > +	return ret;
> > +fail:
> > +	close(dev->fds[0]);
> > +	return ret;
> > +}
> > +
> > +static void ublk_dev_unprep(struct ublk_dev *dev)
> > +{
> > +	if (dev->tgt.ops->deinit_tgt)
> > +		dev->tgt.ops->deinit_tgt(dev);
> > +	close(dev->fds[0]);
> > +}
> > +
> > +static int ublk_queue_io_cmd(struct ublk_queue *q,
> > +		struct ublk_io *io, unsigned tag)
> > +{
> > +	struct ublksrv_io_cmd *cmd;
> > +	struct io_uring_sqe *sqe;
> > +	unsigned int cmd_op = 0;
> > +	__u64 user_data;
> > +
> > +	/* only freed io can be issued */
> > +	if (!(io->flags & UBLKSRV_IO_FREE))
> > +		return 0;
> > +
> > +	/* we issue because we need either fetching or committing */
> > +	if (!(io->flags &
> > +		(UBLKSRV_NEED_FETCH_RQ | UBLKSRV_NEED_COMMIT_RQ_COMP)))
> > +		return 0;
> > +
> > +	if (io->flags & UBLKSRV_NEED_COMMIT_RQ_COMP)
> > +		cmd_op = UBLK_IO_COMMIT_AND_FETCH_REQ;
> > +	else if (io->flags & UBLKSRV_NEED_FETCH_RQ)
> > +		cmd_op = UBLK_IO_FETCH_REQ;
> > +
> > +	sqe = io_uring_get_sqe(&q->ring);
> > +	if (!sqe) {
> > +		ublk_err("%s: run out of sqe %d, tag %d\n",
> > +				__func__, q->q_id, tag);
> > +		return -1;
> > +	}
> > +
> > +	cmd = (struct ublksrv_io_cmd *)ublk_get_sqe_cmd(sqe);
> > +
> > +	if (cmd_op == UBLK_IO_COMMIT_AND_FETCH_REQ)
> > +		cmd->result = io->result;
> > +
> > +	/* These fields should be written once, never change */
> > +	ublk_set_sqe_cmd_op(sqe, cmd_op);
> > +	sqe->fd		= 0;	/* dev->fds[0] */
> > +	sqe->opcode	= IORING_OP_URING_CMD;
> > +	sqe->flags	= IOSQE_FIXED_FILE;
> > +	sqe->rw_flags	= 0;
> > +	cmd->tag	= tag;
> > +	cmd->addr	= (__u64)io->buf_addr;
> > +	cmd->q_id	= q->q_id;
> > +
> > +	user_data = build_user_data(tag, cmd_op, 0, 0);
> > +	io_uring_sqe_set_data64(sqe, user_data);
> > +
> > +	io->flags = 0;
> > +
> > +	q->cmd_inflight += 1;
> > +
> > +	ublk_dbg(UBLK_DBG_IO_CMD, "%s: (qid %d tag %u cmd_op %u) iof %x stopping %d\n",
> > +			__func__, q->q_id, tag, cmd_op,
> > +			io->flags, !!(q->state & UBLKSRV_QUEUE_STOPPING));
> > +	return 1;
> > +}
> > +
> > +static int ublk_complete_io(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(q, io, tag);
> > +}
> > +
> > +static void ublk_submit_fetch_commands(struct ublk_queue *q)
> > +{
> > +	int i = 0;
> > +
> > +	for (i = 0; i < q->q_depth; i++)
> > +		ublk_queue_io_cmd(q, &q->ios[i], i);
> > +}
> > +
> > +static int ublk_queue_is_idle(struct ublk_queue *q)
> > +{
> > +	return !io_uring_sq_ready(&q->ring) && !q->io_inflight;
> > +}
> > +
> > +static int ublk_queue_is_done(struct ublk_queue *q)
> > +{
> > +	return (q->state & UBLKSRV_QUEUE_STOPPING) && ublk_queue_is_idle(q);
> > +}
> > +
> > +static void ublk_queue_discard_io_pages(struct ublk_queue *q)
> > +{
> > +	const struct ublk_dev *dev = q->dev;
> > +	unsigned int io_buf_size = dev->dev_info.max_io_buf_bytes;
> > +	int i = 0;
> > +
> > +	for (i = 0; i < q->q_depth; i++)
> > +		madvise(q->ios[i].buf_addr, io_buf_size, MADV_DONTNEED);
> > +}
> > +
> > +static void ublk_queue_idle_enter(struct ublk_queue *q)
> > +{
> > +	if (q->state & UBLKSRV_QUEUE_IDLE)
> > +		return;
> > +
> > +	ublk_dbg(UBLK_DBG_QUEUE, "dev%d-q%d: enter idle %x\n",
> > +			q->dev->dev_info.dev_id, q->q_id, q->state);
> > +	ublk_queue_discard_io_pages(q);
> > +	q->state |= UBLKSRV_QUEUE_IDLE;
> > +}
> > +
> > +static void ublk_queue_idle_exit(struct ublk_queue *q)
> > +{
> > +	if (q->state & UBLKSRV_QUEUE_IDLE) {
> > +		ublk_dbg(UBLK_DBG_QUEUE, "dev%d-q%d: exit idle %x\n",
> > +			q->dev->dev_info.dev_id, q->q_id, q->state);
> > +		q->state &= ~UBLKSRV_QUEUE_IDLE;
> > +	}
> > +}
> > +
> > +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,
> > +			user_data_to_tag(cqe->user_data),
> > +			user_data_to_op(cqe->user_data));
> > +
> > +	if (q->tgt_ops->tgt_io_done)
> > +		q->tgt_ops->tgt_io_done(q, tag, cqe);
> > +}
> > +
> > +static void ublk_handle_cqe(struct io_uring *r,
> > +		struct io_uring_cqe *cqe, void *data)
> > +{
> > +	struct ublk_queue *q = container_of(r, struct ublk_queue, ring);
> > +	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) &&
> > +		!(q->state & UBLKSRV_QUEUE_STOPPING);
> > +	struct ublk_io *io;
> > +
> > +	ublk_dbg(UBLK_DBG_IO_CMD, "%s: res %d (qid %d tag %u cmd_op %u target %d) stopping %d\n",
> > +			__func__, cqe->res, q->q_id, tag, cmd_op,
> > +			is_target_io(cqe->user_data),
> > +			(q->state & UBLKSRV_QUEUE_STOPPING));
> > +
> > +	/* Don't retrieve io in case of target io */
> > +	if (is_target_io(cqe->user_data)) {
> > +		ublksrv_handle_tgt_cqe(q, cqe);
> > +		return;
> > +	}
> > +
> > +	io = &q->ios[tag];
> > +	q->cmd_inflight--;
> > +
> > +	if (!fetch) {
> > +		q->state |= UBLKSRV_QUEUE_STOPPING;
> > +		io->flags &= ~UBLKSRV_NEED_FETCH_RQ;
> > +	}
> > +
> > +	if (cqe->res == UBLK_IO_RES_OK) {
> > +		ublk_assert(tag < q->q_depth);
> > +		q->tgt_ops->queue_io(q, tag);
> > +	} else {
> > +		/*
> > +		 * COMMIT_REQ will be completed immediately since no fetching
> > +		 * 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_*)
> > +		 *
> > +		 * */
> > +		io->flags = UBLKSRV_IO_FREE;
> > +	}
> 
> if you can return; from above if we can remove the else, no biggy
> though..

I think "else" is just fine.

> > +}
> > +
> > +static int ublk_reap_events_uring(struct io_uring *r)
> > +{
> > +	struct io_uring_cqe *cqe;
> > +	unsigned head;
> > +	int count = 0;
> > +
> > +	io_uring_for_each_cqe(r, head, cqe) {
> > +		ublk_handle_cqe(r, cqe, NULL);
> > +		count += 1;
> > +	}
> > +	io_uring_cq_advance(r, count);
> > +
> > +	return count;
> > +}
> > +
> > +static int ublk_process_io(struct ublk_queue *q)
> > +{
> > +	int ret, reapped;
> > +	struct __kernel_timespec ts = {
> > +		.tv_sec = UBLKSRV_IO_IDLE_SECS,
> > +		.tv_nsec = 0
> > +        };
> > +	struct __kernel_timespec *tsp = (q->state & UBLKSRV_QUEUE_IDLE) ?
> > +		NULL : &ts;
> > +	struct io_uring_cqe *cqe;
> > +
> > +	ublk_dbg(UBLK_DBG_QUEUE, "dev%d-q%d: to_submit %d inflight cmd %u stopping %d\n",
> > +				q->dev->dev_info.dev_id,
> > +				q->q_id, io_uring_sq_ready(&q->ring),
> > +				q->cmd_inflight,
> > +				(q->state & UBLKSRV_QUEUE_STOPPING));
> > +
> > +	if (ublk_queue_is_done(q))
> > +		return -ENODEV;
> > +
> > +	ret = io_uring_submit_and_wait_timeout(&q->ring, &cqe, 1, tsp, NULL);
> > +	reapped = ublk_reap_events_uring(&q->ring);
> > +
> > +	ublk_dbg(UBLK_DBG_QUEUE, "submit result %d, reapped %d stop %d idle %d\n",
> > +			ret, reapped, (q->state & UBLKSRV_QUEUE_STOPPING),
> > +			(q->state & UBLKSRV_QUEUE_IDLE));
> > +
> > +	if (!(q->state & UBLKSRV_QUEUE_STOPPING)) {
> > +		if (ret == -ETIME && reapped == 0 && ublk_queue_is_idle(q))
> > +			ublk_queue_idle_enter(q);
> > +		else
> > +			ublk_queue_idle_exit(q);
> > +	}
> > +	return reapped;
> > +}
> > +
> > +static void *ublk_io_handler_fn(void *data)
> > +{
> > +	struct ublk_queue *q = data;
> > +	int dev_id = q->dev->dev_info.dev_id;
> > +	int ret;
> > +
> > +	ret = ublk_queue_init(q);
> > +	if (ret) {
> > +		ublk_err("ublk dev %d queue %d init queue failed\n",
> > +				dev_id, q->q_id);
> > +		return NULL;
> > +	}
> > +
> > +	/* submit all io commands to ublk driver */
> > +	ublk_submit_fetch_commands(q);
> > +
> > +	ublk_dbg(UBLK_DBG_QUEUE, "tid %d: ublk dev %d queue %d started\n",
> > +			gettid(),
> > +			dev_id, q->q_id);
> > +	do {
> > +		if (ublk_process_io(q) < 0)
> > +			break;
> > +	} while (1);
> > +
> > +	ublk_dbg(UBLK_DBG_QUEUE, "ublk dev %d queue %d exited\n", dev_id, q->q_id);
> > +	ublk_queue_deinit(q);
> > +	return NULL;
> > +}
> > +
> > +static void ublk_set_parameters(struct ublk_dev *dev)
> > +{
> > +	int ret;
> > +
> > +	ret = ublk_ctrl_set_params(dev, &dev->tgt.params);
> > +	if (ret)
> > +		ublk_err("dev %d set basic parameter failed %d\n",
> > +				dev->dev_info.dev_id, ret);
> > +}
> > +
> > +static int ublk_start_daemon(struct ublk_dev *dev)
> > +{
> > +	int ret, i;
> > +	void *thread_ret;
> > +	const struct ublksrv_ctrl_dev_info *dinfo = &dev->dev_info;
> > +
> > +	daemon(1, 1);
> > +
> > +	ublk_dbg(UBLK_DBG_DEV, "%s enter\n", __func__);
> > +
> > +	ret = ublk_dev_prep(dev);
> > +	if (!dev)
> > +		return -ENOMEM;
> > +
> 
> can dev be NULL ever here ? do we need to check ret instaed of dev?
> (I don't think so since looking it ublk_dev_prep() it will not change
> dev value and it is unconditionally deferenced before it's use).
> 
> also shuldn't we return ret here as ublk_dev_prep() will retrun non
> -ENOMEM value ?

Good catch, it is really a bug.

> 
> > +	for (i = 0; i < dinfo->nr_hw_queues; i++) {
> > +		dev->q[i].dev = dev;
> > +		dev->q[i].q_id = i;
> > +		pthread_create(&dev->q[i].thread, NULL,
> > +				ublk_io_handler_fn,
> > +				&dev->q[i]);
> > +	}
> > +
> > +	ublk_set_parameters(dev);
> > +
> > +	/* everything is fine now, start us */
> > +	ret = ublk_ctrl_start_dev(dev, getpid());
> > +	if (ret < 0)
> > +		goto fail;
> > +
> > +	ublk_ctrl_get_info(dev);
> > +	ublk_ctrl_dump(dev, true);
> > +
> > +	/* wait until we are terminated */
> > +	for (i = 0; i < dinfo->nr_hw_queues; i++)
> > +		pthread_join(dev->q[i].thread, &thread_ret);
> > + fail:
> > +	ublk_dev_unprep(dev);
> > +	ublk_dbg(UBLK_DBG_DEV, "%s exit\n", __func__);
> > +
> > +	return ret;
> > +}
> > +
> > +static int cmd_dev_add(int argc, char *argv[])
> > +{
> > +	static const struct option longopts[] = {
> > +		{ "type",		1,	NULL, 't' },
> > +		{ "number",		1,	NULL, 'n' },
> > +		{ "queues",		1,	NULL, 'q' },
> > +		{ "depth",		1,	NULL, 'd' },
> > +		{ "debug_mask",	1,	NULL, 0},
> > +		{ "quiet",	0,	NULL, 0},
> > +		{ NULL }
> > +	};
> > +	const struct ublk_tgt_ops *ops;
> > +	struct ublksrv_ctrl_dev_info *info;
> > +	struct ublk_dev *dev;
> > +	int ret, option_idx, opt;
> > +	const char *tgt_type = NULL;
> > +	int dev_id = -1;
> > +	unsigned nr_queues = 2, depth = UBLK_QUEUE_DEPTH;
> > +
> > +	while ((opt = getopt_long(argc, argv, "-:t:n:d:q:",
> > +				  longopts, &option_idx)) != -1) {
> > +		switch (opt) {
> > +		case 'n':
> > +			dev_id = strtol(optarg, NULL, 10);
> > +			break;
> > +		case 't':
> > +			tgt_type = optarg;
> > +			break;
> > +		case 'q':
> > +			nr_queues = strtol(optarg, NULL, 10);
> > +			break;
> > +		case 'd':
> > +			depth = strtol(optarg, NULL, 10);
> > +			break;
> > +		case 0:
> > +			if (!strcmp(longopts[option_idx].name, "debug_mask"))
> > +				ublk_dbg_mask = strtol(optarg, NULL, 16);
> > +			if (!strcmp(longopts[option_idx].name, "quiet"))
> > +				ublk_dbg_mask = 0;
> > +			break;
> > +		}
> > +	}
> > +
> > +	optind = 0;
> > +
> > +	ops = ublk_find_tgt(tgt_type);
> > +	if (!ops) {
> > +		ublk_err("%s: no such tgt type, type %s\n",
> > +				__func__, tgt_type);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (nr_queues > UBLK_MAX_QUEUES || depth > UBLK_QUEUE_DEPTH) {
> > +		ublk_err("%s: invalid nr_queues or depth queues %u depth %u\n",
> > +				__func__, nr_queues, depth);
> > +		return -EINVAL;
> > +	}
> > +
> > +	dev = ublk_ctrl_init();
> > +	if (!dev) {
> > +		ublk_err("%s: can't alloc dev id %d, type %s\n",
> > +				__func__, dev_id, tgt_type);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	info = &dev->dev_info;
> > +	info->dev_id = dev_id;
> > +        info->nr_hw_queues = nr_queues;
> > +        info->queue_depth = depth;
> > +	dev->tgt.ops = ops;
> > +	dev->tgt.argc = argc;
> > +	dev->tgt.argv = argv;
> > +
> > +	ret = ublk_ctrl_add_dev(dev);
> > +	if (ret < 0) {
> > +		ublk_err("%s: can't add dev id %d, type %s ret %d\n",
> > +				__func__, dev_id, tgt_type, ret);
> > +		goto fail;
> > +	}
> > +
> > +	ret = ublk_start_daemon(dev);
> > +	if (ret < 0) {
> > +		ublk_err("%s: can't start daemon id %d, type %s\n",
> > +				__func__, dev_id, tgt_type);
> > +		goto fail_del;
> > +	}
> > +fail_del:
> > +	ublk_ctrl_del_dev(dev);
> > +fail:
> > +	ublk_ctrl_deinit(dev);
> > +	return ret;
> > +}
> > +
> > +static int ublk_stop_io_daemon(const struct ublk_dev *dev)
> > +{
> > +	int daemon_pid = dev->dev_info.ublksrv_pid;
> > +	int cnt = 0, ret;
> > +
> > +	if (daemon_pid == -1)
> > +		return 0;
> > +
> > +	/* wait until daemon is exited, or timeout after 3 seconds */
> > +	do {
> > +		ret = kill(daemon_pid, 0);
> > +		if (ret)
> > +			break;
> > +		usleep(500000);
> > +		cnt++;
> > +	} while (!ret && cnt < 6);
> > +
> > +	ublk_dbg(UBLK_DBG_DEV, "%s: pid %d ret %d\n", __func__, daemon_pid, ret);
> > +
> > +	return ret != 0 ? 0 : -1;
> > +}
> > +
> > +static int __cmd_dev_del(int number, bool log)
> > +{
> > +	struct ublk_dev *dev;
> > +	int ret;
> > +
> > +	dev = ublk_ctrl_init();
> > +	dev->dev_info.dev_id = number;
> > +
> > +	ret = ublk_ctrl_get_info(dev);
> > +	if (ret < 0) {
> > +		goto fail;
> > +	}
> > +
> > +	ret = ublk_ctrl_stop_dev(dev);
> > +	if (ret < 0) {
> > +		if (log)
> > +			ublk_err("stop dev %d failed\n", number);
> > +		goto fail;
> > +	}
> > +
> > +	ret = ublk_stop_io_daemon(dev);
> > +	if (ret < 0) {
> > +		if (log)
> > +			ublk_err("stop daemon %d failed\n", number);
> > +	}
> > +
> > +	ublk_ctrl_del_dev(dev);
> > +fail:
> > +	ublk_ctrl_deinit(dev);
> > +	return ret;
> > +}
> > +
> > +static int cmd_dev_del(int argc, char *argv[])
> > +{
> > +	static const struct option longopts[] = {
> > +		{ "number",		1,	NULL, 'n' },
> > +		{ "all",		0,	NULL, 'a' },
> > +		{ "debug_mask",	1,	NULL, 0},
> > +		{ NULL }
> > +	};
> > +	int number = -2;
> > +	int opt, i, option_idx;
> > +
> > +	while ((opt = getopt_long(argc, argv, "n:a",
> > +				  longopts, &option_idx)) != -1) {
> > +		switch (opt) {
> > +		case 'a':
> > +			number = -1;
> > +			break;
> > +
> > +		case 'n':
> > +			number = strtol(optarg, NULL, 10);
> > +			break;
> > +		case 0:
> > +			if (!strcmp(longopts[option_idx].name, "debug_mask"))
> > +				ublk_dbg_mask = strtol(optarg, NULL, 16);
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (number >= 0)
> > +		return __cmd_dev_del(number, true);
> > +	else if (number != -1) {
> > +		ublk_err("%s: pass wrong devid or not delete via -a\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < 255; i++)
> > +		__cmd_dev_del(i, false);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __cmd_dev_list(int number, bool log)
> > +{
> > +	struct ublk_dev *dev = ublk_ctrl_init();
> > +	int ret;
> > +
> > +	dev->dev_info.dev_id = number;
> > +
> > +	ret = ublk_ctrl_get_info(dev);
> > +	if (ret < 0) {
> > +		if (log)
> > +			ublk_err("%s: can't get dev info from %d: %d\n",
> > +					__func__, number, ret);
> > +	} else {
> > +		ublk_ctrl_dump(dev, false);
> > +	}
> > +
> > +	ublk_ctrl_deinit(dev);
> > +
> > +	return ret;
> > +}
> > +
> > +
> > +static int cmd_dev_list(int argc, char *argv[])
> > +{
> > +	static const struct option longopts[] = {
> > +		{ "number",		1,	NULL, 'n' },
> > +		{ "all",		0,	NULL, 'a' },
> > +		{ NULL }
> > +	};
> > +	int number = -1;
> > +	int opt, i;
> > +
> > +	while ((opt = getopt_long(argc, argv, "n:a",
> > +				  longopts, NULL)) != -1) {
> > +		switch (opt) {
> > +		case 'a':
> > +			break;
> > +
> > +		case 'n':
> > +			number = strtol(optarg, NULL, 10);
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (number >= 0)
> > +		return __cmd_dev_list(number, true);
> > +
> > +	for (i = 0; i < 255; i++)
> > +		__cmd_dev_list(i, false);
> > +
> > +	return 0;
> > +}
> > +
> > +static int cmd_dev_help(int argc, char *argv[])
> > +{
> > +	printf("%s add -t {null|loop} [-q nr_queues] [-d depth] [-n dev_id] \n",
> > +			argv[0]);
> > +	printf("\t default: nr_queues=2(max 4), depth=128(max 128), dev_id=-1(auto allocation)\n");
> > +	printf("\t -t loop -f backing_file \n");
> > +	printf("\t -t null\n");
> > +	printf("%s del [-n dev_id] -a \n", argv[0]);
> > +	printf("\t -a delete all devices -n delete specified device\n");
> > +	printf("%s list [-n dev_id] -a \n", argv[0]);
> > +	printf("\t -a list all devices, -n list specified device, default -a \n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int ublk_null_tgt_init(struct ublk_dev *dev)
> > +{
> > +	const struct ublksrv_ctrl_dev_info *info = &dev->dev_info;
> > +	unsigned long dev_size = 250UL << 30;
> > +
> > +	dev->tgt.dev_size = dev_size;
> > +	dev->tgt.params = (struct ublk_params) {
> > +		.types = UBLK_PARAM_TYPE_BASIC,
> > +		.basic = {
> > +			.logical_bs_shift	= 9,
> > +			.physical_bs_shift	= 12,
> > +			.io_opt_shift		= 12,
> > +			.io_min_shift		= 9,
> > +			.max_sectors		= info->max_io_buf_bytes >> 9,
> > +			.dev_sectors		= dev_size >> 9,
> > +		},
> > +	};
> > +
> > +	return 0;
> > +}
> > +
> > +static int ublk_null_queue_io(struct ublk_queue *q, int tag)
> > +{
> > +	const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
> > +
> > +	ublk_complete_io(q, tag, iod->nr_sectors << 9);
> > +
> > +	return 0;
> > +}
> > +
> > +static int loop_queue_tgt_io(struct ublk_queue *q, int tag)
> > +{
> > +	const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
> > +	struct io_uring_sqe *sqe = io_uring_get_sqe(&q->ring);
> > +	unsigned ublk_op = ublksrv_get_op(iod);
> > +
> > +	if (!sqe)
> > +		return -ENOMEM;
> > +
> > +	switch (ublk_op) {
> > +	case UBLK_IO_OP_FLUSH:
> > +		io_uring_prep_sync_file_range(sqe, 1 /*fds[1]*/,
> > +				iod->nr_sectors << 9,
> > +				iod->start_sector << 9,
> > +				IORING_FSYNC_DATASYNC);
> > +		io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);
> > +		break;
> > +	case UBLK_IO_OP_WRITE_ZEROES:
> > +	case UBLK_IO_OP_DISCARD:
> > +		return -ENOTSUP;
> > +	case UBLK_IO_OP_READ:
> > +		io_uring_prep_read(sqe, 1 /*fds[1]*/,
> > +				(void *)iod->addr,
> > +				iod->nr_sectors << 9,
> > +				iod->start_sector << 9);
> > +		io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);
> > +		break;
> > +	case UBLK_IO_OP_WRITE:
> > +		io_uring_prep_write(sqe, 1 /*fds[1]*/,
> > +				(void *)iod->addr,
> > +				iod->nr_sectors << 9,
> > +				iod->start_sector << 9);
> > +		io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	q->io_inflight++;
> > +	/* bit63 marks us as tgt io */
> > +	sqe->user_data = build_user_data(tag, ublk_op, 0, 1);
> > +
> > +	ublk_dbg(UBLK_DBG_IO, "%s: tag %d ublk io %x %llx %u\n", __func__, tag,
> > +			iod->op_flags, iod->start_sector, iod->nr_sectors << 9);
> > +	return 1;
> > +}
> > +
> > +static int ublk_loop_queue_io(struct ublk_queue *q, int tag)
> > +{
> > +	int queued = loop_queue_tgt_io(q, tag);
> > +
> > +	if (queued < 0)
> > +		ublk_complete_io(q, tag, queued);
> > +
> > +	return 0;
> > +}
> > +
> > +static void ublk_loop_io_done(struct ublk_queue *q, int tag,
> > +		const struct io_uring_cqe *cqe)
> > +{
> > +	int cqe_tag = user_data_to_tag(cqe->user_data);
> > +
> > +	ublk_assert(tag == cqe_tag);
> > +	ublk_complete_io(q, tag, cqe->res);
> > +	q->io_inflight--;
> > +}
> > +
> > +static void ublk_loop_tgt_deinit(struct ublk_dev *dev)
> > +{
> > +	fsync(dev->fds[1]);
> > +	close(dev->fds[1]);
> > +}
> > +
> > +static int ublk_loop_tgt_init(struct ublk_dev *dev)
> > +{
> > +	static const struct option lo_longopts[] = {
> > +		{ "file",		1,	NULL, 'f' },
> > +		{ NULL }
> > +	};
> > +	unsigned long long bytes;
> > +	char **argv = dev->tgt.argv;
> > +	int argc = dev->tgt.argc;
> > +	char *file = NULL;
> > +	struct stat st;
> > +	int fd, opt;
> > +	struct ublk_params p = {
> > +		.types = UBLK_PARAM_TYPE_BASIC,
> > +		.basic = {
> > +			.logical_bs_shift	= 9,
> > +			.physical_bs_shift	= 12,
> > +			.io_opt_shift	= 12,
> > +			.io_min_shift	= 9,
> > +			.max_sectors = dev->dev_info.max_io_buf_bytes >> 9,
> > +		},
> > +	};
> > +
> > +	while ((opt = getopt_long(argc, argv, "-:f:",
> > +				  lo_longopts, NULL)) != -1) {
> > +		switch (opt) {
> > +		case 'f':
> > +			file = strdup(optarg);
> > +			break;
> > +		}
> > +	}
> > +
> > +	ublk_dbg(UBLK_DBG_DEV, "%s: file %s\n", __func__, file);
> > +
> > +	if (!file)
> > +		return -EINVAL;
> > +
> > +	fd = open(file, O_RDWR);
> > +	if (fd < 0) {
> > +		ublk_err( "%s: backing file %s can't be opened\n",
> > +				__func__, file);
> > +		return -EBADF;
> > +	}
> > +
> > +	if (fstat(fd, &st) < 0) {
> > +		close(fd);
> > +		return -EBADF;
> > +	}
> > +
> > +	if (S_ISBLK(st.st_mode)) {
> > +		unsigned int bs, pbs;
> > +
> > +		if (ioctl(fd, BLKGETSIZE64, &bytes) != 0)
> > +			return -EBADF;
> > +		if (ioctl(fd, BLKSSZGET, &bs) != 0)
> > +			return -1;
> > +		if (ioctl(fd, BLKPBSZGET, &pbs) != 0)
> > +			return -1;
> > +		p.basic.logical_bs_shift = ilog2(bs);
> > +		p.basic.physical_bs_shift = ilog2(pbs);
> > +	} else if (S_ISREG(st.st_mode)) {
> > +		bytes = st.st_size;
> > +	} else {
> > +		bytes = 0;
> > +	}
> > +
> > +	if (fcntl(fd, F_SETFL, O_DIRECT)) {
> > +		p.basic.logical_bs_shift = 9;
> > +		p.basic.physical_bs_shift = 12;
> > +		ublk_log("%s: ublk-loop fallback to buffered IO\n", __func__);
> > +	}
> > +
> > +	dev->tgt.dev_size = bytes;
> > +	p.basic.dev_sectors = bytes >> 9;
> > +	dev->fds[1] = fd;
> > +	dev->nr_fds += 1;
> > +	dev->tgt.params = p;
> > +
> > +	return 0;
> > +}
> > +
> > +const struct ublk_tgt_ops tgt_ops_list[] = {
> > +	{
> > +		.name = "null",
> > +		.init_tgt = ublk_null_tgt_init,
> > +		.queue_io = ublk_null_queue_io,
> > +	},
> > +
> > +	{
> > +		.name = "loop",
> > +		.init_tgt = ublk_loop_tgt_init,
> > +		.deinit_tgt = ublk_loop_tgt_deinit,
> > +		.queue_io = ublk_loop_queue_io,
> > +		.tgt_io_done = ublk_loop_io_done,
> > +	},
> > +};
> > +
> > +static const struct ublk_tgt_ops *ublk_find_tgt(const char *name)
> > +{
> > +	const struct ublk_tgt_ops *ops;
> > +	int i;
> > +
> > +	if (name == NULL)
> > +		return NULL;
> > +
> > +	for (i = 0; sizeof(tgt_ops_list) / sizeof(*ops); i++)
> > +		if (strcmp(tgt_ops_list[i].name, name) == 0)
> > +			return &tgt_ops_list[i];
> > +	return NULL;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	const char *cmd = argv[1];
> > +	int ret = -EINVAL;
> > +
> > +	if (argc == 1)
> > +		goto out;
> > +
> > +	if (!strcmp(cmd, "add"))
> > +		ret = cmd_dev_add(argc, argv);
> > +	else if (!strcmp(cmd, "del"))
> > +		ret = cmd_dev_del(argc, argv);
> > +	else if (!strcmp(cmd, "list"))
> > +		ret = cmd_dev_list(argc, argv);
> > +	else if (!strcmp(cmd, "help"))
> > +		ret = cmd_dev_help(argc, argv);
> > +out:
> > +	if (ret)
> > +		cmd_dev_help(argc, argv);
> > +
> > +	return ret;
> > +}

thanks,
Ming