[PATCH V4 0/2] ublk: add io_uring based userspace block driver

[PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Ming Lei]

Hello Guys,

ublk driver is one kernel driver for implementing generic userspace block
device/driver, which delivers io request from ublk block device(/dev/ublkbN) into
ublk server[1] which is the userspace part of ublk for communicating
with ublk driver and handling specific io logic by its target module.

Another thing ublk driver handles is to copy data between user space buffer
and request/bio's pages, or take zero copy if mm is ready for support it in
future. ublk driver doesn't handle any IO logic of the specific driver, so
it is small/simple, and all io logics are done by the target code in ublkserver.

The above two are main jobs done by ublk driver.

ublk driver can help to move IO logic into userspace, in which the
development work is easier/more effective than doing in kernel, such as,
ublk-loop takes < 200 lines of loop specific code to get basically same 
function with kernel loop block driver, meantime the performance is
is even better than kernel loop with same setting. ublksrv[1] provide built-in
test for comparing both by running "make test T=loop", for example, see
the test result running on VM which is over my lattop(root disk is
nvme/device mapper/xfs):

	[root@ktest-36 ubdsrv]#make -s -C /root/git/ubdsrv/tests run T=loop/001 R=10
	running loop/001
		fio (ublk/loop(/root/git/ubdsrv/tests/tmp/ublk_loop_VqbMA), libaio, bs 4k, dio, hw queues:1)...
		randwrite: jobs 1, iops 32572
		randread: jobs 1, iops 143052
		rw: jobs 1, iops read 29919 write 29964
	
	[root@ktest-36 ubdsrv]# make test T=loop/003
	make -s -C /root/git/ubdsrv/tests run T=loop/003 R=10
	running loop/003
		fio (kernel_loop/kloop(/root/git/ubdsrv/tests/tmp/ublk_loop_ZIVnG), libaio, bs 4k, dio, hw queues:1)...
		randwrite: jobs 1, iops 27436
		randread: jobs 1, iops 95273
		rw: jobs 1, iops read 22542 write 22543 


Another example is high performance qcow2 support[2], which could be built with
ublk framework more easily than doing it inside kernel.

Also there are more people who express interests on userspace block driver[3],
Gabriel Krisman Bertazi proposes this topic in lsf/mm/ebpf 2022 and mentioned
requirement from Google. Ziyang Zhang from Alibaba said they "plan to
replace TCMU by UBD as a new choice" because UBD can get better throughput than
TCMU even with single queue[4], meantime UBD is simple. Also there is userspace
storage service for providing storage to containers.

It is io_uring based: io request is delivered to userspace via new added
io_uring command which has been proved as very efficient for making nvme
passthrough IO to get better IOPS than io_uring(READ/WRITE). Meantime one
shared/mmap buffer is used for sharing io descriptor to userspace, the
buffer is readonly for userspace, each IO just takes 24bytes so far.
It is suggested to use io_uring in userspace(target part of ublk server)
to handle IO request too. And it is still easy for ublkserver to support
io handling by non-io_uring, and this work isn't done yet, but can be
supported easily with help o eventfd.

This way is efficient since no extra io command copy is required, no sleep
is needed in transferring io command to userspace. Meantime the communication
protocol is simple and efficient, one single command of
UBD_IO_COMMIT_AND_FETCH_REQ can handle both fetching io request desc and commit
command result in one trip. IO handling is often batched after single
io_uring_enter() returns, both IO requests from ublk server target and
IO commands could be handled as a whole batch.

And the patch by patch change can be found in the following
tree:

https://github.com/ming1/linux/tree/my_for-5.20-ubd-devel_v4

ublk server repo(master branch):

	https://github.com/ming1/ubdsrv

Any comments are welcome!

Since V3:
- address Gabriel Krisman Bertazi's comments on V3: add userspace data
  validation before handling command, remove warning, ...
- remove UBLK_IO_COMMIT_REQ command as suggested by Zixiang and Gabriel Krisman Bertazi
- fix one request double free when running abort
- rewrite/cleanup ublk_copy_pages(), then this handling becomes very
  clean
- add one command of UBLK_IO_REFETCH_REQ for allowing ublk_drv to build
  as module

Since V2:
- fix one big performance problem:
	https://github.com/ming1/linux/commit/3c9fd476951759858cc548dee4cedc074194d0b0
- rename as ublk, as suggested by Gabriel Krisman Bertazi 
- lots of cleanup & code improvement & bugfix, see details in git
  hisotry


Since V1:

Remove RFC now because ublk driver codes gets lots of cleanup, enhancement and
bug fixes since V1:

- cleanup uapi: remove ublk specific error code,  switch to linux error code,
remove one command op, remove one field from cmd_desc

- add monitor mechanism to handle ubq_daemon being killed, ublksrv[1]
  includes builtin tests for covering heavy IO with deleting ublk / killing
  ubq_daemon at the same time, and V2 pass all the two tests(make test T=generic),
  and the abort/stop mechanism is simple

- fix MQ command buffer mmap bug, and now 'xfstetests -g auto' works well on
  MQ ublk-loop devices(test/scratch)

- improve batching submission as suggested by Jens

- improve handling for starting device, replace random wait/poll with
completion

- all kinds of cleanup, bug fix,..

Ming Lei (2):
  ublk: add io_uring based userspace block driver
  ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

 drivers/block/Kconfig         |    6 +
 drivers/block/Makefile        |    2 +
 drivers/block/ublk_drv.c      | 1701 +++++++++++++++++++++++++++++++++
 include/uapi/linux/ublk_cmd.h |  173 ++++
 4 files changed, 1882 insertions(+)
 create mode 100644 drivers/block/ublk_drv.c
 create mode 100644 include/uapi/linux/ublk_cmd.h

-- 
2.31.1

[PATCH V4 1/2] ublk: add io_uring based userspace block driver

[Ming Lei]

This is the driver part of userspace block driver(ublk driver), the other
part is userspace daemon part(ublksrv)[1].

The two parts communicate by io_uring's IORING_OP_URING_CMD with one
shared cmd buffer for storing io command, and the buffer is read only for
ublksrv, each io command is indexed by io request tag directly, and
is written by ublk driver.

For example, when one READ io request is submitted to ublk block driver, ublk
driver stores the io command into cmd buffer first, then completes one
IORING_OP_URING_CMD for notifying ublksrv, and the URING_CMD is issued to
ublk driver beforehand by ublksrv for getting notification of any new io request,
and each URING_CMD is associated with one io request by tag.

After ublksrv gets the io command, it translates and handles the ublk io
request, such as, for the ublk-loop target, ublksrv translates the request
into same request on another file or disk, like the kernel loop block
driver. In ublksrv's implementation, the io is still handled by io_uring,
and share same ring with IORING_OP_URING_CMD command. When the target io
request is done, the same IORING_OP_URING_CMD is issued to ublk driver for
both committing io request result and getting future notification of new
io request.

Another thing done by ublk driver is to copy data between kernel io
request and ublksrv's io buffer:

1) before ubsrv handles WRITE request, copy the request's data into
ublksrv's userspace io buffer, so that ublksrv can handle the write
request

2) after ubsrv handles READ request, copy ublksrv's userspace io buffer
into this READ request, then ublk driver can complete the READ request

Zero copy may be switched if mm is ready to support it.

ublk driver doesn't handle any logic of the specific user space driver,
so it is small/simple enough.

[1] ublksrv

https://github.com/ming1/ubdsrv

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/Kconfig         |    6 +
 drivers/block/Makefile        |    2 +
 drivers/block/ublk_drv.c      | 1632 +++++++++++++++++++++++++++++++++
 include/uapi/linux/ublk_cmd.h |  156 ++++
 4 files changed, 1796 insertions(+)
 create mode 100644 drivers/block/ublk_drv.c
 create mode 100644 include/uapi/linux/ublk_cmd.h

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index fdb81f2794cd..d218089cdbec 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -408,6 +408,12 @@ config BLK_DEV_RBD
 
 	  If unsure, say N.
 
+config BLK_DEV_UBLK
+	bool "Userspace block driver"
+	select IO_URING
+	help
+          io uring based userspace block driver.
+
 source "drivers/block/rnbd/Kconfig"
 
 endif # BLK_DEV
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index 934a9c7c3a7c..be631352567e 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -39,4 +39,6 @@ obj-$(CONFIG_BLK_DEV_RNBD)	+= rnbd/
 
 obj-$(CONFIG_BLK_DEV_NULL_BLK)	+= null_blk/
 
+obj-$(CONFIG_BLK_DEV_UBLK)			+= ublk_drv.o
+
 swim_mod-y	:= swim.o swim_asm.o
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
new file mode 100644
index 000000000000..0076418e6fad
--- /dev/null
+++ b/drivers/block/ublk_drv.c
@@ -0,0 +1,1632 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Userspace block device - block device which IO is handled from userspace
+ *
+ * Take full use of io_uring passthrough command for communicating with
+ * ublk userspace daemon(ublksrvd) for handling basic IO request.
+ *
+ * Copyright 2022 Ming Lei <ming.lei@redhat.com>
+ *
+ * (part of code stolen from loop.c)
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/sched.h>
+#include <linux/fs.h>
+#include <linux/pagemap.h>
+#include <linux/file.h>
+#include <linux/stat.h>
+#include <linux/errno.h>
+#include <linux/major.h>
+#include <linux/wait.h>
+#include <linux/blkdev.h>
+#include <linux/init.h>
+#include <linux/swap.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/mutex.h>
+#include <linux/writeback.h>
+#include <linux/completion.h>
+#include <linux/highmem.h>
+#include <linux/sysfs.h>
+#include <linux/miscdevice.h>
+#include <linux/falloc.h>
+#include <linux/uio.h>
+#include <linux/ioprio.h>
+#include <linux/sched/mm.h>
+#include <linux/uaccess.h>
+#include <linux/cdev.h>
+#include <linux/io_uring.h>
+#include <linux/blk-mq.h>
+#include <linux/delay.h>
+#include <linux/mm.h>
+#include <asm/page.h>
+#include <linux/task_work.h>
+#include <uapi/linux/ublk_cmd.h>
+
+#define UBLK_MINORS		(1U << MINORBITS)
+
+struct ublk_rq_data {
+	struct callback_head work;
+};
+
+/*
+ * io command is active: sqe cmd is received, and its cqe isn't done
+ *
+ * If the flag is set, the io command is owned by ublk driver, and waited
+ * for incoming blk-mq request from the ublk block device.
+ *
+ * If the flag is cleared, the io command will be completed, and owned by
+ * ublk server.
+ */
+#define UBLK_IO_FLAG_ACTIVE	0x01
+
+/*
+ * IO command is completed via cqe, and it is being handled by ublksrv, and
+ * not committed yet
+ *
+ * Basically exclusively with UBLK_IO_FLAG_ACTIVE, so can be served for
+ * cross verification
+ */
+#define UBLK_IO_FLAG_OWNED_BY_SRV 0x02
+
+/*
+ * IO command is aborted, so this flag is set in case of
+ * !UBLK_IO_FLAG_ACTIVE.
+ *
+ * After this flag is observed, any pending or new incoming request
+ * associated with this io command will be failed immediately
+ */
+#define UBLK_IO_FLAG_ABORTED 0x04
+
+struct ublk_io {
+	/* userspace buffer address from io cmd */
+	__u64	addr;
+	unsigned int flags;
+	int res;
+
+	struct io_uring_cmd *cmd;
+};
+
+struct ublk_queue {
+	int q_id;
+	int q_depth;
+
+	struct task_struct	*ubq_daemon;
+	char *io_cmd_buf;
+
+	unsigned long io_addr;	/* mapped vm address */
+	unsigned int max_io_sz;
+	bool abort_work_pending;
+	unsigned short nr_io_ready;	/* how many ios setup */
+	struct ublk_device *dev;
+	spinlock_t	abort_lock;
+	struct callback_head abort_work;
+	struct ublk_io ios[0];
+};
+
+#define UBLK_DAEMON_MONITOR_PERIOD	(5 * HZ)
+
+struct ublk_device {
+	struct gendisk		*ub_disk;
+	struct request_queue	*ub_queue;
+
+	char	*__queues;
+
+	unsigned short  queue_size;
+	unsigned short  bs_shift;
+	struct ublksrv_ctrl_dev_info	dev_info;
+
+	struct blk_mq_tag_set	tag_set;
+
+	struct cdev		cdev;
+	struct device		cdev_dev;
+
+	atomic_t		ch_open_cnt;
+	int			ub_number;
+
+	struct mutex		mutex;
+
+	struct mm_struct	*mm;
+
+	struct completion	completion;
+	unsigned int		nr_queues_ready;
+	atomic_t		nr_aborted_queues;
+
+	/*
+	 * Our ubq->daemon may be killed without any notification, so
+	 * monitor each queue's daemon periodically
+	 */
+	struct delayed_work	monitor_work;
+	struct work_struct	stop_work;
+};
+
+static dev_t ublk_chr_devt;
+static struct class *ublk_chr_class;
+
+static DEFINE_IDR(ublk_index_idr);
+static DEFINE_SPINLOCK(ublk_idr_lock);
+static wait_queue_head_t ublk_idr_wq;	/* wait until one idr is freed */
+
+static DEFINE_MUTEX(ublk_ctl_mutex);
+
+static struct miscdevice ublk_misc;
+
+static struct ublk_device *ublk_get_device(struct ublk_device *ub)
+{
+	if (kobject_get_unless_zero(&ub->cdev_dev.kobj))
+		return ub;
+	return NULL;
+}
+
+static void ublk_put_device(struct ublk_device *ub)
+{
+	put_device(&ub->cdev_dev);
+}
+
+static inline struct ublk_queue *ublk_get_queue(struct ublk_device *dev,
+		int qid)
+{
+       return (struct ublk_queue *)&(dev->__queues[qid * dev->queue_size]);
+}
+
+static inline bool ublk_rq_has_data(const struct request *rq)
+{
+	return rq->bio && bio_has_data(rq->bio);
+}
+
+static inline struct ublksrv_io_desc *ublk_get_iod(struct ublk_queue *ubq,
+		int tag)
+{
+	return (struct ublksrv_io_desc *)
+		&(ubq->io_cmd_buf[tag * sizeof(struct ublksrv_io_desc)]);
+}
+
+static inline char *ublk_queue_cmd_buf(struct ublk_device *ub, int q_id)
+{
+	return ublk_get_queue(ub, q_id)->io_cmd_buf;
+}
+
+static inline int ublk_queue_cmd_buf_size(struct ublk_device *ub, int q_id)
+{
+	struct ublk_queue *ubq = ublk_get_queue(ub, q_id);
+
+	return round_up(ubq->q_depth * sizeof(struct ublksrv_io_desc),
+			PAGE_SIZE);
+}
+
+static int ublk_open(struct block_device *bdev, fmode_t mode)
+{
+	return 0;
+}
+
+static void ublk_release(struct gendisk *disk, fmode_t mode)
+{
+}
+
+static const struct block_device_operations ub_fops = {
+	.owner =	THIS_MODULE,
+	.open =		ublk_open,
+	.release =	ublk_release,
+};
+
+#define UBLK_MAX_PIN_PAGES	32
+
+struct ublk_map_data {
+	const struct ublk_queue *ubq;
+	const struct request *rq;
+	const struct ublk_io *io;
+	unsigned max_bytes;
+};
+
+struct ublk_io_iter {
+	struct page *pages[UBLK_MAX_PIN_PAGES];
+	unsigned pg_off;	/* offset in the 1st page in pages */
+	int nr_pages;		/* how many page pointers in pages */
+	struct bio *bio;
+	struct bvec_iter iter;
+};
+
+static inline unsigned ublk_copy_io_pages(struct ublk_io_iter *data,
+		unsigned max_bytes, bool to_vm)
+{
+	const unsigned total = min_t(unsigned, max_bytes,
+			PAGE_SIZE - data->pg_off +
+			((data->nr_pages - 1) << PAGE_SHIFT));
+	unsigned done = 0;
+	unsigned pg_idx = 0;
+
+	while (done < total) {
+		struct bio_vec bv = bio_iter_iovec(data->bio, data->iter);
+		const unsigned int bytes = min3(bv.bv_len, total - done,
+				(unsigned)(PAGE_SIZE - data->pg_off));
+		void *bv_buf = bvec_kmap_local(&bv);
+		void *pg_buf = kmap_local_page(data->pages[pg_idx]);
+
+		if (to_vm)
+			memcpy(pg_buf + data->pg_off, bv_buf, bytes);
+		else
+			memcpy(bv_buf, pg_buf + data->pg_off, bytes);
+
+		kunmap_local(pg_buf);
+		kunmap_local(bv_buf);
+
+		/* advance page array */
+		data->pg_off += bytes;
+		if (data->pg_off == PAGE_SIZE) {
+			pg_idx += 1;
+			data->pg_off = 0;
+		}
+
+		done += bytes;
+
+		/* advance bio */
+		bio_advance_iter_single(data->bio, &data->iter, bytes);
+		if (!data->iter.bi_size) {
+			data->bio = data->bio->bi_next;
+			if (data->bio == NULL)
+				break;
+			data->iter = data->bio->bi_iter;
+		}
+	}
+
+	return done;
+}
+
+static inline int ublk_copy_user_pages(struct ublk_map_data *data,
+		bool to_vm)
+{
+	const unsigned int gup_flags = to_vm ? FOLL_WRITE : 0;
+	const unsigned long start_vm = data->io->addr;
+	unsigned int done = 0;
+	struct ublk_io_iter iter = {
+		.pg_off	= start_vm & (PAGE_SIZE - 1),
+		.bio	= data->rq->bio,
+		.iter	= data->rq->bio->bi_iter,
+	};
+	const unsigned int nr_pages = round_up(data->max_bytes +
+			(start_vm & (PAGE_SIZE - 1)), PAGE_SIZE) >> PAGE_SHIFT;
+
+	while (done < nr_pages) {
+		const unsigned to_pin = min_t(unsigned, UBLK_MAX_PIN_PAGES,
+				nr_pages - done);
+		unsigned i, len;
+
+		iter.nr_pages = get_user_pages_fast(start_vm +
+				(done << PAGE_SHIFT), to_pin, gup_flags,
+				iter.pages);
+		if (iter.nr_pages <= 0)
+			return done == 0 ? iter.nr_pages : done;
+		len = ublk_copy_io_pages(&iter, data->max_bytes, to_vm);
+		for (i = 0; i < iter.nr_pages; i++) {
+			if (to_vm)
+				set_page_dirty(iter.pages[i]);
+			put_page(iter.pages[i]);
+		}
+		data->max_bytes -= len;
+		done += iter.nr_pages;
+	}
+
+	return done;
+}
+
+static int ublk_map_io(const struct ublk_queue *ubq, const struct request *req,
+		struct ublk_io *io)
+{
+	const unsigned int rq_bytes = blk_rq_bytes(req);
+	/*
+	 * no zero copy, we delay copy WRITE request data into ublksrv
+	 * context via task_work_add and the big benefit is that pinning
+	 * pages in current context is pretty fast, see ublk_pin_user_pages
+	 */
+	if (req_op(req) != REQ_OP_WRITE && req_op(req) != REQ_OP_FLUSH)
+		return rq_bytes;
+
+	if (ublk_rq_has_data(req)) {
+		struct ublk_map_data data = {
+			.ubq	=	ubq,
+			.rq	=	req,
+			.io	=	io,
+			.max_bytes =	rq_bytes,
+		};
+
+		ublk_copy_user_pages(&data, true);
+
+		return rq_bytes - data.max_bytes;
+	}
+	return rq_bytes;
+}
+
+static int ublk_unmap_io(const struct ublk_queue *ubq,
+		const struct request *req,
+		struct ublk_io *io)
+{
+	const unsigned int rq_bytes = blk_rq_bytes(req);
+
+	if (req_op(req) == REQ_OP_READ && ublk_rq_has_data(req)) {
+		struct ublk_map_data data = {
+			.ubq	=	ubq,
+			.rq	=	req,
+			.io	=	io,
+			.max_bytes =	io->res,
+		};
+
+		WARN_ON_ONCE(io->res > rq_bytes);
+
+		ublk_copy_user_pages(&data, false);
+
+		return io->res - data.max_bytes;
+	}
+	return rq_bytes;
+}
+
+static inline unsigned int ublk_req_build_flags(struct request *req)
+{
+	unsigned flags = 0;
+
+	if (req->cmd_flags & REQ_FAILFAST_DEV)
+		flags |= UBLK_IO_F_FAILFAST_DEV;
+
+	if (req->cmd_flags & REQ_FAILFAST_TRANSPORT)
+		flags |= UBLK_IO_F_FAILFAST_TRANSPORT;
+
+	if (req->cmd_flags & REQ_FAILFAST_DRIVER)
+		flags |= UBLK_IO_F_FAILFAST_DRIVER;
+
+	if (req->cmd_flags & REQ_META)
+		flags |= UBLK_IO_F_META;
+
+	if (req->cmd_flags & REQ_INTEGRITY)
+		flags |= UBLK_IO_F_INTEGRITY;
+
+	if (req->cmd_flags & REQ_FUA)
+		flags |= UBLK_IO_F_FUA;
+
+	if (req->cmd_flags & REQ_PREFLUSH)
+		flags |= UBLK_IO_F_PREFLUSH;
+
+	if (req->cmd_flags & REQ_NOUNMAP)
+		flags |= UBLK_IO_F_NOUNMAP;
+
+	if (req->cmd_flags & REQ_SWAP)
+		flags |= UBLK_IO_F_SWAP;
+
+	return flags;
+}
+
+static int ublk_setup_iod(struct ublk_queue *ubq, struct request *req)
+{
+	struct ublksrv_io_desc *iod = ublk_get_iod(ubq, req->tag);
+	struct ublk_io *io = &ubq->ios[req->tag];
+	u32 ublk_op;
+
+	switch (req_op(req)) {
+	case REQ_OP_READ:
+		ublk_op = UBLK_IO_OP_READ;
+		break;
+	case REQ_OP_WRITE:
+		ublk_op = UBLK_IO_OP_WRITE;
+		break;
+	case REQ_OP_FLUSH:
+		ublk_op = UBLK_IO_OP_FLUSH;
+		break;
+	case REQ_OP_DISCARD:
+		ublk_op = UBLK_IO_OP_DISCARD;
+		break;
+	case REQ_OP_WRITE_ZEROES:
+		ublk_op = UBLK_IO_OP_WRITE_ZEROES;
+		break;
+	default:
+		return BLK_STS_IOERR;
+	}
+
+	/* need to translate since kernel may change */
+	iod->op_flags = ublk_op | ublk_req_build_flags(req);
+	iod->nr_sectors = blk_rq_sectors(req);
+	iod->start_sector = blk_rq_pos(req);
+	iod->addr = io->addr;
+
+	return BLK_STS_OK;
+}
+
+static blk_status_t ublk_queue_rq(struct blk_mq_hw_ctx *hctx,
+		const struct blk_mq_queue_data *bd)
+{
+	struct ublk_queue *ubq = hctx->driver_data;
+	struct request *rq = bd->rq;
+	struct ublk_rq_data *data = blk_mq_rq_to_pdu(rq);
+	enum task_work_notify_mode notify_mode = bd->last ?
+		TWA_SIGNAL_NO_IPI : TWA_NONE;
+	blk_status_t res;
+
+	/* fill iod to slot in io cmd buffer */
+	res = ublk_setup_iod(ubq, rq);
+	if (res != BLK_STS_OK)
+		return BLK_STS_IOERR;
+
+	blk_mq_start_request(bd->rq);
+
+	/*
+	 * run data copy in task work context for WRITE, and complete io_uring
+	 * cmd there too.
+	 *
+	 * This way should improve batching, meantime pinning pages in current
+	 * context is pretty fast.
+	 *
+	 * If we can't add the task work, something must be wrong, schedule
+	 * monitor work immediately.
+	 */
+	if (task_work_add(ubq->ubq_daemon, &data->work, notify_mode)) {
+		struct ublk_device *ub = rq->q->queuedata;
+
+		mod_delayed_work(system_wq, &ub->monitor_work, 0);
+		return BLK_STS_IOERR;
+	}
+
+	return BLK_STS_OK;
+}
+
+static bool ubq_daemon_is_dying(struct ublk_queue *ubq)
+{
+	return ubq->ubq_daemon->flags & PF_EXITING;
+}
+
+static void ublk_commit_rqs(struct blk_mq_hw_ctx *hctx)
+{
+	struct ublk_queue *ubq = hctx->driver_data;
+
+	__set_notify_signal(ubq->ubq_daemon);
+	if (ubq_daemon_is_dying(ubq)) {
+		struct ublk_device *ub = hctx->queue->queuedata;
+
+		mod_delayed_work(system_wq, &ub->monitor_work, 0);
+	}
+}
+
+/* todo: handle partial completion */
+static void ublk_complete_rq(struct request *req)
+{
+	struct ublk_queue *ubq = req->mq_hctx->driver_data;
+	struct ublk_io *io = &ubq->ios[req->tag];
+	unsigned int unmapped_bytes;
+
+	/* failed read IO if nothing is read */
+	if (!io->res && req_op(req) == REQ_OP_READ)
+		io->res = -EIO;
+
+	if (io->res < 0) {
+		blk_mq_end_request(req, errno_to_blk_status(io->res));
+		return;
+	}
+
+	/*
+	 * FLUSH or DISCARD usually won't return bytes returned, so end them
+	 * directly.
+	 *
+	 * Both the two needn't unmap.
+	 */
+	if (req_op(req) != REQ_OP_READ && req_op(req) != REQ_OP_WRITE) {
+		blk_mq_end_request(req, BLK_STS_OK);
+		return;
+	}
+
+	/* for READ request, writing data in iod->addr to rq buffers */
+	unmapped_bytes = ublk_unmap_io(ubq, req, io);
+
+	/*
+	 * Extremely impossible since we got data filled in just before
+	 *
+	 * Re-read simply for this unlikely case.
+	 */
+	if (unlikely(unmapped_bytes < io->res))
+		io->res = unmapped_bytes;
+
+	if (blk_update_request(req, BLK_STS_OK, io->res))
+		blk_mq_requeue_request(req, true);
+	else
+		__blk_mq_end_request(req, BLK_STS_OK);
+}
+
+/*
+ * __ublk_fail_req() may be called from abort context or ->ubq_daemon
+ * context during exiting, so lock is required.
+ *
+ * Also aborting may not be started yet, keep in mind that one failed
+ * request may be issued by block layer again.
+ */
+static void __ublk_fail_req(struct ublk_io *io, struct request *req)
+{
+	WARN_ON_ONCE(io->flags & UBLK_IO_FLAG_ACTIVE);
+
+	if (!(io->flags & UBLK_IO_FLAG_ABORTED)) {
+		io->flags |= UBLK_IO_FLAG_ABORTED;
+		blk_mq_end_request(req, BLK_STS_IOERR);
+	}
+}
+
+#define UBLK_REQUEUE_DELAY_MS	3
+
+static void ublk_rq_task_work_fn(struct callback_head *work)
+{
+	bool task_exiting = !!(current->flags & PF_EXITING);
+	struct ublk_rq_data *data = container_of(work,
+			struct ublk_rq_data, work);
+	struct request *req = blk_mq_rq_from_pdu(data);
+	struct ublk_queue *ubq = req->mq_hctx->driver_data;
+	struct ublk_io *io = &ubq->ios[req->tag];
+	struct ublk_device *ub = req->q->queuedata;
+	int ret;
+
+	pr_devel("%s: complete: op %d, qid %d tag %d io_flags %x addr %llx\n",
+			__func__, io->cmd->cmd_op, ubq->q_id, req->tag, io->flags,
+			ublk_get_iod(ubq, req->tag)->addr);
+
+	/*
+	 * If task is exiting, we may be run from exit_task_work() in
+	 * do_exit(), and may race with ublk_abort_queue(), so lock is
+	 * needed.
+	 */
+	if (unlikely(task_exiting)) {
+		ret = -ESRCH;
+		spin_lock(&ubq->abort_lock);
+	} else {
+		unsigned int mapped_bytes = ublk_map_io(ubq, req, io);
+
+		/*
+		 * Nothing mapped, retry until we succeed.
+		 *
+		 * We may never succeed in mapping any bytes here because of
+		 * OOM. TODO: reserve one buffer with single page pinned for
+		 * providing forward progress guarantee.
+		 */
+		if (unlikely(!mapped_bytes)) {
+			blk_mq_requeue_request(req, false);
+			blk_mq_delay_kick_requeue_list(req->q,
+					UBLK_REQUEUE_DELAY_MS);
+			return;
+		}
+
+		/* partially mapped, update io descriptor */
+		if (unlikely(mapped_bytes != blk_rq_bytes(req)))
+			ublk_get_iod(ubq, req->tag)->nr_sectors =
+				mapped_bytes >> 9;
+		ret = 0;
+	}
+
+	/*
+	 * Request is re-issued after this io command is aborted, we still
+	 * need to fail it immediately
+         */
+	if (unlikely(io->flags & UBLK_IO_FLAG_ABORTED)) {
+		blk_mq_end_request(req, BLK_STS_IOERR);
+		if (task_exiting)
+			spin_unlock(&ubq->abort_lock);
+		return;
+	}
+
+	/* mark this cmd owned by ublksrv */
+	io->flags |= UBLK_IO_FLAG_OWNED_BY_SRV;
+
+	/*
+	 * clear ACTIVE since we are done with this sqe/cmd slot
+	 * We can only accept io cmd in case of being not active.
+	 */
+	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
+
+	/* tell ublksrv one io request is coming */
+	io_uring_cmd_done(io->cmd, ret, 0);
+
+	/*
+	 * in case task is exiting, our partner has gone, so schedule monitor
+	 * work immediately for aborting queue
+	 */
+	if (task_exiting) {
+		spin_unlock(&ubq->abort_lock);
+		mod_delayed_work(system_wq, &ub->monitor_work, 0);
+	}
+}
+
+static int ublk_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
+		unsigned int hctx_idx)
+{
+	struct ublk_device *ub = hctx->queue->queuedata;
+	struct ublk_queue *ubq = ublk_get_queue(ub, hctx->queue_num);
+
+	hctx->driver_data = ubq;
+	return 0;
+}
+
+static int ublk_init_rq(struct blk_mq_tag_set *set, struct request *req,
+		unsigned int hctx_idx, unsigned int numa_node)
+{
+	struct ublk_rq_data *data = blk_mq_rq_to_pdu(req);
+
+	init_task_work(&data->work, ublk_rq_task_work_fn);
+
+	return 0;
+}
+
+static const struct blk_mq_ops ublk_mq_ops = {
+	.queue_rq       = ublk_queue_rq,
+	.commit_rqs     = ublk_commit_rqs,
+	.init_hctx	= ublk_init_hctx,
+	.init_request	= ublk_init_rq,
+};
+
+static int ublk_ch_open(struct inode *inode, struct file *filp)
+{
+	struct ublk_device *ub = container_of(inode->i_cdev,
+			struct ublk_device, cdev);
+
+	if (atomic_cmpxchg(&ub->ch_open_cnt, 0, 1) == 0) {
+		filp->private_data = ub;
+		return 0;
+	}
+	return -EBUSY;
+}
+
+static int ublk_ch_release(struct inode *inode, struct file *filp)
+{
+	struct ublk_device *ub = filp->private_data;
+
+	while (atomic_cmpxchg(&ub->ch_open_cnt, 1, 0) != 1)
+		cpu_relax();
+
+	filp->private_data = NULL;
+	return 0;
+}
+
+/* map pre-allocated per-queue cmd buffer to ublksrv daemon */
+static int ublk_ch_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct ublk_device *ub = filp->private_data;
+	size_t sz = vma->vm_end - vma->vm_start;
+	unsigned max_sz = UBLK_MAX_QUEUE_DEPTH * sizeof(struct ublksrv_io_desc);
+	unsigned long pfn, end, phys_off = vma->vm_pgoff << PAGE_SHIFT;
+	int q_id, ret = 0;
+
+	mutex_lock(&ub->mutex);
+	if (!ub->mm)
+		ub->mm = current->mm;
+	if (current->mm != ub->mm)
+		ret = -EINVAL;
+	mutex_unlock(&ub->mutex);
+
+	if (ret)
+		return ret;
+
+	if (vma->vm_flags & VM_WRITE)
+		return -EPERM;
+
+	end = UBLKSRV_CMD_BUF_OFFSET + ub->dev_info.nr_hw_queues * max_sz;
+	if (phys_off < UBLKSRV_CMD_BUF_OFFSET || phys_off >= end)
+		return -EINVAL;
+
+	q_id = (phys_off - UBLKSRV_CMD_BUF_OFFSET) / max_sz;
+	pr_devel("%s: qid %d, pid %d, addr %lx pg_off %lx sz %lu\n",
+			__func__, q_id, current->pid, vma->vm_start,
+			phys_off, (unsigned long)sz);
+
+	if (sz != ublk_queue_cmd_buf_size(ub, q_id))
+		return -EINVAL;
+
+	pfn = virt_to_phys(ublk_queue_cmd_buf(ub, q_id)) >> PAGE_SHIFT;
+	return remap_pfn_range(vma, vma->vm_start, pfn, sz, vma->vm_page_prot);
+}
+
+static void ublk_commit_completion(struct ublk_device *ub,
+		struct ublksrv_io_cmd *ub_cmd)
+{
+	u32 qid = ub_cmd->q_id, tag = ub_cmd->tag;
+	struct ublk_queue *ubq = ublk_get_queue(ub, qid);
+	struct ublk_io *io = &ubq->ios[tag];
+	struct request *req;
+
+	/* now this cmd slot is owned by nbd driver */
+	io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+	io->res = ub_cmd->result;
+
+	/* find the io request and complete */
+	req = blk_mq_tag_to_rq(ub->tag_set.tags[qid], tag);
+
+	if (req && likely(!blk_should_fake_timeout(req->q)))
+		ublk_complete_rq(req);
+}
+
+/*
+ * Focus on aborting any in-flight request scheduled to run via task work
+ */
+static void __ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
+{
+	bool task_exiting = !!(ubq->ubq_daemon->flags & PF_EXITING);
+	int i;
+
+	if (!task_exiting)
+		goto out;
+
+	for (i = 0; i < ubq->q_depth; i++) {
+		struct ublk_io *io = &ubq->ios[i];
+
+		if (!(io->flags & UBLK_IO_FLAG_ACTIVE)) {
+			struct request *rq;
+
+			/*
+			 * Either we fail the request or ublk_rq_task_work_fn
+			 * will do it
+			 */
+			rq = blk_mq_tag_to_rq(ub->tag_set.tags[ubq->q_id], i);
+			if (rq)
+				__ublk_fail_req(io, rq);
+		}
+	}
+ out:
+	ubq->abort_work_pending = false;
+	ublk_put_device(ub);
+}
+
+static void ublk_queue_task_work_fn(struct callback_head *work)
+{
+	struct ublk_queue *ubq = container_of(work, struct ublk_queue,
+			abort_work);
+
+	/*
+	 * Lock is only required in case of exiting ubq_daemon, but harmless
+	 * to grab it for running from task work too
+	 *
+	 * We are serialized with ublk_rq_task_work_fn() strictly.
+	 */
+	spin_lock(&ubq->abort_lock);
+	__ublk_abort_queue(ubq->dev, ubq);
+	spin_unlock(&ubq->abort_lock);
+}
+
+
+static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
+{
+	bool put_dev;
+
+	if (!ublk_get_device(ub))
+		return;
+
+	spin_lock(&ubq->abort_lock);
+	if (!ubq->abort_work_pending) {
+		ubq->abort_work_pending = true;
+		put_dev = false;
+		if (task_work_add(ubq->ubq_daemon, &ubq->abort_work,
+					TWA_SIGNAL)) {
+			__ublk_abort_queue(ub, ubq);
+		}
+	} else {
+		put_dev = true;
+	}
+	spin_unlock(&ubq->abort_lock);
+
+	/*
+	 * can't put device with ->abort_lock held, otherwise UAF
+	 * is triggered
+	 */
+	if (put_dev)
+		ublk_put_device(ub);
+}
+
+static void ublk_cancel_queue(struct ublk_queue *ubq)
+{
+	int i;
+
+	for (i = 0; i < ubq->q_depth; i++) {
+		struct ublk_io *io = &ubq->ios[i];
+
+		if (io->flags & UBLK_IO_FLAG_ACTIVE)
+			io_uring_cmd_done(io->cmd, UBLK_IO_RES_ABORT, 0);
+	}
+}
+
+/* Cancel all pending commands, must be called after del_gendisk() returns */
+static void ublk_cancel_dev(struct ublk_device *ub)
+{
+	int i;
+
+	for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
+		ublk_cancel_queue(ublk_get_queue(ub, i));
+}
+
+static void ublk_stop_dev(struct ublk_device *ub)
+{
+	mutex_lock(&ub->mutex);
+	if (!disk_live(ub->ub_disk))
+		goto unlock;
+
+	del_gendisk(ub->ub_disk);
+	ub->dev_info.state = UBLK_S_DEV_DEAD;
+	ub->dev_info.ublksrv_pid = -1;
+	ublk_cancel_dev(ub);
+ unlock:
+	mutex_unlock(&ub->mutex);
+	cancel_delayed_work_sync(&ub->monitor_work);
+}
+
+static int ublk_ctrl_stop_dev(struct ublk_device *ub)
+{
+	ublk_stop_dev(ub);
+	cancel_work_sync(&ub->stop_work);
+	return 0;
+}
+
+static inline bool ublk_queue_ready(struct ublk_queue *ubq)
+{
+	return ubq->nr_io_ready == ubq->q_depth;
+}
+
+/* device can only be started after all IOs are ready */
+static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
+{
+	mutex_lock(&ub->mutex);
+	ubq->nr_io_ready++;
+	if (ublk_queue_ready(ubq)) {
+		ubq->ubq_daemon = current;
+		get_task_struct(ubq->ubq_daemon);
+		ub->nr_queues_ready++;
+	}
+	if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues)
+		complete_all(&ub->completion);
+	mutex_unlock(&ub->mutex);
+}
+
+static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	struct ublksrv_io_cmd *ub_cmd = (struct ublksrv_io_cmd *)cmd->cmd;
+	struct ublk_device *ub = cmd->file->private_data;
+	struct ublk_queue *ubq;
+	struct ublk_io *io;
+	u32 cmd_op = cmd->cmd_op;
+	unsigned tag = ub_cmd->tag;
+	int ret = -EINVAL;
+
+	pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
+			__func__, cmd->cmd_op, ub_cmd->q_id, tag,
+			ub_cmd->result);
+
+	if (!(issue_flags & IO_URING_F_SQE128))
+		goto out;
+
+	if (ub_cmd->q_id >= ub->dev_info.nr_hw_queues)
+		goto out;
+
+	ubq = ublk_get_queue(ub, ub_cmd->q_id);
+	if (!ubq || ub_cmd->q_id != ubq->q_id)
+		goto out;
+
+	if (ubq->ubq_daemon && ubq->ubq_daemon != current)
+		goto out;
+
+	if (tag >= ubq->q_depth)
+		goto out;
+
+	io = &ubq->ios[tag];
+
+	/* there is pending io cmd, something must be wrong */
+	if (io->flags & UBLK_IO_FLAG_ACTIVE) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	switch (cmd_op) {
+	case UBLK_IO_FETCH_REQ:
+		/* UBLK_IO_FETCH_REQ is only allowed before queue is setup */
+		if (ublk_queue_ready(ubq)) {
+			ret = -EBUSY;
+			goto out;
+		}
+		/*
+		 * The io is being handled by server, so COMMIT_RQ is expected
+		 * instead of FETCH_REQ
+		 */
+		if (io->flags & UBLK_IO_FLAG_OWNED_BY_SRV)
+			goto out;
+		/* FETCH_RQ has to provide IO buffer */
+		if (!ub_cmd->addr)
+			goto out;
+		io->cmd = cmd;
+		io->flags |= UBLK_IO_FLAG_ACTIVE;
+		io->addr = ub_cmd->addr;
+
+		ublk_mark_io_ready(ub, ubq);
+		break;
+	case UBLK_IO_COMMIT_AND_FETCH_REQ:
+		/* FETCH_RQ has to provide IO buffer */
+		if (!ub_cmd->addr)
+			goto out;
+		if (!(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV))
+			goto out;
+		io->addr = ub_cmd->addr;
+		io->flags |= UBLK_IO_FLAG_ACTIVE;
+		io->cmd = cmd;
+		ublk_commit_completion(ub, ub_cmd);
+		break;
+	default:
+		goto out;
+	}
+	return -EIOCBQUEUED;
+
+ out:
+	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
+	io_uring_cmd_done(cmd, ret, 0);
+	pr_devel("%s: complete: cmd op %d, tag %d ret %x io_flags %x\n",
+			__func__, cmd_op, tag, ret, io->flags);
+	return -EIOCBQUEUED;
+}
+
+static const struct file_operations ublk_ch_fops = {
+	.owner = THIS_MODULE,
+	.open = ublk_ch_open,
+	.release = ublk_ch_release,
+	.llseek = no_llseek,
+	.uring_cmd = ublk_ch_uring_cmd,
+	.mmap = ublk_ch_mmap,
+};
+
+static void ublk_deinit_queue(struct ublk_device *ub, int q_id)
+{
+	int size = ublk_queue_cmd_buf_size(ub, q_id);
+	struct ublk_queue *ubq = ublk_get_queue(ub, q_id);
+
+	if (ubq->ubq_daemon)
+		put_task_struct(ubq->ubq_daemon);
+	if (ubq->io_cmd_buf)
+		free_pages((unsigned long)ubq->io_cmd_buf, get_order(size));
+}
+
+static int ublk_init_queue(struct ublk_device *ub, int q_id)
+{
+	struct ublk_queue *ubq = ublk_get_queue(ub, q_id);
+	gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO;
+	void *ptr;
+	int size;
+
+	ubq->q_id = q_id;
+	ubq->q_depth = ub->dev_info.queue_depth;
+	size = ublk_queue_cmd_buf_size(ub, q_id);
+
+	ptr = (void *) __get_free_pages(gfp_flags, get_order(size));
+	if (!ptr)
+		return -ENOMEM;
+
+	init_task_work(&ubq->abort_work, ublk_queue_task_work_fn);
+	ubq->io_cmd_buf = ptr;
+	ubq->dev = ub;
+	spin_lock_init(&ubq->abort_lock);
+	return 0;
+}
+
+static void ublk_deinit_queues(struct ublk_device *ub)
+{
+	int nr_queues = ub->dev_info.nr_hw_queues;
+	int i;
+
+	if (!ub->__queues)
+		return;
+
+	for (i = 0; i < nr_queues; i++)
+		ublk_deinit_queue(ub, i);
+	kfree(ub->__queues);
+}
+
+static int ublk_init_queues(struct ublk_device *ub)
+{
+	int nr_queues = ub->dev_info.nr_hw_queues;
+	int depth = ub->dev_info.queue_depth;
+	int ubq_size = sizeof(struct ublk_queue) + depth * sizeof(struct ublk_io);
+	int i, ret = -ENOMEM;
+
+	ub->queue_size = ubq_size;
+	ub->__queues = kcalloc(nr_queues, ubq_size, GFP_KERNEL);
+	if (!ub->__queues)
+		return ret;
+
+	for (i = 0; i < nr_queues; i++) {
+		if (ublk_init_queue(ub, i))
+			goto fail;
+	}
+
+	init_completion(&ub->completion);
+	return 0;
+
+ fail:
+	ublk_deinit_queues(ub);
+	return ret;
+}
+
+static int __ublk_alloc_dev_number(struct ublk_device *ub, int idx)
+{
+	int i = idx;
+	int err;
+
+	spin_lock(&ublk_idr_lock);
+	/* allocate id, if @id >= 0, we're requesting that specific id */
+	if (i >= 0) {
+		err = idr_alloc(&ublk_index_idr, ub, i, i + 1, GFP_NOWAIT);
+		if (err == -ENOSPC)
+			err = -EEXIST;
+	} else {
+		err = idr_alloc(&ublk_index_idr, ub, 0, 0, GFP_NOWAIT);
+	}
+	spin_unlock(&ublk_idr_lock);
+
+	if (err >= 0)
+		ub->ub_number = err;
+
+	return err;
+}
+
+static struct ublk_device *__ublk_create_dev(int idx)
+{
+	struct ublk_device *ub = NULL;
+	int ret;
+
+	ub = kzalloc(sizeof(*ub), GFP_KERNEL);
+	if (!ub)
+		return ERR_PTR(-ENOMEM);
+
+	ret = __ublk_alloc_dev_number(ub, idx);
+	if (ret < 0) {
+		kfree(ub);
+		return ERR_PTR(ret);
+	}
+	return ub;
+}
+
+static void __ublk_destroy_dev(struct ublk_device *ub)
+{
+	spin_lock(&ublk_idr_lock);
+	idr_remove(&ublk_index_idr, ub->ub_number);
+	wake_up_all(&ublk_idr_wq);
+	spin_unlock(&ublk_idr_lock);
+
+	mutex_destroy(&ub->mutex);
+
+	kfree(ub);
+}
+
+static void ublk_cdev_rel(struct device *dev)
+{
+	struct ublk_device *ub = container_of(dev, struct ublk_device, cdev_dev);
+
+	put_disk(ub->ub_disk);
+
+	blk_mq_free_tag_set(&ub->tag_set);
+
+	ublk_deinit_queues(ub);
+
+	__ublk_destroy_dev(ub);
+}
+
+static int ublk_add_chdev(struct ublk_device *ub)
+{
+	struct device *dev = &ub->cdev_dev;
+	int minor = ub->ub_number;
+	int ret;
+
+	dev->parent = ublk_misc.this_device;
+	dev->devt = MKDEV(MAJOR(ublk_chr_devt), minor);
+	dev->class = ublk_chr_class;
+	dev->release = ublk_cdev_rel;
+	device_initialize(dev);
+
+	ret = dev_set_name(dev, "ublkc%d", minor);
+	if (ret)
+		goto fail;
+
+	cdev_init(&ub->cdev, &ublk_ch_fops);
+	ret = cdev_device_add(&ub->cdev, dev);
+	if (ret)
+		goto fail;
+	return 0;
+ fail:
+	put_device(dev);
+	return ret;
+}
+
+static void ublk_stop_work_fn(struct work_struct *work)
+{
+	struct ublk_device *ub =
+		container_of(work, struct ublk_device, stop_work);
+
+	ublk_stop_dev(ub);
+}
+
+static void ublk_daemon_monitor_work(struct work_struct *work)
+{
+	struct ublk_device *ub =
+		container_of(work, struct ublk_device, monitor_work.work);
+	int i;
+
+	for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
+		struct ublk_queue *ubq = ublk_get_queue(ub, i);
+
+		if (ubq_daemon_is_dying(ubq)) {
+			schedule_work(&ub->stop_work);
+
+			/* abort queue is for making forward progress */
+			ublk_abort_queue(ub, ubq);
+		}
+	}
+
+	/*
+	 * We can't schedule monitor work after ublk_remove() is started.
+	 *
+	 * No need ub->mutex, monitor work are canceled after state is marked
+	 * as DEAD, so DEAD state is observed reliably.
+	 */
+	if (ub->dev_info.state != UBLK_S_DEV_DEAD)
+		schedule_delayed_work(&ub->monitor_work,
+				UBLK_DAEMON_MONITOR_PERIOD);
+}
+
+static void ublk_update_capacity(struct ublk_device *ub)
+{
+	unsigned int max_rq_bytes;
+
+	/* make max request buffer size aligned with PAGE_SIZE */
+	max_rq_bytes = round_down(ub->dev_info.rq_max_blocks <<
+			ub->bs_shift, PAGE_SIZE);
+	ub->dev_info.rq_max_blocks = max_rq_bytes >> ub->bs_shift;
+
+	set_capacity(ub->ub_disk, ub->dev_info.dev_blocks << (ub->bs_shift - 9));
+}
+
+/* add disk & cdev, cleanup everything in case of failure */
+static int ublk_add_dev(struct ublk_device *ub)
+{
+	struct gendisk *disk;
+	int err = -ENOMEM;
+	int bsize;
+
+	/* We are not ready to support zero copy */
+	ub->dev_info.flags[0] &= ~UBLK_F_SUPPORT_ZERO_COPY;
+
+	bsize = ub->dev_info.block_size;
+	ub->bs_shift = ilog2(bsize);
+
+	ub->dev_info.nr_hw_queues = min_t(unsigned int,
+			ub->dev_info.nr_hw_queues, nr_cpu_ids);
+
+	INIT_WORK(&ub->stop_work, ublk_stop_work_fn);
+	INIT_DELAYED_WORK(&ub->monitor_work, ublk_daemon_monitor_work);
+
+	if (ublk_init_queues(ub))
+		goto out_destroy_dev;
+
+	ub->tag_set.ops = &ublk_mq_ops;
+	ub->tag_set.nr_hw_queues = ub->dev_info.nr_hw_queues;
+	ub->tag_set.queue_depth = ub->dev_info.queue_depth;
+	ub->tag_set.numa_node = NUMA_NO_NODE;
+	ub->tag_set.cmd_size = sizeof(struct ublk_rq_data);
+	ub->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+	ub->tag_set.driver_data = ub;
+
+	err = blk_mq_alloc_tag_set(&ub->tag_set);
+	if (err)
+		goto out_deinit_queues;
+
+	disk = ub->ub_disk = blk_mq_alloc_disk(&ub->tag_set, ub);
+	if (IS_ERR(disk)) {
+		err = PTR_ERR(disk);
+		goto out_cleanup_tags;
+	}
+	ub->ub_queue = ub->ub_disk->queue;
+
+	ub->ub_queue->queuedata = ub;
+
+	blk_queue_logical_block_size(ub->ub_queue, bsize);
+	blk_queue_physical_block_size(ub->ub_queue, bsize);
+	blk_queue_io_min(ub->ub_queue, bsize);
+
+	blk_queue_max_hw_sectors(ub->ub_queue, ub->dev_info.rq_max_blocks <<
+			(ub->bs_shift - 9));
+
+	ub->ub_queue->limits.discard_granularity = PAGE_SIZE;
+
+	blk_queue_max_discard_sectors(ub->ub_queue, UINT_MAX >> 9);
+	blk_queue_max_write_zeroes_sectors(ub->ub_queue, UINT_MAX >> 9);
+
+	ublk_update_capacity(ub);
+
+	disk->fops		= &ub_fops;
+	disk->private_data	= ub;
+	disk->queue		= ub->ub_queue;
+	sprintf(disk->disk_name, "ublkb%d", ub->ub_number);
+
+	mutex_init(&ub->mutex);
+
+	/* add char dev so that ublksrv daemon can be setup */
+	err = ublk_add_chdev(ub);
+	if (err)
+		return err;
+
+	/* don't expose disk now until we got start command from cdev */
+
+	return 0;
+
+out_cleanup_tags:
+	blk_mq_free_tag_set(&ub->tag_set);
+out_deinit_queues:
+	ublk_deinit_queues(ub);
+out_destroy_dev:
+	__ublk_destroy_dev(ub);
+	return err;
+}
+
+static void ublk_remove(struct ublk_device *ub)
+{
+	ublk_ctrl_stop_dev(ub);
+
+	cdev_device_del(&ub->cdev, &ub->cdev_dev);
+	put_device(&ub->cdev_dev);
+}
+
+static struct ublk_device *ublk_get_device_from_id(int idx)
+{
+	struct ublk_device *ub = NULL;
+
+	if (idx < 0)
+		return NULL;
+
+	spin_lock(&ublk_idr_lock);
+	ub = idr_find(&ublk_index_idr, idx);
+	if (ub)
+		ub = ublk_get_device(ub);
+	spin_unlock(&ublk_idr_lock);
+
+	return ub;
+}
+
+static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd)
+{
+	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
+	int ret = -EINVAL;
+	int ublksrv_pid = (int)header->data[0];
+	unsigned long dev_blocks = header->data[1];
+
+	if (ublksrv_pid <= 0)
+		return ret;
+
+	wait_for_completion_interruptible(&ub->completion);
+
+	schedule_delayed_work(&ub->monitor_work, UBLK_DAEMON_MONITOR_PERIOD);
+
+	mutex_lock(&ub->mutex);
+	if (!disk_live(ub->ub_disk)) {
+		/* We may get disk size updated */
+		if (dev_blocks) {
+			ub->dev_info.dev_blocks = dev_blocks;
+			ublk_update_capacity(ub);
+		}
+		ub->dev_info.ublksrv_pid = ublksrv_pid;
+		ret = add_disk(ub->ub_disk);
+		if (!ret)
+			ub->dev_info.state = UBLK_S_DEV_LIVE;
+	} else {
+		ret = -EEXIST;
+	}
+	mutex_unlock(&ub->mutex);
+
+	return ret;
+}
+
+static struct blk_mq_hw_ctx *ublk_get_hw_queue(struct ublk_device *ub,
+		unsigned int index)
+{
+	struct blk_mq_hw_ctx *hctx;
+	unsigned long i;
+
+	queue_for_each_hw_ctx(ub->ub_queue, hctx, i)
+		if (hctx->queue_num == index)
+			return hctx;
+	return NULL;
+}
+
+static int ublk_ctrl_get_queue_affinity(struct io_uring_cmd *cmd)
+{
+	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
+	void __user *argp = (void __user *)(unsigned long)header->addr;
+	struct blk_mq_hw_ctx *hctx;
+	struct ublk_device *ub;
+	unsigned long queue;
+	unsigned int retlen;
+	int ret;
+
+	ub = ublk_get_device_from_id(header->dev_id);
+	if (!ub)
+		goto out;
+
+	ret = -EINVAL;
+	queue = header->data[0];
+	if (queue >= ub->dev_info.nr_hw_queues)
+		goto out;
+	hctx = ublk_get_hw_queue(ub, queue);
+	if (!hctx)
+		goto out;
+
+	retlen = min_t(unsigned short, header->len, cpumask_size());
+	if (copy_to_user(argp, hctx->cpumask, retlen)) {
+		ret = -EFAULT;
+		goto out;
+	}
+	if (retlen != header->len) {
+		if (clear_user(argp + retlen, header->len - retlen)) {
+			ret = -EFAULT;
+			goto out;
+		}
+	}
+	ret = 0;
+ out:
+	if (ub)
+		ublk_put_device(ub);
+	return ret;
+}
+
+static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_dev_info *info,
+		void __user *argp, int idx)
+{
+	struct ublk_device *ub;
+	int ret;
+
+	ret = mutex_lock_killable(&ublk_ctl_mutex);
+	if (ret)
+		return ret;
+
+	ub = __ublk_create_dev(idx);
+	if (!IS_ERR_OR_NULL(ub)) {
+		memcpy(&ub->dev_info, info, sizeof(*info));
+
+		/* update device id */
+		ub->dev_info.dev_id = ub->ub_number;
+
+		ret = ublk_add_dev(ub);
+		if (!ret) {
+			if (copy_to_user(argp, &ub->dev_info, sizeof(*info))) {
+				ublk_remove(ub);
+				ret = -EFAULT;
+			}
+		}
+	} else {
+		if (IS_ERR(ub))
+			ret = PTR_ERR(ub);
+		else
+			ret = -ENOMEM;
+	}
+	mutex_unlock(&ublk_ctl_mutex);
+
+	return ret;
+}
+
+static inline bool ublk_idr_freed(int id)
+{
+	void *ptr;
+
+	spin_lock(&ublk_idr_lock);
+	ptr = idr_find(&ublk_index_idr, id);
+	spin_unlock(&ublk_idr_lock);
+
+	return ptr == NULL;
+}
+
+static int ublk_ctrl_del_dev(int idx)
+{
+	struct ublk_device *ub;
+	int ret;
+
+	ret = mutex_lock_killable(&ublk_ctl_mutex);
+	if (ret)
+		return ret;
+
+	ub = ublk_get_device_from_id(idx);
+	if (ub) {
+		ublk_remove(ub);
+		ublk_put_device(ub);
+		ret = 0;
+	} else {
+		ret = -ENODEV;
+	}
+
+	/*
+	 * Wait until the idr is removed, then it can be reused after
+	 * DEL_DEV command is returned.
+	 */
+	if (!ret)
+		wait_event(ublk_idr_wq, ublk_idr_freed(idx));
+	mutex_unlock(&ublk_ctl_mutex);
+
+	return ret;
+}
+
+
+static inline void ublk_dump_dev_info(struct ublksrv_ctrl_dev_info *info)
+{
+	pr_devel("%s: dev id %d flags %llx\n", __func__,
+			info->dev_id, info->flags[0]);
+	pr_devel("\t nr_hw_queues %d queue_depth %d block size %d dev_capacity %lld\n",
+			info->nr_hw_queues, info->queue_depth,
+			info->block_size, info->dev_blocks);
+}
+
+static inline void ublk_ctrl_cmd_dump(struct io_uring_cmd *cmd)
+{
+	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
+
+	pr_devel("%s: cmd_op %x, dev id %d qid %d data %llx buf %llx len %u\n",
+			__func__, cmd->cmd_op, header->dev_id, header->queue_id,
+			header->data[0], header->addr, header->len);
+}
+
+static int ublk_ctrl_cmd_validate(struct io_uring_cmd *cmd,
+		struct ublksrv_ctrl_dev_info *info)
+{
+	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
+	u32 cmd_op = cmd->cmd_op;
+	void __user *argp = (void __user *)(unsigned long)header->addr;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	switch (cmd_op) {
+	case UBLK_CMD_GET_DEV_INFO:
+		if (header->len < sizeof(*info) || !header->addr)
+			return -EINVAL;
+		break;
+	case UBLK_CMD_ADD_DEV:
+		if (header->len < sizeof(*info) || !header->addr)
+			return -EINVAL;
+		if (copy_from_user(info, argp, sizeof(*info)) != 0)
+			return -EFAULT;
+		ublk_dump_dev_info(info);
+		if (header->dev_id != info->dev_id) {
+			printk(KERN_WARNING "%s: cmd %x, dev id not match %u %u\n",
+					__func__, cmd_op, header->dev_id,
+					info->dev_id);
+			return -EINVAL;
+		}
+		if (header->queue_id != (u16)-1) {
+			printk(KERN_WARNING "%s: cmd %x queue_id is wrong %x\n",
+					__func__, cmd_op, header->queue_id);
+			return -EINVAL;
+		}
+		break;
+	case UBLK_CMD_GET_QUEUE_AFFINITY:
+		if ((header->len * BITS_PER_BYTE) < nr_cpu_ids)
+			return -EINVAL;
+		if (header->len & (sizeof(unsigned long)-1))
+			return -EINVAL;
+		if (!header->addr)
+			return -EINVAL;
+	};
+
+	return 0;
+}
+
+static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
+		unsigned int issue_flags)
+{
+	struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd;
+	void __user *argp = (void __user *)(unsigned long)header->addr;
+	struct ublksrv_ctrl_dev_info info;
+	u32 cmd_op = cmd->cmd_op;
+	struct ublk_device *ub;
+	int ret = -EINVAL;
+
+	ublk_ctrl_cmd_dump(cmd);
+
+	if (!(issue_flags & IO_URING_F_SQE128))
+		goto out;
+
+	ret = ublk_ctrl_cmd_validate(cmd, &info);
+	if (ret)
+		goto out;
+
+	ret = -ENODEV;
+	switch (cmd_op) {
+	case UBLK_CMD_START_DEV:
+		ub = ublk_get_device_from_id(header->dev_id);
+		if (ub) {
+			ret = ublk_ctrl_start_dev(ub, cmd);
+			ublk_put_device(ub);
+		}
+		break;
+	case UBLK_CMD_STOP_DEV:
+		ub = ublk_get_device_from_id(header->dev_id);
+		if (ub) {
+			ret = ublk_ctrl_stop_dev(ub);
+			ublk_put_device(ub);
+		}
+		break;
+	case UBLK_CMD_GET_DEV_INFO:
+		ub = ublk_get_device_from_id(header->dev_id);
+		if (ub) {
+			if (copy_to_user(argp, &ub->dev_info, sizeof(info)))
+				ret = -EFAULT;
+			else
+				ret = 0;
+			ublk_put_device(ub);
+		}
+		break;
+	case UBLK_CMD_ADD_DEV:
+		ret = ublk_ctrl_add_dev(&info, argp, header->dev_id);
+		break;
+	case UBLK_CMD_DEL_DEV:
+		ret = ublk_ctrl_del_dev(header->dev_id);
+		break;
+	case UBLK_CMD_GET_QUEUE_AFFINITY:
+		ret = ublk_ctrl_get_queue_affinity(cmd);
+		break;
+	default:
+		break;
+	};
+ out:
+	io_uring_cmd_done(cmd, ret, 0);
+	pr_devel("%s: cmd done ret %d cmd_op %x, dev id %d qid %d\n",
+			__func__, ret, cmd->cmd_op, header->dev_id, header->queue_id);
+	return -EIOCBQUEUED;
+}
+
+static const struct file_operations ublk_ctl_fops = {
+	.open		= nonseekable_open,
+	.uring_cmd      = ublk_ctrl_uring_cmd,
+	.owner		= THIS_MODULE,
+	.llseek		= noop_llseek,
+};
+
+static struct miscdevice ublk_misc = {
+	.minor		= MISC_DYNAMIC_MINOR,
+	.name		= "ublk-control",
+	.fops		= &ublk_ctl_fops,
+};
+
+static int __init ublk_init(void)
+{
+	int ret;
+
+	init_waitqueue_head(&ublk_idr_wq);
+
+	ret = misc_register(&ublk_misc);
+	if (ret)
+		return ret;
+
+	ret = alloc_chrdev_region(&ublk_chr_devt, 0, UBLK_MINORS, "ublk-char");
+	if (ret)
+		goto unregister_mis;
+
+	ublk_chr_class = class_create(THIS_MODULE, "ublk-char");
+	if (IS_ERR(ublk_chr_class)) {
+		ret = PTR_ERR(ublk_chr_class);
+		goto free_chrdev_region;
+	}
+	return 0;
+
+free_chrdev_region:
+	unregister_chrdev_region(ublk_chr_devt, UBLK_MINORS);
+unregister_mis:
+	misc_deregister(&ublk_misc);
+	return ret;
+}
+
+static void __exit ublk_exit(void)
+{
+	struct ublk_device *ub;
+	int id;
+
+	class_destroy(ublk_chr_class);
+
+	misc_deregister(&ublk_misc);
+
+	idr_for_each_entry(&ublk_index_idr, ub, id)
+		ublk_remove(ub);
+
+	idr_destroy(&ublk_index_idr);
+	unregister_chrdev_region(ublk_chr_devt, UBLK_MINORS);
+}
+
+module_init(ublk_init);
+module_exit(ublk_exit);
+
+MODULE_AUTHOR("Ming Lei <ming.lei@redhat.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
new file mode 100644
index 000000000000..4f0c16ec875e
--- /dev/null
+++ b/include/uapi/linux/ublk_cmd.h
@@ -0,0 +1,156 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef USER_BLK_DRV_CMD_INC_H
+#define USER_BLK_DRV_CMD_INC_H
+
+#include <linux/types.h>
+
+/* ublk server command definition */
+
+/*
+ * Admin commands, issued by ublk server, and handled by ublk driver.
+ */
+#define	UBLK_CMD_GET_QUEUE_AFFINITY	0x01
+#define	UBLK_CMD_GET_DEV_INFO	0x02
+#define	UBLK_CMD_ADD_DEV		0x04
+#define	UBLK_CMD_DEL_DEV		0x05
+#define	UBLK_CMD_START_DEV	0x06
+#define	UBLK_CMD_STOP_DEV	0x07
+
+/*
+ * IO commands, issued by ublk server, and handled by ublk driver.
+ *
+ * FETCH_REQ: issued via sqe(URING_CMD) beforehand for fetching IO request
+ *      from ublk driver, should be issued only when starting device. After
+ *      the associated cqe is returned, request's tag can be retrieved via
+ *      cqe->userdata.
+ *
+ * COMMIT_AND_FETCH_REQ: issued via sqe(URING_CMD) after ublkserver handled
+ *      this IO request, request's handling result is committed to ublk
+ *      driver, meantime FETCH_REQ is piggyback, and FETCH_REQ has to be
+ *      handled before completing io request.
+ */
+#define	UBLK_IO_FETCH_REQ		0x20
+#define	UBLK_IO_COMMIT_AND_FETCH_REQ	0x21
+
+/* only ABORT means that no re-fetch */
+#define UBLK_IO_RES_OK			0
+#define UBLK_IO_RES_ABORT		(-ENODEV)
+
+#define UBLKSRV_CMD_BUF_OFFSET	0
+#define UBLKSRV_IO_BUF_OFFSET	0x80000000
+
+/* tag bit is 12bit, so at most 4096 IOs for each queue */
+#define UBLK_MAX_QUEUE_DEPTH	4096
+
+/*
+ * zero copy requires 4k block size, and can remap ublk driver's io
+ * request into ublksrv's vm space
+ */
+#define UBLK_F_SUPPORT_ZERO_COPY	(1UL << 0)
+
+/* device state */
+#define UBLK_S_DEV_DEAD	0
+#define UBLK_S_DEV_LIVE	1
+
+/* shipped via sqe->cmd of io_uring command */
+struct ublksrv_ctrl_cmd {
+	/* sent to which device, must be valid */
+	__u32	dev_id;
+
+	/* sent to which queue, must be -1 if the cmd isn't for queue */
+	__u16	queue_id;
+	/*
+	 * cmd specific buffer, can be IN or OUT.
+	 */
+	__u16	len;
+	__u64	addr;
+
+	/* inline data */
+	__u64	data[2];
+};
+
+struct ublksrv_ctrl_dev_info {
+	__u16	nr_hw_queues;
+	__u16	queue_depth;
+	__u16	block_size;
+	__u16	state;
+
+	__u32	rq_max_blocks;
+	__u32	dev_id;
+
+	__u64   dev_blocks;
+
+	__s32	ublksrv_pid;
+	__s32	reserved0;
+	__u64	flags[2];
+
+	/* For ublksrv internal use, invisible to ublk driver */
+	__u64	ublksrv_flags;
+	__u64	reserved1[9];
+};
+
+#define		UBLK_IO_OP_READ		0
+#define		UBLK_IO_OP_WRITE		1
+#define		UBLK_IO_OP_FLUSH		2
+#define		UBLK_IO_OP_DISCARD	3
+#define		UBLK_IO_OP_WRITE_SAME	4
+#define		UBLK_IO_OP_WRITE_ZEROES	5
+
+#define		UBLK_IO_F_FAILFAST_DEV		(1U << 8)
+#define		UBLK_IO_F_FAILFAST_TRANSPORT	(1U << 9)
+#define		UBLK_IO_F_FAILFAST_DRIVER	(1U << 10)
+#define		UBLK_IO_F_META			(1U << 11)
+#define		UBLK_IO_F_INTEGRITY		(1U << 12)
+#define		UBLK_IO_F_FUA			(1U << 13)
+#define		UBLK_IO_F_PREFLUSH		(1U << 14)
+#define		UBLK_IO_F_NOUNMAP		(1U << 15)
+#define		UBLK_IO_F_SWAP			(1U << 16)
+
+/*
+ * io cmd is described by this structure, and stored in share memory, indexed
+ * by request tag.
+ *
+ * The data is stored by ublk driver, and read by ublksrv after one fetch command
+ * returns.
+ */
+struct ublksrv_io_desc {
+	/* op: bit 0-7, flags: bit 8-31 */
+	__u32		op_flags;
+
+	__u32		nr_sectors;
+
+	/* start sector for this io */
+	__u64		start_sector;
+
+	/* buffer address in ublksrv daemon vm space, from ublk driver */
+	__u64		addr;
+};
+
+static inline __u8 ublksrv_get_op(const struct ublksrv_io_desc *iod)
+{
+	return iod->op_flags & 0xff;
+}
+
+static inline __u32 ublksrv_get_flags(const struct ublksrv_io_desc *iod)
+{
+	return iod->op_flags >> 8;
+}
+
+/* issued to ublk driver via /dev/ublkcN */
+struct ublksrv_io_cmd {
+	__u16	q_id;
+
+	/* for fetch/commit which result */
+	__u16	tag;
+
+	/* io result, it is valid for COMMIT* command only */
+	__s32	result;
+
+	/*
+	 * userspace buffer address in ublksrv daemon process, valid for
+	 * FETCH* command only
+	 */
+	__u64	addr;
+};
+
+#endif
-- 
2.31.1

[PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Ming Lei]

Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
ubq daemon context, so we can avoid to call task_work_add(), then
it is fine to build ublk driver as module.

In this way, iops is affected a bit, but just by ~5% on ublk/null,
given io_uring provides pretty good batching issuing & completing.

One thing to be careful is race between ->queue_rq() and handling
abort, which is avoided by quiescing queue when aborting queue.
Except for that, handling abort becomes much easier with
UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
anything done in ubq daemon kernel context.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/Kconfig         |   2 +-
 drivers/block/ublk_drv.c      | 121 ++++++++++++++++++++++++++--------
 include/uapi/linux/ublk_cmd.h |  17 +++++
 3 files changed, 113 insertions(+), 27 deletions(-)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index d218089cdbec..2ba77fd960c2 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -409,7 +409,7 @@ config BLK_DEV_RBD
 	  If unsure, say N.
 
 config BLK_DEV_UBLK
-	bool "Userspace block driver"
+	tristate "Userspace block driver"
 	select IO_URING
 	help
           io uring based userspace block driver.
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 0076418e6fad..98482f8d1a77 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -92,6 +92,7 @@ struct ublk_queue {
 	int q_id;
 	int q_depth;
 
+	unsigned long flags;
 	struct task_struct	*ubq_daemon;
 	char *io_cmd_buf;
 
@@ -141,6 +142,15 @@ struct ublk_device {
 	struct work_struct	stop_work;
 };
 
+#define ublk_use_task_work(ubq)						\
+({                                                                      \
+	bool ret = false;						\
+	if (IS_BUILTIN(CONFIG_BLK_DEV_UBLK) &&                          \
+			!((ubq)->flags & UBLK_F_NEED_REFETCH))		\
+		ret = true;						\
+	ret;								\
+})
+
 static dev_t ublk_chr_devt;
 static struct class *ublk_chr_class;
 
@@ -429,6 +439,26 @@ static int ublk_setup_iod(struct ublk_queue *ubq, struct request *req)
 	return BLK_STS_OK;
 }
 
+static bool ubq_daemon_is_dying(struct ublk_queue *ubq)
+{
+	return ubq->ubq_daemon->flags & PF_EXITING;
+}
+
+static void ubq_complete_io_cmd(struct ublk_io *io, int res)
+{
+	/* mark this cmd owned by ublksrv */
+	io->flags |= UBLK_IO_FLAG_OWNED_BY_SRV;
+
+	/*
+	 * clear ACTIVE since we are done with this sqe/cmd slot
+	 * We can only accept io cmd in case of being not active.
+	 */
+	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
+
+	/* tell ublksrv one io request is coming */
+	io_uring_cmd_done(io->cmd, res, 0);
+}
+
 static blk_status_t ublk_queue_rq(struct blk_mq_hw_ctx *hctx,
 		const struct blk_mq_queue_data *bd)
 {
@@ -456,30 +486,38 @@ static blk_status_t ublk_queue_rq(struct blk_mq_hw_ctx *hctx,
 	 * If we can't add the task work, something must be wrong, schedule
 	 * monitor work immediately.
 	 */
-	if (task_work_add(ubq->ubq_daemon, &data->work, notify_mode)) {
-		struct ublk_device *ub = rq->q->queuedata;
+	if (ublk_use_task_work(ubq)) {
+		if (task_work_add(ubq->ubq_daemon, &data->work, notify_mode)) {
+			struct ublk_device *ub = rq->q->queuedata;
 
-		mod_delayed_work(system_wq, &ub->monitor_work, 0);
-		return BLK_STS_IOERR;
+			mod_delayed_work(system_wq, &ub->monitor_work, 0);
+			return BLK_STS_IOERR;
+		}
+	} else {
+		if (ubq_daemon_is_dying(ubq)) {
+			struct ublk_device *ub = rq->q->queuedata;
+
+			mod_delayed_work(system_wq, &ub->monitor_work, 0);
+			return BLK_STS_IOERR;
+		}
+
+		ubq_complete_io_cmd(&ubq->ios[rq->tag], UBLK_IO_RES_REFETCH);
 	}
 
 	return BLK_STS_OK;
 }
 
-static bool ubq_daemon_is_dying(struct ublk_queue *ubq)
-{
-	return ubq->ubq_daemon->flags & PF_EXITING;
-}
-
 static void ublk_commit_rqs(struct blk_mq_hw_ctx *hctx)
 {
 	struct ublk_queue *ubq = hctx->driver_data;
 
-	__set_notify_signal(ubq->ubq_daemon);
-	if (ubq_daemon_is_dying(ubq)) {
-		struct ublk_device *ub = hctx->queue->queuedata;
+	if (ublk_use_task_work(ubq)) {
+		__set_notify_signal(ubq->ubq_daemon);
+		if (ubq_daemon_is_dying(ubq)) {
+			struct ublk_device *ub = hctx->queue->queuedata;
 
-		mod_delayed_work(system_wq, &ub->monitor_work, 0);
+			mod_delayed_work(system_wq, &ub->monitor_work, 0);
+		}
 	}
 }
 
@@ -604,17 +642,7 @@ static void ublk_rq_task_work_fn(struct callback_head *work)
 		return;
 	}
 
-	/* mark this cmd owned by ublksrv */
-	io->flags |= UBLK_IO_FLAG_OWNED_BY_SRV;
-
-	/*
-	 * clear ACTIVE since we are done with this sqe/cmd slot
-	 * We can only accept io cmd in case of being not active.
-	 */
-	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
-
-	/* tell ublksrv one io request is coming */
-	io_uring_cmd_done(io->cmd, ret, 0);
+	ubq_complete_io_cmd(io, ret);
 
 	/*
 	 * in case task is exiting, our partner has gone, so schedule monitor
@@ -737,13 +765,26 @@ static void ublk_commit_completion(struct ublk_device *ub,
  * Focus on aborting any in-flight request scheduled to run via task work
  */
 static void __ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
+	__releases(&ubq->abort_lock)
 {
 	bool task_exiting = !!(ubq->ubq_daemon->flags & PF_EXITING);
 	int i;
+	bool quiesced = false;
 
 	if (!task_exiting)
 		goto out;
 
+	/*
+	 * quiesce queue so that we can avoid to race with ublk_queue_rq()
+	 * wrt. dealing with io flags
+	 */
+	if (ubq->flags & UBLK_F_NEED_REFETCH) {
+		spin_unlock(&ubq->abort_lock);
+		blk_mq_quiesce_queue(ub->ub_queue);
+		spin_lock(&ubq->abort_lock);
+		quiesced = true;
+	}
+
 	for (i = 0; i < ubq->q_depth; i++) {
 		struct ublk_io *io = &ubq->ios[i];
 
@@ -759,6 +800,8 @@ static void __ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
 				__ublk_fail_req(io, rq);
 		}
 	}
+	if (quiesced)
+		blk_mq_unquiesce_queue(ub->ub_queue);
  out:
 	ubq->abort_work_pending = false;
 	ublk_put_device(ub);
@@ -792,8 +835,12 @@ static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq)
 	if (!ubq->abort_work_pending) {
 		ubq->abort_work_pending = true;
 		put_dev = false;
-		if (task_work_add(ubq->ubq_daemon, &ubq->abort_work,
-					TWA_SIGNAL)) {
+		if (ublk_use_task_work(ubq)) {
+			if (task_work_add(ubq->ubq_daemon,
+					  &ubq->abort_work, TWA_SIGNAL)) {
+				__ublk_abort_queue(ub, ubq);
+			}
+		} else {
 			__ublk_abort_queue(ub, ubq);
 		}
 	} else {
@@ -872,6 +919,16 @@ static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
 	mutex_unlock(&ub->mutex);
 }
 
+static void ublk_handle_refetch(struct ublk_device *ub,
+		struct ublk_queue *ubq, int tag)
+{
+	struct request *req = blk_mq_tag_to_rq(ub->tag_set.tags[ubq->q_id],
+			tag);
+	struct ublk_rq_data *data = blk_mq_rq_to_pdu(req);
+
+	ublk_rq_task_work_fn(&data->work);
+}
+
 static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	struct ublksrv_io_cmd *ub_cmd = (struct ublksrv_io_cmd *)cmd->cmd;
@@ -943,6 +1000,15 @@ static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 		io->cmd = cmd;
 		ublk_commit_completion(ub, ub_cmd);
 		break;
+	case UBLK_IO_REFETCH_REQ:
+		if (!(io->flags & UBLK_IO_FLAG_OWNED_BY_SRV))
+			goto out;
+		io->addr = ub_cmd->addr;
+		io->cmd = cmd;
+		io->flags |= UBLK_IO_FLAG_ACTIVE;
+		io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+		ublk_handle_refetch(ub, ubq, tag);
+		break;
 	default:
 		goto out;
 	}
@@ -983,6 +1049,7 @@ static int ublk_init_queue(struct ublk_device *ub, int q_id)
 	void *ptr;
 	int size;
 
+	ubq->flags = ub->dev_info.flags[0];
 	ubq->q_id = q_id;
 	ubq->q_depth = ub->dev_info.queue_depth;
 	size = ublk_queue_cmd_buf_size(ub, q_id);
@@ -1381,6 +1448,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_dev_info *info,
 
 		/* update device id */
 		ub->dev_info.dev_id = ub->ub_number;
+		if (IS_MODULE(CONFIG_BLK_DEV_UBLK))
+			ub->dev_info.flags[0] |= UBLK_F_NEED_REFETCH;
 
 		ret = ublk_add_dev(ub);
 		if (!ret) {
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index 4f0c16ec875e..fe4a2c7c8349 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -28,12 +28,19 @@
  *      this IO request, request's handling result is committed to ublk
  *      driver, meantime FETCH_REQ is piggyback, and FETCH_REQ has to be
  *      handled before completing io request.
+ *
+ * REFETCH_REQ: issued via sqe(URING_CMD) after ublk driver returns
+ * 	UBLK_IO_REFETCH_REQ, which only purpose is to fetch io request data
+ * 	from the userspace ublk server context in case that task_work_add
+ * 	isn't available because ublk driver is built as module
  */
 #define	UBLK_IO_FETCH_REQ		0x20
 #define	UBLK_IO_COMMIT_AND_FETCH_REQ	0x21
+#define	UBLK_IO_REFETCH_REQ		0x22
 
 /* only ABORT means that no re-fetch */
 #define UBLK_IO_RES_OK			0
+#define UBLK_IO_RES_REFETCH		1
 #define UBLK_IO_RES_ABORT		(-ENODEV)
 
 #define UBLKSRV_CMD_BUF_OFFSET	0
@@ -48,6 +55,16 @@
  */
 #define UBLK_F_SUPPORT_ZERO_COPY	(1UL << 0)
 
+/*
+ * When NEED_REFETCH is set, ublksrv has to issue UBLK_IO_REFETCH_REQ
+ * command after ublk driver returns UBLK_IO_REFETCH_REQ.
+ *
+ * This flag is negotiated during handling UBLK_CMD_ADD_DEV. If ublksrv
+ * sets it, ublk driver can't clear it. But if ublk driver sets it back
+ * to ublksrv, ublksrv has to handle it correctly.
+ */
+#define UBLK_F_NEED_REFETCH		(1UL << 1)
+
 /* device state */
 #define UBLK_S_DEV_DEAD	0
 #define UBLK_S_DEV_LIVE	1
-- 
2.31.1

Re: [PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Xiaoguang Wang]

Hello Ming,

First thanks for this great work, I think ublk will be a powerful
replacement for tcmu. I read your v3 ublk kernel and user-space
codes, and have some ublk design questions here:

1) As I said before, currently ublk still needs user-space backstore
to allocate io buffer in advance, for example, UBLK_IO_FETCH_REQ
command needs to set ublk_io->addr when starting device. Currently,
some of our internal business may create hundreds or thousands of
tcmu devices in one host, when switching to ublk, it'll need user-space
backstore to allocate lots of memory in advance, which will waste memory
in a host with swap off. Also user-space backstore may use advanced
network components, they may maintain internal memory pool, which
can be used as io buffer.

So I'd like to suggest that at least we add a new flag and keep GET_DATA
command. When used, FETCH command does not need to pass io
buffer addr, and backstore needs to send a GET_DATA command with
user addr for write request.

Support high flexibility, let's user decides what's best for them.

2) complicated ublk user-space
First forgive me  I think current ublk user-space codes looks somewhat
complicated:
  1. UBLK_CMD_START_DEV and io handler worker need to be
in different task context, because UBLK_CMD_START_DEV needs
to wait the number of queue depth of sqes to be submitted in advance.

  2. mixed getting ublk command and target io handle in one io_uring instance
I'm not sure it's a good design, see ublksrv_handle_cqe(), which contains
many flag handle work and is_target_io() check, I think the data flow is not
that clear for me at least

  3. helper like tcmulib_get_next_command()
I wonder whether current ublk user-space can offer similar helper which
will return a set of io commands to backstore easily.

I'd like to suggest:
1. When starting ublk dev, pass io_uring fd for every queue, then in
blk-mq->queue_rq(), it'll generate one cqe for every coming request,
not need to issue fetch sqes command in advance, kernel codes would
simplify a bit,  UBLK_IO_FLAG_ACTIVE may be discarded. And helper
like returning a set of io command would be added easily. Note these
io_uring fd would be just used for notifying io command generated.

2. We use another io_uring fd per queue to handle GET_DATA or
COMMIT_REQ command. Indeed, if we can support synchronous ioctl
interface to do GET_DATA and COMMIT_REQ, we may make libublk
really simple.


Here I'd like to describe how we use tcmu. A main thread call
tcmulib_get_next_command() to get a set of io commands, then
it dispatches them to user-space io wokers. Take write requests as
example, io worker use ioctl(2) to get data from bios, and send
data to distributed fs, finally call ioctl(2) to commit req. Multiple
io workers can run concurrently. Since GET_DATA(write request)
or COMMIT_REQ(read request) mainly do memcpy work, one
io_uring instance will just do these jobs sequentially, which may
not take advantage of multi-cpu.

So finally, I would suggest ublk or libulk just offer basic interface:
add/start/delete dev interface, get io commands sescriptor, get io
data, commit io helper. Let's user-space target make decisions,
for example, whether to use eventfd.

Thanks for your patience


Regards,
Xiaoguang Wang

> Hello Guys,
>
> ublk driver is one kernel driver for implementing generic userspace block
> device/driver, which delivers io request from ublk block device(/dev/ublkbN) into
> ublk server[1] which is the userspace part of ublk for communicating
> with ublk driver and handling specific io logic by its target module.
>
> Another thing ublk driver handles is to copy data between user space buffer
> and request/bio's pages, or take zero copy if mm is ready for support it in
> future. ublk driver doesn't handle any IO logic of the specific driver, so
> it is small/simple, and all io logics are done by the target code in ublkserver.
>
> The above two are main jobs done by ublk driver.
>
> ublk driver can help to move IO logic into userspace, in which the
> development work is easier/more effective than doing in kernel, such as,
> ublk-loop takes < 200 lines of loop specific code to get basically same 
> function with kernel loop block driver, meantime the performance is
> is even better than kernel loop with same setting. ublksrv[1] provide built-in
> test for comparing both by running "make test T=loop", for example, see
> the test result running on VM which is over my lattop(root disk is
> nvme/device mapper/xfs):
>
> 	[root@ktest-36 ubdsrv]#make -s -C /root/git/ubdsrv/tests run T=loop/001 R=10
> 	running loop/001
> 		fio (ublk/loop(/root/git/ubdsrv/tests/tmp/ublk_loop_VqbMA), libaio, bs 4k, dio, hw queues:1)...
> 		randwrite: jobs 1, iops 32572
> 		randread: jobs 1, iops 143052
> 		rw: jobs 1, iops read 29919 write 29964
> 	
> 	[root@ktest-36 ubdsrv]# make test T=loop/003
> 	make -s -C /root/git/ubdsrv/tests run T=loop/003 R=10
> 	running loop/003
> 		fio (kernel_loop/kloop(/root/git/ubdsrv/tests/tmp/ublk_loop_ZIVnG), libaio, bs 4k, dio, hw queues:1)...
> 		randwrite: jobs 1, iops 27436
> 		randread: jobs 1, iops 95273
> 		rw: jobs 1, iops read 22542 write 22543 
>
>
> Another example is high performance qcow2 support[2], which could be built with
> ublk framework more easily than doing it inside kernel.
>
> Also there are more people who express interests on userspace block driver[3],
> Gabriel Krisman Bertazi proposes this topic in lsf/mm/ebpf 2022 and mentioned
> requirement from Google. Ziyang Zhang from Alibaba said they "plan to
> replace TCMU by UBD as a new choice" because UBD can get better throughput than
> TCMU even with single queue[4], meantime UBD is simple. Also there is userspace
> storage service for providing storage to containers.
>
> It is io_uring based: io request is delivered to userspace via new added
> io_uring command which has been proved as very efficient for making nvme
> passthrough IO to get better IOPS than io_uring(READ/WRITE). Meantime one
> shared/mmap buffer is used for sharing io descriptor to userspace, the
> buffer is readonly for userspace, each IO just takes 24bytes so far.
> It is suggested to use io_uring in userspace(target part of ublk server)
> to handle IO request too. And it is still easy for ublkserver to support
> io handling by non-io_uring, and this work isn't done yet, but can be
> supported easily with help o eventfd.
>
> This way is efficient since no extra io command copy is required, no sleep
> is needed in transferring io command to userspace. Meantime the communication
> protocol is simple and efficient, one single command of
> UBD_IO_COMMIT_AND_FETCH_REQ can handle both fetching io request desc and commit
> command result in one trip. IO handling is often batched after single
> io_uring_enter() returns, both IO requests from ublk server target and
> IO commands could be handled as a whole batch.
>
> And the patch by patch change can be found in the following
> tree:
>
> https://github.com/ming1/linux/tree/my_for-5.20-ubd-devel_v4
>
> ublk server repo(master branch):
>
> 	https://github.com/ming1/ubdsrv
>
> Any comments are welcome!
>
> Since V3:
> - address Gabriel Krisman Bertazi's comments on V3: add userspace data
>   validation before handling command, remove warning, ...
> - remove UBLK_IO_COMMIT_REQ command as suggested by Zixiang and Gabriel Krisman Bertazi
> - fix one request double free when running abort
> - rewrite/cleanup ublk_copy_pages(), then this handling becomes very
>   clean
> - add one command of UBLK_IO_REFETCH_REQ for allowing ublk_drv to build
>   as module
>
> Since V2:
> - fix one big performance problem:
> 	https://github.com/ming1/linux/commit/3c9fd476951759858cc548dee4cedc074194d0b0
> - rename as ublk, as suggested by Gabriel Krisman Bertazi 
> - lots of cleanup & code improvement & bugfix, see details in git
>   hisotry
>
>
> Since V1:
>
> Remove RFC now because ublk driver codes gets lots of cleanup, enhancement and
> bug fixes since V1:
>
> - cleanup uapi: remove ublk specific error code,  switch to linux error code,
> remove one command op, remove one field from cmd_desc
>
> - add monitor mechanism to handle ubq_daemon being killed, ublksrv[1]
>   includes builtin tests for covering heavy IO with deleting ublk / killing
>   ubq_daemon at the same time, and V2 pass all the two tests(make test T=generic),
>   and the abort/stop mechanism is simple
>
> - fix MQ command buffer mmap bug, and now 'xfstetests -g auto' works well on
>   MQ ublk-loop devices(test/scratch)
>
> - improve batching submission as suggested by Jens
>
> - improve handling for starting device, replace random wait/poll with
> completion
>
> - all kinds of cleanup, bug fix,..
>
> Ming Lei (2):
>   ublk: add io_uring based userspace block driver
>   ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module
>
>  drivers/block/Kconfig         |    6 +
>  drivers/block/Makefile        |    2 +
>  drivers/block/ublk_drv.c      | 1701 +++++++++++++++++++++++++++++++++
>  include/uapi/linux/ublk_cmd.h |  173 ++++
>  4 files changed, 1882 insertions(+)
>  create mode 100644 drivers/block/ublk_drv.c
>  create mode 100644 include/uapi/linux/ublk_cmd.h
>

Re: [PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Ming Lei]

Hi Xiaoguang,

On Mon, Jul 11, 2022 at 07:32:19PM +0800, Xiaoguang Wang wrote:
> Hello Ming,
> 
> First thanks for this great work, I think ublk will be a powerful
> replacement for tcmu. I read your v3 ublk kernel and user-space
> codes, and have some ublk design questions here:
> 
> 1) As I said before, currently ublk still needs user-space backstore
> to allocate io buffer in advance, for example, UBLK_IO_FETCH_REQ
> command needs to set ublk_io->addr when starting device. Currently,
> some of our internal business may create hundreds or thousands of
> tcmu devices in one host, when switching to ublk, it'll need user-space
> backstore to allocate lots of memory in advance, which will waste memory
> in a host with swap off. Also user-space backstore may use advanced
> network components, they may maintain internal memory pool, which
> can be used as io buffer.
> 
> So I'd like to suggest that at least we add a new flag and keep GET_DATA
> command. When used, FETCH command does not need to pass io
> buffer addr, and backstore needs to send a GET_DATA command with
> user addr for write request.
> 
> Support high flexibility, let's user decides what's best for them.

Please take a look at v4 patches or cover letter at least before asking
this question.

V4 adds one new command of REFETCH for supporting to build ublk driver
as module, you can allocate buffer when receiving REFETCH command
in userspace target code by adding one pair of callbacks.

Also the latest ublkserver adds callback for target code to pre-allocate
buffer, then if you have pre-allocated io buffer, the buffer can be passed
to driver via FETCH command during setting up queue.

Actually I have implemented pinning page during the whole io lifetime,
then the pre-allocated io buffers can be reclaimed without needing
swapout by kernel when io is completed:

https://github.com/ming1/linux/commits/ubd-master

So the preallocation is just allocation on virtual memory space, and
the pages are pinned actually when io is handled. After io handling is
done, kernel can reclaim pages at will without needing swapout on
these io pages.

> 
> 2) complicated ublk user-space
> First forgive me :) I think current ublk user-space codes looks somewhat
> complicated:

Please just look at libublksrv code, and example of demo_null.c &
demo_event.c.

Zixiang mentioned that it basically did almost everything what you
requested before:

https://lore.kernel.org/linux-block/532eb193-06cf-96a3-684f-cc7e16db5ddf@linux.alibaba.com/

Maybe you two need to talk a bit.

>   1. UBLK_CMD_START_DEV and io handler worker need to be
> in different task context, because UBLK_CMD_START_DEV needs
> to wait the number of queue depth of sqes to be submitted in advance.

Of course we have to wait until all IO commands are issued to driver,
since block IO can come to /dev/ublkbN after UBLK_CMD_START_DEV returns,
and /dev/ublkbN is exposed to userspace in running UBLK_CMD_START_DEV.

What is the matter of this kind of handling?

Also with libublksrv, you can do everything just in single task context,
see:

https://github.com/ming1/ubdsrv/blob/master/demo_null.c

> 
>   2. mixed getting ublk command and target io handle in one io_uring instance
> I'm not sure it's a good design, see ublksrv_handle_cqe(), which contains

io_uring is supposed to be bound with context, and serves all IOs
issued from this context. That is exactly typical AIO use pattern,
please look at example of t/io_uring.c in fio project, which can accept
lots of files in command line, then handle IOs to all these files in one
single io_uring context. Here /dev/ublkcN is just one file, we handle
IOs to other files and /dev/ublkcN in single io_uring/context, then
all of them can be handled at batching, then each single syscall can
handle more IOs, that is one important reason why io_uring performs so well.

> many flag handle work and is_target_io() check, I think the data flow is not
> that clear for me at least :)

/* 
 * this flag is set if we need to tell ublk driver to fetch io req only,
 * just needed in setup stage.  
 */
#define UBLKSRV_NEED_FETCH_RQ		(1UL << 0)

/* when one io is handled, we set this flag for committing io result */
#define UBLKSRV_NEED_COMMIT_RQ_COMP	(1UL << 1)

/*
 * this flag is set in case the command slot is free to issue new command;
 * cleared when io command is issued to driver.
 */
#define UBLKSRV_IO_FREE			(1UL << 2)

/*
 * added in v4, set in case UBLK_IO_RES_REFETCH is returned from driver,
 * so REFETCH command is issued to driver
 */
#define UBLKSRV_NEED_REFETCH_RQ		(1UL << 3)

Note, the flags are just for handling io commands.

> 
>   3. helper like tcmulib_get_next_command()
> I wonder whether current ublk user-space can offer similar helper which
> will return a set of io commands to backstore easily.

No, io command is supposed to use by libublksrv internal use, and target
code should _not_ deal with any io command.

The target code should just focus on implementing ->handle_io_async() in
which one new io command is received from driver, same with
->target_io_done() which is called when one target io is completed by
io_uring.

If target code doesn't use io_uring to handle io, please refer to
example of demo_event.c, in which ->handle_event() is required for
supporting to handle io in another contexts by either io_uring or libaio
or whatever. ->handle_event() is called when io_uring(for issuing io
command) is waken up by eventfd, which is triggered by target code
itself(two eventfd APIs).

> 
> I'd like to suggest:
> 1. When starting ublk dev, pass io_uring fd for every queue, then in
> blk-mq->queue_rq(), it'll generate one cqe for every coming request,
> not need to issue fetch sqes command in advance, kernel codes would

Why do you think it isn't good to issue fetch sqes in advance? It costs
nothing, meantime userspace can get io request pretty fast.

Actually you are suggesting one fundamental change to io_uring given
the current io_uring use model is that userspace issues io via sqe, and
kernel(io_uring) completes io via cqe, and sqe and cqe are in two rings
actually.

That current io_uring doesn't support to complete cqe to userspace without
issuing any sqe, also not see any benefit we can get in this way. If you
have, please explain it in details.


> simplify a bit,  UBLK_IO_FLAG_ACTIVE may be discarded. And helper
> like returning a set of io command would be added easily. Note these
> io_uring fd would be just used for notifying io command generated.
> 
> 2. We use another io_uring fd per queue to handle GET_DATA or
> COMMIT_REQ command. Indeed, if we can support synchronous ioctl
> interface to do GET_DATA and COMMIT_REQ, we may make libublk
> really simple.

IMO that won't be good idea. One important reason why io_uring is so
efficient is that batching issue/completion in single syscall. And using
ioctl to handle io can be too slow.

> 
> 
> Here I'd like to describe how we use tcmu. A main thread call
> tcmulib_get_next_command() to get a set of io commands, then
> it dispatches them to user-space io wokers. Take write requests as
> example, io worker use ioctl(2) to get data from bios, and send
> data to distributed fs, finally call ioctl(2) to commit req. Multiple

Hammm, not mentioning pthread communication, it takes at least 3 syscalls
for handling one io, how can you expect this way to work efficiently?

With ublk, usually we handle dozens or even more than hundred of IOs in
single io_uring_enter() syscall.

> io workers can run concurrently. Since GET_DATA(write request)
> or COMMIT_REQ(read request) mainly do memcpy work, one
> io_uring instance will just do these jobs sequentially, which may
> not take advantage of multi-cpu.

IMO you can implement target code to handle io in other pthreads against
current libublksrv design, see demo_event.c. Or if you think it is still
enough, please just share with us what the problem is. Without
understanding the problem, I won't try to figure out any solution or
change.

Again, the goal of ublk aims at implementing high performance & generic
userspace user space block driver.



Thanks,
Ming

Re: [PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Gabriel Krisman Bertazi]

Ming Lei <ming.lei@redhat.com> writes:

> Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
> ubq daemon context, so we can avoid to call task_work_add(), then
> it is fine to build ublk driver as module.
>
> In this way, iops is affected a bit, but just by ~5% on ublk/null,
> given io_uring provides pretty good batching issuing & completing.
>
> One thing to be careful is race between ->queue_rq() and handling
> abort, which is avoided by quiescing queue when aborting queue.
> Except for that, handling abort becomes much easier with
> UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
> anything done in ubq daemon kernel context.

Hi Ming,

FWIW, I'm not very fond this change.  It adds complexity to the kernel
driver and to the userspace server implementation, who now have to deal
with different interface semantics just because the driver was built-in
or built as a module.  I don't think the tristate support warrants such
complexity.  I was hoping we might get away with exporting that symbol
or adding a built-in ubd-specific wrapper that can be exported and
invokes task_work_add.

Either way, Alibaba seems to consider this feature useful, and if that
is the case, we can just not use it on our side.

That said, the patch looks good to me, just a minor comment inline.

Thanks,

> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/block/Kconfig         |   2 +-
>  drivers/block/ublk_drv.c      | 121 ++++++++++++++++++++++++++--------
>  include/uapi/linux/ublk_cmd.h |  17 +++++
>  3 files changed, 113 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index d218089cdbec..2ba77fd960c2 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -409,7 +409,7 @@ config BLK_DEV_RBD
>  	  If unsure, say N.
>  
>  config BLK_DEV_UBLK
> -	bool "Userspace block driver"
> +	tristate "Userspace block driver"
>  	select IO_URING
>  	help
>            io uring based userspace block driver.
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 0076418e6fad..98482f8d1a77 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -92,6 +92,7 @@ struct ublk_queue {
>  	int q_id;
>  	int q_depth;
>  
> +	unsigned long flags;
>  	struct task_struct	*ubq_daemon;
>  	char *io_cmd_buf;
>  
> @@ -141,6 +142,15 @@ struct ublk_device {
>  	struct work_struct	stop_work;
>  };
>  
> +#define ublk_use_task_work(ubq)						\
> +({                                                                      \
> +	bool ret = false;						\
> +	if (IS_BUILTIN(CONFIG_BLK_DEV_UBLK) &&                          \
> +			!((ubq)->flags & UBLK_F_NEED_REFETCH))		\
> +		ret = true;						\
> +	ret;								\
> +})
> +

This should be an inline function, IMO.


-- 
Gabriel Krisman Bertazi

Re: [PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Ziyang Zhang]

On 2022/7/12 04:06, Gabriel Krisman Bertazi wrote:
> Ming Lei <ming.lei@redhat.com> writes:
> 
>> Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
>> ubq daemon context, so we can avoid to call task_work_add(), then
>> it is fine to build ublk driver as module.
>>
>> In this way, iops is affected a bit, but just by ~5% on ublk/null,
>> given io_uring provides pretty good batching issuing & completing.
>>
>> One thing to be careful is race between ->queue_rq() and handling
>> abort, which is avoided by quiescing queue when aborting queue.
>> Except for that, handling abort becomes much easier with
>> UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
>> anything done in ubq daemon kernel context.
> 
> Hi Ming,
> 
> FWIW, I'm not very fond this change.  It adds complexity to the kernel
> driver and to the userspace server implementation, who now have to deal
> with different interface semantics just because the driver was built-in
> or built as a module.  I don't think the tristate support warrants such
> complexity.  I was hoping we might get away with exporting that symbol
> or adding a built-in ubd-specific wrapper that can be exported and
> invokes task_work_add.
> 
> Either way, Alibaba seems to consider this feature useful, and if that
> is the case, we can just not use it on our side.

Our app handles IOs itself with network(RPC) and internal memory pool
so UBLK_IO_REFETCH_REQ
(actually I think it is like NEED_GET_DATA in the earlist version :) )
is helpful to us because we can assign data buffer address AFTER the app
gets one IO requests(WRITE, with data size) and we avoid PRE-allocating buffers.

Besides, adding UBLK_IO_REFETCH_REQ is helpful to build ublk driver as module
It seems like kernel developers do not want a built-in driver. :)

Maybe your app is different from ours(you may not need to handle IOs by yourelf).

Thanks， 
Ziyang Zhang


> 
> That said, the patch looks good to me, just a minor comment inline.
> 
> Thanks,
> 
>> Signed-off-by: Ming Lei <ming.lei@redhat.com>
>> ---
>>  drivers/block/Kconfig         |   2 +-
>>  drivers/block/ublk_drv.c      | 121 ++++++++++++++++++++++++++--------
>>  include/uapi/linux/ublk_cmd.h |  17 +++++
>>  3 files changed, 113 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
>> index d218089cdbec..2ba77fd960c2 100644
>> --- a/drivers/block/Kconfig
>> +++ b/drivers/block/Kconfig
>> @@ -409,7 +409,7 @@ config BLK_DEV_RBD
>>  	  If unsure, say N.
>>  
>>  config BLK_DEV_UBLK
>> -	bool "Userspace block driver"
>> +	tristate "Userspace block driver"
>>  	select IO_URING
>>  	help
>>            io uring based userspace block driver.
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 0076418e6fad..98482f8d1a77 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -92,6 +92,7 @@ struct ublk_queue {
>>  	int q_id;
>>  	int q_depth;
>>  
>> +	unsigned long flags;
>>  	struct task_struct	*ubq_daemon;
>>  	char *io_cmd_buf;
>>  
>> @@ -141,6 +142,15 @@ struct ublk_device {
>>  	struct work_struct	stop_work;
>>  };
>>  
>> +#define ublk_use_task_work(ubq)						\
>> +({                                                                      \
>> +	bool ret = false;						\
>> +	if (IS_BUILTIN(CONFIG_BLK_DEV_UBLK) &&                          \
>> +			!((ubq)->flags & UBLK_F_NEED_REFETCH))		\
>> +		ret = true;						\
>> +	ret;								\
>> +})
>> +
> 
> This should be an inline function, IMO.
> 
> 

Re: [PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Ming Lei]

Hi Gabriel,

On Mon, Jul 11, 2022 at 04:06:04PM -0400, Gabriel Krisman Bertazi wrote:
> Ming Lei <ming.lei@redhat.com> writes:
> 
> > Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
> > ubq daemon context, so we can avoid to call task_work_add(), then
> > it is fine to build ublk driver as module.
> >
> > In this way, iops is affected a bit, but just by ~5% on ublk/null,
> > given io_uring provides pretty good batching issuing & completing.
> >
> > One thing to be careful is race between ->queue_rq() and handling
> > abort, which is avoided by quiescing queue when aborting queue.
> > Except for that, handling abort becomes much easier with
> > UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
> > anything done in ubq daemon kernel context.
> 
> Hi Ming,
> 
> FWIW, I'm not very fond this change.  It adds complexity to the kernel
> driver and to the userspace server implementation, who now have to deal

IMO, this way just adds dozens line of code, no much complexity. The only
complexity in ublk driver should be in aborting code, which is actually
originated from concurrent aborting work and running task work which may be
run after task is exiting. But any storage driver's aborting/error
handling code is complicated.

Using REFETCH_REQ actually becomes much easier for handling abort which is
run exclusively with any code running in ubq daemon context, but with
performance cost.

> with different interface semantics just because the driver was built-in
> or built as a module.  I don't think the tristate support warrants such
> complexity.  I was hoping we might get away with exporting that symbol
> or adding a built-in ubd-specific wrapper that can be exported and
> invokes task_work_add.

If task_work_add can be exported, that would be very great.

Cc more guys who contributed to task_work_add().

Oleg, Jens and other guys, I am wondering if you may share your opinion
wrt. exporting task_work_add for ublk's use case? Then ublk_drv can be
built as module without this patch.

> 
> Either way, Alibaba seems to consider this feature useful, and if that
> is the case, we can just not use it on our side.

If this new added command is proved as useful, we may add it without
binding with module if task_work_add can be exported.

> 
> That said, the patch looks good to me, just a minor comment inline.
> 
> Thanks,
> 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/block/Kconfig         |   2 +-
> >  drivers/block/ublk_drv.c      | 121 ++++++++++++++++++++++++++--------
> >  include/uapi/linux/ublk_cmd.h |  17 +++++
> >  3 files changed, 113 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> > index d218089cdbec..2ba77fd960c2 100644
> > --- a/drivers/block/Kconfig
> > +++ b/drivers/block/Kconfig
> > @@ -409,7 +409,7 @@ config BLK_DEV_RBD
> >  	  If unsure, say N.
> >  
> >  config BLK_DEV_UBLK
> > -	bool "Userspace block driver"
> > +	tristate "Userspace block driver"
> >  	select IO_URING
> >  	help
> >            io uring based userspace block driver.
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 0076418e6fad..98482f8d1a77 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -92,6 +92,7 @@ struct ublk_queue {
> >  	int q_id;
> >  	int q_depth;
> >  
> > +	unsigned long flags;
> >  	struct task_struct	*ubq_daemon;
> >  	char *io_cmd_buf;
> >  
> > @@ -141,6 +142,15 @@ struct ublk_device {
> >  	struct work_struct	stop_work;
> >  };
> >  
> > +#define ublk_use_task_work(ubq)						\
> > +({                                                                      \
> > +	bool ret = false;						\
> > +	if (IS_BUILTIN(CONFIG_BLK_DEV_UBLK) &&                          \
> > +			!((ubq)->flags & UBLK_F_NEED_REFETCH))		\
> > +		ret = true;						\
> > +	ret;								\
> > +})
> > +
> 
> This should be an inline function, IMO.

You are right. I worried that compiler may not be intelligent
enough for handling the code correctly. But just tried inline,
task_work_add() won't be linked in if CONFIG_BLK_DEV_UBLK is
defined as m.


Thanks,
Ming

Re: [PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Ming Lei]

On Tue, Jul 12, 2022 at 10:26:47AM +0800, Ziyang Zhang wrote:
> On 2022/7/12 04:06, Gabriel Krisman Bertazi wrote:
> > Ming Lei <ming.lei@redhat.com> writes:
> > 
> >> Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
> >> ubq daemon context, so we can avoid to call task_work_add(), then
> >> it is fine to build ublk driver as module.
> >>
> >> In this way, iops is affected a bit, but just by ~5% on ublk/null,
> >> given io_uring provides pretty good batching issuing & completing.
> >>
> >> One thing to be careful is race between ->queue_rq() and handling
> >> abort, which is avoided by quiescing queue when aborting queue.
> >> Except for that, handling abort becomes much easier with
> >> UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
> >> anything done in ubq daemon kernel context.
> > 
> > Hi Ming,
> > 
> > FWIW, I'm not very fond this change.  It adds complexity to the kernel
> > driver and to the userspace server implementation, who now have to deal
> > with different interface semantics just because the driver was built-in
> > or built as a module.  I don't think the tristate support warrants such
> > complexity.  I was hoping we might get away with exporting that symbol
> > or adding a built-in ubd-specific wrapper that can be exported and
> > invokes task_work_add.
> > 
> > Either way, Alibaba seems to consider this feature useful, and if that
> > is the case, we can just not use it on our side.
> 
> Our app handles IOs itself with network(RPC) and internal memory pool
> so UBLK_IO_REFETCH_REQ
> (actually I think it is like NEED_GET_DATA in the earlist version :) )
> is helpful to us because we can assign data buffer address AFTER the app
> gets one IO requests(WRITE, with data size) and we avoid PRE-allocating buffers.

Maybe you can consider to switch to pre-allocation.

The patch[1] for pinning io vm pages in the io lifetime has been done, just
not included in this patchset, and it passes all the builtin tests, but
there is still space for further optimization.

With that patchset[1] in, io pages becomes pinned during whole io handling time,
after io is done, mm can reclaim these pages without needing to swapout. It
works like madvise(MADV_DONTNEED).

[1] https://github.com/ming1/linux/commits/ubd-master


Thanks, 
Ming

Re: [PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Xiaoguang Wang]

Hello Ming,

> Hi Xiaoguang,
>
> On Mon, Jul 11, 2022 at 07:32:19PM +0800, Xiaoguang Wang wrote:
> Please take a look at v4 patches or cover letter at least before asking
> this question.
Yeah, I should be, really sorry.
>
> V4 adds one new command of REFETCH for supporting to build ublk driver
> as module, you can allocate buffer when receiving REFETCH command
> in userspace target code by adding one pair of callbacks.
>
> Also the latest ublkserver adds callback for target code to pre-allocate
> buffer, then if you have pre-allocated io buffer, the buffer can be passed
> to driver via FETCH command during setting up queue.
Now my concern about io buffer management has gone, thanks.

>
> Actually I have implemented pinning page during the whole io lifetime,
> then the pre-allocated io buffers can be reclaimed without needing
> swapout by kernel when io is completed:
>
> https://github.com/ming1/linux/commits/ubd-master
>
> So the preallocation is just allocation on virtual memory space, and
> the pages are pinned actually when io is handled. After io handling is
> done, kernel can reclaim pages at will without needing swapout on
> these io pages.
OK, I'll learn codes later.

>
>> 2) complicated ublk user-space
>> First forgive me :) I think current ublk user-space codes looks somewhat
>> complicated:
> Please just look at libublksrv code, and example of demo_null.c &
> demo_event.c.
OK.

>
>
> Of course we have to wait until all IO commands are issued to driver,
> since block IO can come to /dev/ublkbN after UBLK_CMD_START_DEV returns,
> and /dev/ublkbN is exposed to userspace in running UBLK_CMD_START_DEV.
>
> What is the matter of this kind of handling?
>
> Also with libublksrv, you can do everything just in single task context,
> see:
>
> https://github.com/ming1/ubdsrv/blob/master/demo_null.c
No, indeed I don't mean that there are something wrong with your
implementation. I just try to see whether I can simplify it a bit.

If we adopt to pass one io_uring fd per queue when starting device,
blk-mq's queue_rq() will get corresponding io_uring file for this queue and
use it to generate cqes directly to notify new io commands inserted,
then UBLK_CMD_START_DEV doesn't need to wait, and can be in the
same thread with ublksrv_queue_init or ublksrv_process_io.
Seems that for demo_null.c, they are still in different threads.

For current io_uring implementation, one sqe may generate one or more
cqes, but indeed, we can generate cqes without submitting sqes, just
fill one event to io_uring ctx.

Just suggestions :)
>
>>   2. mixed getting ublk command and target io handle in one io_uring instance
>> I'm not sure it's a good design, see ublksrv_handle_cqe(), which contains
> io_uring is supposed to be bound with context, and serves all IOs
> issued from this context. That is exactly typical AIO use pattern,
> please look at example of t/io_uring.c in fio project, which can accept
> lots of files in command line, then handle IOs to all these files in one
> single io_uring context. Here /dev/ublkcN is just one file, we handle
> IOs to other files and /dev/ublkcN in single io_uring/context, then
> all of them can be handled at batching, then each single syscall can
> handle more IOs, that is one important reason why io_uring performs so well.
Yeah, I understand that you're doing your best to improve ublk performance,
and I'm a early developer of io_uring and know how it works :)

It maybe just because of my poor design poor taste, I think put
io command descriptors acquire and io command handling together
seem not decouple well.
>
>> many flag handle work and is_target_io() check, I think the data flow is not
>> that clear for me at least :)
> /* 
>  * this flag is set if we need to tell ublk driver to fetch io req only,
>  * just needed in setup stage.  
>  */
> #define UBLKSRV_NEED_FETCH_RQ		(1UL << 0)
>
> /* when one io is handled, we set this flag for committing io result */
> #define UBLKSRV_NEED_COMMIT_RQ_COMP	(1UL << 1)
>
> /*
>  * this flag is set in case the command slot is free to issue new command;
>  * cleared when io command is issued to driver.
>  */
> #define UBLKSRV_IO_FREE			(1UL << 2)
>
> /*
>  * added in v4, set in case UBLK_IO_RES_REFETCH is returned from driver,
>  * so REFETCH command is issued to driver
>  */
> #define UBLKSRV_NEED_REFETCH_RQ		(1UL << 3)
>
> Note, the flags are just for handling io commands.
>
>>   3. helper like tcmulib_get_next_command()
>> I wonder whether current ublk user-space can offer similar helper which
>> will return a set of io commands to backstore easily.
> No, io command is supposed to use by libublksrv internal use, and target
> code should _not_ deal with any io command.
Seems different from design ideas of tcmu.

>
> The target code should just focus on implementing ->handle_io_async() in
> which one new io command is received from driver, same with
> ->target_io_done() which is called when one target io is completed by
> io_uring.
>
> If target code doesn't use io_uring to handle io, please refer to
> example of demo_event.c, in which ->handle_event() is required for
> supporting to handle io in another contexts by either io_uring or libaio
> or whatever. ->handle_event() is called when io_uring(for issuing io
> command) is waken up by eventfd, which is triggered by target code
> itself(two eventfd APIs).
OK.

>
>> I'd like to suggest:
>> 1. When starting ublk dev, pass io_uring fd for every queue, then in
>> blk-mq->queue_rq(), it'll generate one cqe for every coming request,
>> not need to issue fetch sqes command in advance, kernel codes would
> Why do you think it isn't good to issue fetch sqes in advance? It costs
> nothing, meantime userspace can get io request pretty fast.
>
> Actually you are suggesting one fundamental change to io_uring given
> the current io_uring use model is that userspace issues io via sqe, and
> kernel(io_uring) completes io via cqe, and sqe and cqe are in two rings
> actually.
>
> That current io_uring doesn't support to complete cqe to userspace without
> issuing any sqe, also not see any benefit we can get in this way. If you
> have, please explain it in details.
Hard to say it's one fundamental change, io_uring can easily add such
a helper which generates cqes but needs not to submit sqes, which contains
  allocate one cqe, with user_data, res
  io_commit_cqring(ctx);

As I said before, there maybe such benefits:
1. may decouple io command descriptor acquire and io command handling well.
At least helper like tcmulib_get_next_command maybe added easily. I'm not sure, some
applications based on tcmu previously may need this helper.

2. UBLK_CMD_START_DEV won't need to wait another thread context to submit
number of queue depth of sqes firstly, but I admit that it's not a big issue.

>
>
>> simplify a bit,  UBLK_IO_FLAG_ACTIVE may be discarded. And helper
>> like returning a set of io command would be added easily. Note these
>> io_uring fd would be just used for notifying io command generated.
>>
>> 2. We use another io_uring fd per queue to handle GET_DATA or
>> COMMIT_REQ command. Indeed, if we can support synchronous ioctl
>> interface to do GET_DATA and COMMIT_REQ, we may make libublk
>> really simple.
> IMO that won't be good idea. One important reason why io_uring is so
> efficient is that batching issue/completion in single syscall. And using
> ioctl to handle io can be too slow.
>
>>
>> Here I'd like to describe how we use tcmu. A main thread call
>> tcmulib_get_next_command() to get a set of io commands, then
>> it dispatches them to user-space io wokers. Take write requests as
>> example, io worker use ioctl(2) to get data from bios, and send
>> data to distributed fs, finally call ioctl(2) to commit req. Multiple
> Hammm, not mentioning pthread communication, it takes at least 3 syscalls
> for handling one io, how can you expect this way to work efficiently?
I admit batch will be good, and syscalls userspace and kernel context switch
introduce overhead. But for big io requests, batch in one context is not good. In
the example of read requests, if io request is big, say 1MB, io_uring will do
commit req sequentially, which indeed mainly do memcpy work. But if users
can choose to issue multiple ioctls which do commit req concurrently, I think
user may get higher io throughput.

And in this case, user may not care userspace and kernel context switch overhead at all.

Or to put it another way, should libublk offer synchronous programming interface ?

>
> With ublk, usually we handle dozens or even more than hundred of IOs in
> single io_uring_enter() syscall.
>
>> io workers can run concurrently. Since GET_DATA(write request)
>> or COMMIT_REQ(read request) mainly do memcpy work, one
>> io_uring instance will just do these jobs sequentially, which may
>> not take advantage of multi-cpu.
> IMO you can implement target code to handle io in other pthreads against
> current libublksrv design, see demo_event.c. Or if you think it is still
> enough, please just share with us what the problem is. Without
> understanding the problem, I won't try to figure out any solution or
> change.
I need to read your ublk userspace codes carefully, if I made
some noises, sorry.
>
> Again, the goal of ublk aims at implementing high performance & generic
> userspace user space block driver.
Yeah, sure, thanks for this work again.

Regards,
Xiaoguang Wang
>
>
>
> Thanks,
> Ming

Re: [PATCH V4 2/2] ublk_drv: add UBLK_IO_REFETCH_REQ for supporting to build as module

[Ming Lei]

On Tue, Jul 12, 2022 at 10:33:34AM +0800, Ming Lei wrote:
> Hi Gabriel,
> 
> On Mon, Jul 11, 2022 at 04:06:04PM -0400, Gabriel Krisman Bertazi wrote:
> > Ming Lei <ming.lei@redhat.com> writes:
> > 
> > > Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in
> > > ubq daemon context, so we can avoid to call task_work_add(), then
> > > it is fine to build ublk driver as module.
> > >
> > > In this way, iops is affected a bit, but just by ~5% on ublk/null,
> > > given io_uring provides pretty good batching issuing & completing.
> > >
> > > One thing to be careful is race between ->queue_rq() and handling
> > > abort, which is avoided by quiescing queue when aborting queue.
> > > Except for that, handling abort becomes much easier with
> > > UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with
> > > anything done in ubq daemon kernel context.
> > 
> > Hi Ming,
> > 
> > FWIW, I'm not very fond this change.  It adds complexity to the kernel
> > driver and to the userspace server implementation, who now have to deal
> 
> IMO, this way just adds dozens line of code, no much complexity. The only
> complexity in ublk driver should be in aborting code, which is actually
> originated from concurrent aborting work and running task work which may be
> run after task is exiting. But any storage driver's aborting/error
> handling code is complicated.
> 
> Using REFETCH_REQ actually becomes much easier for handling abort which is
> run exclusively with any code running in ubq daemon context, but with
> performance cost.
> 
> > with different interface semantics just because the driver was built-in
> > or built as a module.  I don't think the tristate support warrants such
> > complexity.  I was hoping we might get away with exporting that symbol
> > or adding a built-in ubd-specific wrapper that can be exported and
> > invokes task_work_add.
> 
> If task_work_add can be exported, that would be very great.

Another choice is to use io_uring_cmd_complete_in_task which is actually
exported, now we can build ublk_drv as module by using io_uring_cmd_complete_in_task
without needing one new command.

Thanks,
Ming

Re: [PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Ming Lei]

On Tue, Jul 12, 2022 at 04:44:03PM +0800, Xiaoguang Wang wrote:
> Hello Ming,
> 
> > Hi Xiaoguang,
> >
> > On Mon, Jul 11, 2022 at 07:32:19PM +0800, Xiaoguang Wang wrote:
> > Please take a look at v4 patches or cover letter at least before asking
> > this question.
> Yeah, I should be, really sorry.
> >
> > V4 adds one new command of REFETCH for supporting to build ublk driver
> > as module, you can allocate buffer when receiving REFETCH command
> > in userspace target code by adding one pair of callbacks.
> >
> > Also the latest ublkserver adds callback for target code to pre-allocate
> > buffer, then if you have pre-allocated io buffer, the buffer can be passed
> > to driver via FETCH command during setting up queue.
> Now my concern about io buffer management has gone, thanks.
> 
> >
> > Actually I have implemented pinning page during the whole io lifetime,
> > then the pre-allocated io buffers can be reclaimed without needing
> > swapout by kernel when io is completed:
> >
> > https://github.com/ming1/linux/commits/ubd-master
> >
> > So the preallocation is just allocation on virtual memory space, and
> > the pages are pinned actually when io is handled. After io handling is
> > done, kernel can reclaim pages at will without needing swapout on
> > these io pages.
> OK, I'll learn codes later.
> 
> >
> >> 2) complicated ublk user-space
> >> First forgive me :) I think current ublk user-space codes looks somewhat
> >> complicated:
> > Please just look at libublksrv code, and example of demo_null.c &
> > demo_event.c.
> OK.
> 
> >
> >
> > Of course we have to wait until all IO commands are issued to driver,
> > since block IO can come to /dev/ublkbN after UBLK_CMD_START_DEV returns,
> > and /dev/ublkbN is exposed to userspace in running UBLK_CMD_START_DEV.
> >
> > What is the matter of this kind of handling?
> >
> > Also with libublksrv, you can do everything just in single task context,
> > see:
> >
> > https://github.com/ming1/ubdsrv/blob/master/demo_null.c
> No, indeed I don't mean that there are something wrong with your
> implementation. I just try to see whether I can simplify it a bit.
> 
> If we adopt to pass one io_uring fd per queue when starting device,
> blk-mq's queue_rq() will get corresponding io_uring file for this queue and
> use it to generate cqes directly to notify new io commands inserted,
> then UBLK_CMD_START_DEV doesn't need to wait, and can be in the
> same thread with ublksrv_queue_init or ublksrv_process_io.
> Seems that for demo_null.c, they are still in different threads.
> 
> For current io_uring implementation, one sqe may generate one or more
> cqes, but indeed, we can generate cqes without submitting sqes, just
> fill one event to io_uring ctx.
> Just suggestions :)

I don't have any interest in so unusual usage of io_uring, especially it
needs fundamental change in io_uring.

Compared with kernel side change, removing waiting until queue setup in
userspace side simplifies nothing.

> >
> >>   2. mixed getting ublk command and target io handle in one io_uring instance
> >> I'm not sure it's a good design, see ublksrv_handle_cqe(), which contains
> > io_uring is supposed to be bound with context, and serves all IOs
> > issued from this context. That is exactly typical AIO use pattern,
> > please look at example of t/io_uring.c in fio project, which can accept
> > lots of files in command line, then handle IOs to all these files in one
> > single io_uring context. Here /dev/ublkcN is just one file, we handle
> > IOs to other files and /dev/ublkcN in single io_uring/context, then
> > all of them can be handled at batching, then each single syscall can
> > handle more IOs, that is one important reason why io_uring performs so well.
> Yeah, I understand that you're doing your best to improve ublk performance,
> and I'm a early developer of io_uring and know how it works :)
> 
> It maybe just because of my poor design poor taste, I think put
> io command descriptors acquire and io command handling together
> seem not decouple well.
> >
> >> many flag handle work and is_target_io() check, I think the data flow is not
> >> that clear for me at least :)
> > /* 
> >  * this flag is set if we need to tell ublk driver to fetch io req only,
> >  * just needed in setup stage.  
> >  */
> > #define UBLKSRV_NEED_FETCH_RQ		(1UL << 0)
> >
> > /* when one io is handled, we set this flag for committing io result */
> > #define UBLKSRV_NEED_COMMIT_RQ_COMP	(1UL << 1)
> >
> > /*
> >  * this flag is set in case the command slot is free to issue new command;
> >  * cleared when io command is issued to driver.
> >  */
> > #define UBLKSRV_IO_FREE			(1UL << 2)
> >
> > /*
> >  * added in v4, set in case UBLK_IO_RES_REFETCH is returned from driver,
> >  * so REFETCH command is issued to driver
> >  */
> > #define UBLKSRV_NEED_REFETCH_RQ		(1UL << 3)
> >
> > Note, the flags are just for handling io commands.
> >
> >>   3. helper like tcmulib_get_next_command()
> >> I wonder whether current ublk user-space can offer similar helper which
> >> will return a set of io commands to backstore easily.
> > No, io command is supposed to use by libublksrv internal use, and target
> > code should _not_ deal with any io command.
> Seems different from design ideas of tcmu.
> 
> >
> > The target code should just focus on implementing ->handle_io_async() in
> > which one new io command is received from driver, same with
> > ->target_io_done() which is called when one target io is completed by
> > io_uring.
> >
> > If target code doesn't use io_uring to handle io, please refer to
> > example of demo_event.c, in which ->handle_event() is required for
> > supporting to handle io in another contexts by either io_uring or libaio
> > or whatever. ->handle_event() is called when io_uring(for issuing io
> > command) is waken up by eventfd, which is triggered by target code
> > itself(two eventfd APIs).
> OK.
> 
> >
> >> I'd like to suggest:
> >> 1. When starting ublk dev, pass io_uring fd for every queue, then in
> >> blk-mq->queue_rq(), it'll generate one cqe for every coming request,
> >> not need to issue fetch sqes command in advance, kernel codes would
> > Why do you think it isn't good to issue fetch sqes in advance? It costs
> > nothing, meantime userspace can get io request pretty fast.
> >
> > Actually you are suggesting one fundamental change to io_uring given
> > the current io_uring use model is that userspace issues io via sqe, and
> > kernel(io_uring) completes io via cqe, and sqe and cqe are in two rings
> > actually.
> >
> > That current io_uring doesn't support to complete cqe to userspace without
> > issuing any sqe, also not see any benefit we can get in this way. If you
> > have, please explain it in details.
> Hard to say it's one fundamental change, io_uring can easily add such
> a helper which generates cqes but needs not to submit sqes, which contains
>   allocate one cqe, with user_data, res
>   io_commit_cqring(ctx);

You still need io_kiocb/io_uring_cmd and both are allocated in
submission code path, since 'io_ring_ctx' is private for
io_uring.

And ublk server still needs one command to send io result to ublk driver,
that said this way saves nothing for us cause ublk driver uses single
command for both fetching io request and committing io result.

Easy to say than done, you may try to write a patch to verify your
ideas, especially no one uses io_ring in this way.

> 
> As I said before, there maybe such benefits:
> 1. may decouple io command descriptor acquire and io command handling well.
> At least helper like tcmulib_get_next_command maybe added easily. I'm not sure, some
> applications based on tcmu previously may need this helper.

Why do we need similar helper of tcmulib_get_next_command()? for what
purpose? In current design of ublk server, it should be enough for target
code to focus on ->handle_io_async(), ->target_io_done() in case of handling
target io via ubq io_uring, ->handle_event() in case of handling target io in
other contexts.

ublk server is one io_uring application, and interfaces provided
are based on io_uring design/interfaces. I guess TCMU isn't based on
io_uring, so it may have original style interface. You may ask
TCMU guys if it is possible to reuse current interface for supporting
io_uring with expected performance.

> 
> 2. UBLK_CMD_START_DEV won't need to wait another thread context to submit
> number of queue depth of sqes firstly, but I admit that it's not a big issue.

Yeah, it simplifies nothing, compared with fundamental io_uring change
and ublk driver side change.

> 
> >
> >
> >> simplify a bit,  UBLK_IO_FLAG_ACTIVE may be discarded. And helper
> >> like returning a set of io command would be added easily. Note these
> >> io_uring fd would be just used for notifying io command generated.
> >>
> >> 2. We use another io_uring fd per queue to handle GET_DATA or
> >> COMMIT_REQ command. Indeed, if we can support synchronous ioctl
> >> interface to do GET_DATA and COMMIT_REQ, we may make libublk
> >> really simple.
> > IMO that won't be good idea. One important reason why io_uring is so
> > efficient is that batching issue/completion in single syscall. And using
> > ioctl to handle io can be too slow.
> >
> >>
> >> Here I'd like to describe how we use tcmu. A main thread call
> >> tcmulib_get_next_command() to get a set of io commands, then
> >> it dispatches them to user-space io wokers. Take write requests as
> >> example, io worker use ioctl(2) to get data from bios, and send
> >> data to distributed fs, finally call ioctl(2) to commit req. Multiple
> > Hammm, not mentioning pthread communication, it takes at least 3 syscalls
> > for handling one io, how can you expect this way to work efficiently?
> I admit batch will be good, and syscalls userspace and kernel context switch
> introduce overhead. But for big io requests, batch in one context is not good. In
> the example of read requests, if io request is big, say 1MB, io_uring will do
> commit req sequentially, which indeed mainly do memcpy work. But if users
> can choose to issue multiple ioctls which do commit req concurrently, I think
> user may get higher io throughput.

If BS is big, single job could saturate devices bw easily, multiple jobs won't
make a difference, will it? Not mention sync cost among multiple jobs.

Not mention current ublk server does support multiple io jobs.

> 
> And in this case, user may not care userspace and kernel context switch overhead at all.
> 
> Or to put it another way, should libublk offer synchronous programming interface ?

It depends what the sync interface is.

You can call sync read()/write() for target io directly in ->handdle_io_async(),
or call them in other pthread(s) just by telling ubq after these sync ios are done
with the eventfd API.

demo_null.c is actually one such example.

> 
> >
> > With ublk, usually we handle dozens or even more than hundred of IOs in
> > single io_uring_enter() syscall.
> >
> >> io workers can run concurrently. Since GET_DATA(write request)
> >> or COMMIT_REQ(read request) mainly do memcpy work, one
> >> io_uring instance will just do these jobs sequentially, which may
> >> not take advantage of multi-cpu.
> > IMO you can implement target code to handle io in other pthreads against
> > current libublksrv design, see demo_event.c. Or if you think it is still
> > enough, please just share with us what the problem is. Without
> > understanding the problem, I won't try to figure out any solution or
> > change.
> I need to read your ublk userspace codes carefully, if I made

One small suggestion, you may start with the two builtin examples, both
are simple & clean, and the big one is only 300+ LOC, really complicated?


Thanks.
Ming

Re: [PATCH V4 0/2] ublk: add io_uring based userspace block driver

[Xiaoguang Wang]

hi,

>>>
>>> If we adopt to pass one io_uring fd per queue when starting device,
>>> blk-mq's queue_rq() will get corresponding io_uring file for this queue and
>>> use it to generate cqes directly to notify new io commands inserted,
>>> then UBLK_CMD_START_DEV doesn't need to wait, and can be in the
>>> same thread with ublksrv_queue_init or ublksrv_process_io.
>>> Seems that for demo_null.c, they are still in different threads.
>>>
>>> For current io_uring implementation, one sqe may generate one or more
>>> cqes, but indeed, we can generate cqes without submitting sqes, just
>>> fill one event to io_uring ctx.
>>> Just suggestions :)
> I don't have any interest in so unusual usage of io_uring, especially it
> needs fundamental change in io_uring.
Got it, I see your point now and will respect that.

>
> Compared with kernel side change, removing waiting until queue setup in
> userspace side simplifies nothing.
>
> You still need io_kiocb/io_uring_cmd and both are allocated in
> submission code path, since 'io_ring_ctx' is private for
> io_uring.
>
> And ublk server still needs one command to send io result to ublk driver,
> that said this way saves nothing for us cause ublk driver uses single
> command for both fetching io request and committing io result.
>
> Easy to say than done, you may try to write a patch to verify your
> ideas, especially no one uses io_ring in this way.
I have done a hack change in io_uring:
iff --git a/fs/io_uring.c b/fs/io_uring.c
index 5ff2cdb425bc..e6696319148e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -11958,6 +11958,21 @@ static int io_get_ext_arg(unsigned flags, const void __user *argp, size_t *argsz
        return 0;
 }

+static u64 tst_count;
+
+static void io_gen_cqe_direct(struct file *file)
+{
+       struct io_ring_ctx *ctx;
+       ctx = file->private_data;
+
+       printk(KERN_ERR "tst_count %llu\n", tst_count);
+       spin_lock(&ctx->completion_lock);
+       io_fill_cqe_aux(ctx, tst_count++, 0, 0);
+       io_commit_cqring(ctx);
+       spin_unlock(&ctx->completion_lock);
+       io_cqring_ev_posted(ctx);
+}
+
 SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
                u32, min_complete, u32, flags, const void __user *, argp,
                size_t, argsz)
@@ -12005,6 +12020,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
        if (unlikely(ctx->flags & IORING_SETUP_R_DISABLED))
                goto out;

+       io_gen_cqe_direct(f.file);
        /*
         * For SQ polling, the thread will do all submissions and completions.
         * Just return the requested submit count, and wake the thread if

And in user-space:
ret = io_uring_queue_init(QD, &ring, 0);

for (i = 0; i < 100; i++) {
    io_uring_submit_and_wait(&ring, 0);
    io_uring_wait_cqe(&ring, &cqe);
    printf("lege user_data %llu\n", cqe->user_data);
    io_uring_cqe_seen(&ring, cqe);
}

Now user-space app will get cqes continually, by it does not submit any sqes.
Indeed, io_fill_cqe_aux() has been used somewhere in kernel io_uring, for example,
sent one cqe from one io_uring instance to another instance.

I had planned to use io_gen_cqe_direct() in ublk's queue_rq to notify io requests
inserted. But now I'm fine that dropping this idea.
>
>> As I said before, there maybe such benefits:
>> 1. may decouple io command descriptor acquire and io command handling well.
>> At least helper like tcmulib_get_next_command maybe added easily. I'm not sure, some
>> applications based on tcmu previously may need this helper.
> Why do we need similar helper of tcmulib_get_next_command()? for what
> purpose? In current design of ublk server, it should be enough for target
> code to focus on ->handle_io_async(), ->target_io_done() in case of handling
> target io via ubq io_uring, ->handle_event() in case of handling target io in
> other contexts.
I'll have a deep think about it. Initially I tried to make libublk offer
similar programming interface to libtcmu, so apps can switch to ublk
easily, but indeed they maybe totally different things.

Sorry for noises again.
>
> ublk server is one io_uring application, and interfaces provided
> are based on io_uring design/interfaces. I guess TCMU isn't based on
> io_uring, so it may have original style interface. You may ask
> TCMU guys if it is possible to reuse current interface for supporting
> io_uring with expected performance.
>
>> 2. UBLK_CMD_START_DEV won't need to wait another thread context to submit
>> number of queue depth of sqes firstly, but I admit that it's not a big issue.
> Yeah, it simplifies nothing, compared with fundamental io_uring change
> and ublk driver side change.
>
>>
>> I admit batch will be good, and syscalls userspace and kernel context switch
>> introduce overhead. But for big io requests, batch in one context is not good. In
>> the example of read requests, if io request is big, say 1MB, io_uring will do
>> commit req sequentially, which indeed mainly do memcpy work. But if users
>> can choose to issue multiple ioctls which do commit req concurrently, I think
>> user may get higher io throughput.
> If BS is big, single job could saturate devices bw easily, multiple jobs won't
> make a difference, will it? Not mention sync cost among multiple jobs.n
No, I don't consider device scenes, at least for us, our target will visit distributed
file systems, which can offer very high io throughput, far greater than normal device.
>
> Not mention current ublk server does support multiple io jobs.
Yeah, I see that. But for read request, commit req commands will still be done
in ubq->ubq_daemon task context, and commit req commands mainly do memcpy work.

Regards,
Xiaoguang Wang

>
>> And in this case, user may not care userspace and kernel context switch overhead at all.
>>
>> Or to put it another way, should libublk offer synchronous programming interface ?
> It depends what the sync interface is.
>
> You can call sync read()/write() for target io directly in ->handdle_io_async(),
> or call them in other pthread(s) just by telling ubq after these sync ios are done
> with the eventfd API.
>
> demo_null.c is actually one such example.
>
>>> With ublk, usually we handle dozens or even more than hundred of IOs in
>>> single io_uring_enter() syscall.
>>>
>>>> io workers can run concurrently. Since GET_DATA(write request)
>>>> or COMMIT_REQ(read request) mainly do memcpy work, one
>>>> io_uring instance will just do these jobs sequentially, which may
>>>> not take advantage of multi-cpu.
>>> IMO you can implement target code to handle io in other pthreads against
>>> current libublksrv design, see demo_event.c. Or if you think it is still
>>> enough, please just share with us what the problem is. Without
>>> understanding the problem, I won't try to figure out any solution or
>>> change.
>> I need to read your ublk userspace codes carefully, if I made
> One small suggestion, you may start with the two builtin examples, both
> are simple & clean, and the big one is only 300+ LOC, really complicated?
>
>
> Thanks.
> Ming