* [PATCH 0/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies
@ 2026-10-06 8:55 Yoav Cohen
2026-10-06 8:55 ` [PATCH 1/2] " Yoav Cohen
2026-10-06 8:55 ` [PATCH 2/2] selftests: ublk: add test for rq_affinity completion redirection Yoav Cohen
0 siblings, 2 replies; 4+ messages in thread
From: Yoav Cohen @ 2026-10-06 8:55 UTC (permalink / raw)
To: Ming Lei, Jens Axboe, linux-block, csander; +Cc: jholzman, omril
ublk ends successfully completed requests inline on the server thread,
so rq_affinity never redirects completion work back to the submitting
CPU the way it does for NVMe, SCSI, virtio-blk, loop, nbd and rnbd.
Patch 1 fixes that and adds a feature flag so servers can detect
support. Patch 2 adds a selftest for it.
With an SPDK-style polling user-space block server at queue depth 96,
rq_affinity=2 gives 7-8% higher IOPS than today's always-inline
completion by offloading a saturated server CPU; rq_affinity=0 keeps
the old behavior for anyone who wants to opt out.
Yoav Cohen (2):
ublk: complete requests via blk_mq_complete_request() so rq_affinity
applies
selftests: ublk: add test for rq_affinity completion redirection
drivers/block/ublk_drv.c | 72 +++++++++++++++++--------
include/uapi/linux/ublk_cmd.h | 8 +++
tools/testing/selftests/ublk/Makefile | 1 +
tools/testing/selftests/ublk/kublk.c | 1 +
tools/testing/selftests/ublk/test_generic_18.sh | 42 +++++++++++++++
5 files changed, 103 insertions(+), 21 deletions(-)
create mode 100755 tools/testing/selftests/ublk/test_generic_18.sh
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies
2026-10-06 8:55 [PATCH 0/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies Yoav Cohen
@ 2026-10-06 8:55 ` Yoav Cohen
2026-10-06 14:56 ` Ming Lei
2026-10-06 8:55 ` [PATCH 2/2] selftests: ublk: add test for rq_affinity completion redirection Yoav Cohen
1 sibling, 1 reply; 4+ messages in thread
From: Yoav Cohen @ 2026-10-06 8:55 UTC (permalink / raw)
To: Ming Lei, Jens Axboe, linux-block, csander; +Cc: jholzman, omril, Yoav Cohen
ublk ends successfully completed requests inline on the server thread
that issued UBLK_IO_COMMIT_AND_FETCH_REQ, so rq_affinity never
redirects completion back to the submitting CPU, unlike NVMe, SCSI,
virtio-blk, loop, nbd and rnbd.
Route completion through blk_mq_complete_request_remote(), with a new
->complete() callback, ublk_end_rq(). Add UBLK_F_SUPPORT_RQ_AFFINITY
so servers can detect support via UBLK_CMD_GET_FEATURES.
Signed-off-by: Yoav Cohen <yoav@nvidia.com>
---
drivers/block/ublk_drv.c | 72 +++++++++++++++++++++++++----------
include/uapi/linux/ublk_cmd.h | 8 ++++
2 files changed, 59 insertions(+), 21 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 66eb55e7162e..48bdb5d782ee 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -90,7 +90,8 @@
| UBLK_F_BATCH_IO \
| UBLK_F_NO_AUTO_PART_SCAN \
| UBLK_F_SHMEM_ZC \
- | UBLK_F_IO_DESC_SIZE)
+ | UBLK_F_IO_DESC_SIZE \
+ | UBLK_F_SUPPORT_RQ_AFFINITY)
#define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
| UBLK_F_USER_RECOVERY_REISSUE \
@@ -1549,13 +1550,54 @@ static void ublk_end_request(struct request *req, blk_status_t error)
local_bh_enable();
}
+/*
+ * Update @req with its result and requeue it if it was only partially
+ * completed. Returns true if @req was requeued, in which case the caller
+ * must not touch it any further.
+ *
+ * Run bio->bi_end_io() with softirqs disabled. If the final fput happens
+ * off this path, then that will prevent ublk's blkdev_release() from
+ * being called on current's task work, see fput() implementation.
+ *
+ * This matters for the caller completing @req locally on the ublk
+ * server's own thread: it may already be holding disk->open_mutex, e.g.
+ * reading the partition table from bdev_open(), and an fput() running
+ * inline there could deadlock on it. Preferably we would not be doing
+ * IO with a mutex held that is also used for release, but this
+ * work-around will suffice for now. A caller reached instead via
+ * blk_mq_complete_request_remote()'s softirq/IPI redirect never runs on
+ * that thread, so it isn't exposed to this hazard, but disabling
+ * softirqs here is harmless for it too.
+ */
+static inline bool ublk_update_and_requeue(struct request *req,
+ struct ublk_io *io)
+{
+ bool requeue;
+
+ local_bh_disable();
+ requeue = blk_update_request(req, BLK_STS_OK, io->res);
+ local_bh_enable();
+ if (requeue)
+ blk_mq_requeue_request(req, true);
+ return requeue;
+}
+
+static void ublk_end_rq(struct request *req)
+{
+ struct ublk_queue *ubq = req->mq_hctx->driver_data;
+ struct ublk_io *io = &ubq->ios[req->tag];
+
+ if (!ublk_update_and_requeue(req, io) &&
+ likely(!blk_should_fake_timeout(req->q)))
+ __blk_mq_end_request(req, BLK_STS_OK);
+}
+
/* todo: handle partial completion */
static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
bool need_map, struct io_comp_batch *iob)
{
unsigned int unmapped_bytes;
blk_status_t res = BLK_STS_OK;
- bool requeue;
/* failed read IO if nothing is read */
if (!io->res && req_op(req) == REQ_OP_READ)
@@ -1588,25 +1630,11 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
io->res = unmapped_bytes;
}
- /*
- * Run bio->bi_end_io() with softirqs disabled. If the final fput
- * happens off this path, then that will prevent ublk's blkdev_release()
- * from being called on current's task work, see fput() implementation.
- *
- * Otherwise, ublk server may not provide forward progress in case of
- * reading the partition table from bdev_open() with disk->open_mutex
- * held, and causes dead lock as we could already be holding
- * disk->open_mutex here.
- *
- * Preferably we would not be doing IO with a mutex held that is also
- * used for release, but this work-around will suffice for now.
- */
- local_bh_disable();
- requeue = blk_update_request(req, BLK_STS_OK, io->res);
- local_bh_enable();
- if (requeue)
- blk_mq_requeue_request(req, true);
- else if (likely(!blk_should_fake_timeout(req->q))) {
+ if (blk_mq_complete_request_remote(req))
+ return;
+
+ if (!ublk_update_and_requeue(req, io) &&
+ likely(!blk_should_fake_timeout(req->q))) {
if (blk_mq_add_to_batch(req, iob, false, blk_mq_end_request_batch))
return;
__blk_mq_end_request(req, BLK_STS_OK);
@@ -2347,6 +2375,7 @@ static const struct blk_mq_ops ublk_mq_ops = {
.queue_rqs = ublk_queue_rqs,
.init_hctx = ublk_init_hctx,
.timeout = ublk_timeout,
+ .complete = ublk_end_rq,
};
static const struct blk_mq_ops ublk_batch_mq_ops = {
@@ -2355,6 +2384,7 @@ static const struct blk_mq_ops ublk_batch_mq_ops = {
.queue_rqs = ublk_batch_queue_rqs,
.init_hctx = ublk_init_hctx,
.timeout = ublk_timeout,
+ .complete = ublk_end_rq,
};
static void ublk_queue_reinit(struct ublk_device *ub, struct ublk_queue *ubq)
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index 33b25dd13965..47b5fba9519c 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -420,6 +420,14 @@ struct ublk_shmem_buf_reg {
/* ublksrv_io_desc size is specified by ublksrv_ctrl_dev_info's io_desc_size */
#define UBLK_F_IO_DESC_SIZE (1ULL << 20)
+/*
+ * Request completion honors the block device's rq_affinity setting
+ * (/sys/block/ublkbN/queue/rq_affinity): the submitting CPU's completion
+ * work can run there instead of always on the ublk server's CPU. Without
+ * this feature, rq_affinity has no effect on ublk devices.
+ */
+#define UBLK_F_SUPPORT_RQ_AFFINITY (1ULL << 21)
+
/* device state */
#define UBLK_S_DEV_DEAD 0
#define UBLK_S_DEV_LIVE 1
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] selftests: ublk: add test for rq_affinity completion redirection
2026-10-06 8:55 [PATCH 0/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies Yoav Cohen
2026-10-06 8:55 ` [PATCH 1/2] " Yoav Cohen
@ 2026-10-06 8:55 ` Yoav Cohen
1 sibling, 0 replies; 4+ messages in thread
From: Yoav Cohen @ 2026-10-06 8:55 UTC (permalink / raw)
To: Ming Lei, Jens Axboe, linux-block, csander; +Cc: jholzman, omril, Yoav Cohen
Add UBLK_F_SUPPORT_RQ_AFFINITY to kublk's feature table, and add
test_generic_18.sh: writes and verifies data on a loop device with
I/O pinned to one CPU while cycling rq_affinity through 0, 1 and 2.
Skipped on kernels without UBLK_F_SUPPORT_RQ_AFFINITY.
Signed-off-by: Yoav Cohen <yoav@nvidia.com>
---
tools/testing/selftests/ublk/Makefile | 1 +
tools/testing/selftests/ublk/kublk.c | 1 +
.../testing/selftests/ublk/test_generic_18.sh | 42 +++++++++++++++++++
3 files changed, 44 insertions(+)
create mode 100755 tools/testing/selftests/ublk/test_generic_18.sh
diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index 37883e9d50ec..d12e9879b89a 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -19,6 +19,7 @@ TEST_PROGS += test_generic_12.sh
TEST_PROGS += test_generic_13.sh
TEST_PROGS += test_generic_16.sh
TEST_PROGS += test_generic_17.sh
+TEST_PROGS += test_generic_18.sh
TEST_PROGS += test_batch_01.sh
TEST_PROGS += test_batch_02.sh
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 2400b4615766..c750e10a69bf 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -2083,6 +2083,7 @@ static int cmd_dev_get_features(void)
FEAT_NAME(UBLK_F_NO_AUTO_PART_SCAN),
FEAT_NAME(UBLK_F_SHMEM_ZC),
FEAT_NAME(UBLK_F_IO_DESC_SIZE),
+ FEAT_NAME(UBLK_F_SUPPORT_RQ_AFFINITY),
};
struct ublk_dev *dev;
__u64 features = 0;
diff --git a/tools/testing/selftests/ublk/test_generic_18.sh b/tools/testing/selftests/ublk/test_generic_18.sh
new file mode 100755
index 000000000000..5afb202f1ef5
--- /dev/null
+++ b/tools/testing/selftests/ublk/test_generic_18.sh
@@ -0,0 +1,42 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
+
+ERR_CODE=0
+
+if ! _have_program fio; then
+ exit "$UBLK_SKIP_CODE"
+fi
+
+if ! _have_feature "SUPPORT_RQ_AFFINITY"; then
+ exit "$UBLK_SKIP_CODE"
+fi
+
+_prep_test "loop" "write and verify I/O across rq_affinity settings"
+
+_create_backfile 0 256M
+
+dev_id=$(_add_ublk_dev -t loop "${UBLK_BACKFILES[0]}")
+_check_add_dev $TID $?
+
+rq_affinity_path=/sys/block/ublkb"${dev_id}"/queue/rq_affinity
+
+for affinity in 0 1 2; do
+ echo "${affinity}" > "${rq_affinity_path}"
+
+ # Pin submission to a single CPU so rq_affinity has somewhere to
+ # redirect completion to.
+ taskset -c 0 fio --name=verify --rw=randwrite --direct=1 \
+ --ioengine=libaio --bs=8k --iodepth=32 --verify=crc32c \
+ --do_verify=1 --verify_state_save=0 \
+ --filename=/dev/ublkb"${dev_id}" --size=256M > /dev/null
+ if [ $? -ne 0 ]; then
+ echo "fio verify failed with rq_affinity=${affinity}"
+ ERR_CODE=255
+ break
+ fi
+done
+
+_cleanup_test
+_show_result $TID $ERR_CODE
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies
2026-10-06 8:55 ` [PATCH 1/2] " Yoav Cohen
@ 2026-10-06 14:56 ` Ming Lei
0 siblings, 0 replies; 4+ messages in thread
From: Ming Lei @ 2026-10-06 14:56 UTC (permalink / raw)
To: Yoav Cohen; +Cc: Jens Axboe, linux-block, csander, jholzman, omril
On Tue, Oct 06, 2026 at 11:55:16AM +0300, Yoav Cohen wrote:
> ublk ends successfully completed requests inline on the server thread
> that issued UBLK_IO_COMMIT_AND_FETCH_REQ, so rq_affinity never
> redirects completion back to the submitting CPU, unlike NVMe, SCSI,
> virtio-blk, loop, nbd and rnbd.
>
> Route completion through blk_mq_complete_request_remote(), with a new
> ->complete() callback, ublk_end_rq(). Add UBLK_F_SUPPORT_RQ_AFFINITY
> so servers can detect support via UBLK_CMD_GET_FEATURES.
>
> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> ---
> drivers/block/ublk_drv.c | 72 +++++++++++++++++++++++++----------
> include/uapi/linux/ublk_cmd.h | 8 ++++
> 2 files changed, 59 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 66eb55e7162e..48bdb5d782ee 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -90,7 +90,8 @@
> | UBLK_F_BATCH_IO \
> | UBLK_F_NO_AUTO_PART_SCAN \
> | UBLK_F_SHMEM_ZC \
> - | UBLK_F_IO_DESC_SIZE)
> + | UBLK_F_IO_DESC_SIZE \
> + | UBLK_F_SUPPORT_RQ_AFFINITY)
>
> #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> | UBLK_F_USER_RECOVERY_REISSUE \
> @@ -1549,13 +1550,54 @@ static void ublk_end_request(struct request *req, blk_status_t error)
> local_bh_enable();
> }
>
> +/*
> + * Update @req with its result and requeue it if it was only partially
> + * completed. Returns true if @req was requeued, in which case the caller
> + * must not touch it any further.
> + *
> + * Run bio->bi_end_io() with softirqs disabled. If the final fput happens
> + * off this path, then that will prevent ublk's blkdev_release() from
> + * being called on current's task work, see fput() implementation.
> + *
> + * This matters for the caller completing @req locally on the ublk
> + * server's own thread: it may already be holding disk->open_mutex, e.g.
> + * reading the partition table from bdev_open(), and an fput() running
> + * inline there could deadlock on it. Preferably we would not be doing
> + * IO with a mutex held that is also used for release, but this
> + * work-around will suffice for now. A caller reached instead via
> + * blk_mq_complete_request_remote()'s softirq/IPI redirect never runs on
> + * that thread, so it isn't exposed to this hazard, but disabling
> + * softirqs here is harmless for it too.
> + */
> +static inline bool ublk_update_and_requeue(struct request *req,
> + struct ublk_io *io)
> +{
> + bool requeue;
> +
> + local_bh_disable();
> + requeue = blk_update_request(req, BLK_STS_OK, io->res);
> + local_bh_enable();
> + if (requeue)
> + blk_mq_requeue_request(req, true);
> + return requeue;
> +}
> +
> +static void ublk_end_rq(struct request *req)
> +{
> + struct ublk_queue *ubq = req->mq_hctx->driver_data;
> + struct ublk_io *io = &ubq->ios[req->tag];
> +
> + if (!ublk_update_and_requeue(req, io) &&
> + likely(!blk_should_fake_timeout(req->q)))
> + __blk_mq_end_request(req, BLK_STS_OK);
> +}
> +
> /* todo: handle partial completion */
> static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
> bool need_map, struct io_comp_batch *iob)
> {
> unsigned int unmapped_bytes;
> blk_status_t res = BLK_STS_OK;
> - bool requeue;
>
> /* failed read IO if nothing is read */
> if (!io->res && req_op(req) == REQ_OP_READ)
> @@ -1588,25 +1630,11 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
> io->res = unmapped_bytes;
> }
>
> - /*
> - * Run bio->bi_end_io() with softirqs disabled. If the final fput
> - * happens off this path, then that will prevent ublk's blkdev_release()
> - * from being called on current's task work, see fput() implementation.
> - *
> - * Otherwise, ublk server may not provide forward progress in case of
> - * reading the partition table from bdev_open() with disk->open_mutex
> - * held, and causes dead lock as we could already be holding
> - * disk->open_mutex here.
> - *
> - * Preferably we would not be doing IO with a mutex held that is also
> - * used for release, but this work-around will suffice for now.
> - */
> - local_bh_disable();
> - requeue = blk_update_request(req, BLK_STS_OK, io->res);
> - local_bh_enable();
> - if (requeue)
> - blk_mq_requeue_request(req, true);
> - else if (likely(!blk_should_fake_timeout(req->q))) {
> + if (blk_mq_complete_request_remote(req))
> + return;
Only copy-mode READs get the new completion path. In __ublk_complete_rq(), anything
that needs no copy back to the request jumps to exit: ublk_end_request(), which
completes inline as before:
- all WRITEs, flush and discard;
- all USER_COPY, ZERO_COPY, AUTO_BUF_REG and shmem zero-copy I/O;
- every error.
Only a successful READ in copy mode reaches the new blk_mq_complete_request_remote()
call. So the uapi comment "Request completion honors … rq_affinity" is false for most I/O.
> +
> + if (!ublk_update_and_requeue(req, io) &&
> + likely(!blk_should_fake_timeout(req->q))) {
> if (blk_mq_add_to_batch(req, iob, false, blk_mq_end_request_batch))
> return;
> __blk_mq_end_request(req, BLK_STS_OK);
> @@ -2347,6 +2375,7 @@ static const struct blk_mq_ops ublk_mq_ops = {
> .queue_rqs = ublk_queue_rqs,
> .init_hctx = ublk_init_hctx,
> .timeout = ublk_timeout,
> + .complete = ublk_end_rq,
> };
>
> static const struct blk_mq_ops ublk_batch_mq_ops = {
> @@ -2355,6 +2384,7 @@ static const struct blk_mq_ops ublk_batch_mq_ops = {
> .queue_rqs = ublk_batch_queue_rqs,
> .init_hctx = ublk_init_hctx,
> .timeout = ublk_timeout,
> + .complete = ublk_end_rq,
> };
>
> static void ublk_queue_reinit(struct ublk_device *ub, struct ublk_queue *ubq)
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index 33b25dd13965..47b5fba9519c 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -420,6 +420,14 @@ struct ublk_shmem_buf_reg {
> /* ublksrv_io_desc size is specified by ublksrv_ctrl_dev_info's io_desc_size */
> #define UBLK_F_IO_DESC_SIZE (1ULL << 20)
>
> +/*
> + * Request completion honors the block device's rq_affinity setting
> + * (/sys/block/ublkbN/queue/rq_affinity): the submitting CPU's completion
> + * work can run there instead of always on the ublk server's CPU. Without
> + * this feature, rq_affinity has no effect on ublk devices.
> + */
> +#define UBLK_F_SUPPORT_RQ_AFFINITY (1ULL << 21)
This flag should be checked somewhere given it is passed from userspace,
otherwise it can be nop.
Thanks,
Ming
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-06 14:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 8:55 [PATCH 0/2] ublk: complete requests via blk_mq_complete_request() so rq_affinity applies Yoav Cohen
2026-10-06 8:55 ` [PATCH 1/2] " Yoav Cohen
2026-10-06 14:56 ` Ming Lei
2026-10-06 8:55 ` [PATCH 2/2] selftests: ublk: add test for rq_affinity completion redirection Yoav Cohen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox