* [PATCH v2 0/2] ublk: honor rq_affinity on request completion
@ 2026-10-06 19:03 Yoav Cohen
2026-10-06 19:03 ` [PATCH v2 1/2] " Yoav Cohen
2026-10-06 19:03 ` [PATCH v2 2/2] selftests: ublk: add rq_affinity test Yoav Cohen
0 siblings, 2 replies; 7+ messages in thread
From: Yoav Cohen @ 2026-10-06 19:03 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,
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.
v2, addressing Ming Lei's review of v1:
- v1 only redirected the classic-copy-mode successful-read completion;
every write, flush/discard, error, and zero-copy/user-copy/
auto-buf-reg/shmem-zc I/O still completed inline as before, so
rq_affinity had no effect for most traffic. ublk_end_rq() now
re-derives which completion path __ublk_complete_rq() took from
@req/@io state, so every outcome goes through
blk_mq_complete_request_remote().
- v1 ran the new completion path unconditionally on any patched
kernel. v2 adds UBLK_F_SUPPORT_RQ_AFFINITY, which a server must
request at ADD_DEV time; without it, behavior is unchanged.
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: honor rq_affinity on request completion
selftests: ublk: add rq_affinity test
drivers/block/ublk_drv.c | 98 ++++++++++++++-----
include/uapi/linux/ublk_cmd.h | 8 ++
tools/testing/selftests/ublk/Makefile | 1 +
tools/testing/selftests/ublk/kublk.c | 6 +-
.../testing/selftests/ublk/test_generic_18.sh | 42 ++++++++
5 files changed, 132 insertions(+), 23 deletions(-)
create mode 100755 tools/testing/selftests/ublk/test_generic_18.sh
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] ublk: honor rq_affinity on request completion
2026-10-06 19:03 [PATCH v2 0/2] ublk: honor rq_affinity on request completion Yoav Cohen
@ 2026-10-06 19:03 ` Yoav Cohen
2026-10-06 21:53 ` Caleb Sander Mateos
2026-10-09 14:04 ` Ming Lei
2026-10-06 19:03 ` [PATCH v2 2/2] selftests: ublk: add rq_affinity test Yoav Cohen
1 sibling, 2 replies; 7+ messages in thread
From: Yoav Cohen @ 2026-10-06 19:03 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(). This covers every completion
outcome -- reads, writes, flush/discard, errors, and the zero-copy/
user-copy/auto-buf-reg/shmem-zc paths that skip the copy-back step --
not just the classic-copy-mode successful-read case, since
ublk_end_rq() re-derives which of those __ublk_complete_rq() took from
@req and @io state rather than assuming one.
Gate it behind a new UBLK_F_SUPPORT_RQ_AFFINITY flag that a server
must request at ADD_DEV time; servers that don't ask for it keep
today's always-local completion unchanged. Add ublk_support_rq_affinity()
alongside the existing ublk_support_*() helpers, and check it before
calling blk_mq_complete_request_remote() at both call sites.
Signed-off-by: Yoav Cohen <yoav@nvidia.com>
---
drivers/block/ublk_drv.c | 98 +++++++++++++++++++++++++++--------
include/uapi/linux/ublk_cmd.h | 8 +++
2 files changed, 84 insertions(+), 22 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 66eb55e7162e..b94293296b73 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 \
@@ -452,6 +453,11 @@ static inline bool ublk_support_user_copy(const struct ublk_queue *ubq)
return ubq->flags & UBLK_F_USER_COPY;
}
+static inline bool ublk_support_rq_affinity(const struct ublk_queue *ubq)
+{
+ return ubq->flags & UBLK_F_SUPPORT_RQ_AFFINITY;
+}
+
static inline bool ublk_dev_support_user_copy(const struct ublk_device *ub)
{
return ub->dev_info.flags & UBLK_F_USER_COPY;
@@ -1468,6 +1474,14 @@ static inline bool ublk_need_unmap_req(const struct request *req)
(req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
}
+/* Whether __ublk_complete_rq() takes the copy-back/partial-completion path */
+static inline bool ublk_rq_need_unmap(const struct ublk_queue *ubq,
+ struct request *req)
+{
+ return ublk_need_map_io(ubq) && ublk_need_unmap_req(req) &&
+ !ublk_iod_is_shmem_zc(ubq, req->tag);
+}
+
static unsigned int ublk_map_io(const struct request *req,
const struct ublk_io *io)
{
@@ -1549,13 +1563,62 @@ 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];
+
+ /* matches the two outcomes __ublk_complete_rq() may have redirected */
+ if (io->res >= 0 && ublk_rq_need_unmap(ubq, req)) {
+ if (!ublk_update_and_requeue(req, io) &&
+ likely(!blk_should_fake_timeout(req->q)))
+ __blk_mq_end_request(req, BLK_STS_OK);
+ return;
+ }
+
+ ublk_end_request(req, io->res < 0 ? errno_to_blk_status(io->res) :
+ 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)
{
+ struct ublk_queue *ubq = req->mq_hctx->driver_data;
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)
@@ -1568,7 +1631,7 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
/* shmem zero copy: no data to unmap, pages already shared */
if (!need_map || !ublk_need_unmap_req(req) ||
- ublk_iod_is_shmem_zc(req->mq_hctx->driver_data, req->tag))
+ ublk_iod_is_shmem_zc(ubq, req->tag))
goto exit;
/* for READ request, writing data in iod->addr to rq buffers */
@@ -1582,31 +1645,18 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
if (unlikely(unmapped_bytes < io->res)) {
if (unlikely(!unmapped_bytes)) {
res = BLK_STS_IOERR;
+ io->res = -EIO;
goto exit;
}
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 (ublk_support_rq_affinity(ubq) && 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);
@@ -1614,6 +1664,8 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
return;
exit:
+ if (ublk_support_rq_affinity(ubq) && blk_mq_complete_request_remote(req))
+ return;
ublk_end_request(req, res);
}
@@ -2347,6 +2399,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 +2408,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] 7+ messages in thread
* [PATCH v2 2/2] selftests: ublk: add rq_affinity test
2026-10-06 19:03 [PATCH v2 0/2] ublk: honor rq_affinity on request completion Yoav Cohen
2026-10-06 19:03 ` [PATCH v2 1/2] " Yoav Cohen
@ 2026-10-06 19:03 ` Yoav Cohen
1 sibling, 0 replies; 7+ messages in thread
From: Yoav Cohen @ 2026-10-06 19:03 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 a
--rq_affinity flag to request it at ADD_DEV time, 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 | 6 ++-
.../testing/selftests/ublk/test_generic_18.sh | 42 +++++++++++++++++++
3 files changed, 48 insertions(+), 1 deletion(-)
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..e3afef0e7628 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;
@@ -2180,7 +2181,7 @@ static void __cmd_create_help(char *exe, bool recovery)
printf("\t[--nthreads threads] [--per_io_tasks]\n");
printf("\t[--integrity_capable] [--integrity_reftag] [--metadata_size SIZE] "
"[--pi_offset OFFSET] [--csum_type ip|t10dif|nvme] [--tag_size SIZE]\n");
- printf("\t[--batch|-b] [--rotate_auto_buf] [--no_auto_part_scan]\n");
+ printf("\t[--batch|-b] [--rotate_auto_buf] [--no_auto_part_scan] [--rq_affinity]\n");
printf("\t[--io_desc_size SIZE]\n");
printf("\t[target options] [backfile1] [backfile2] ...\n");
printf("\tdefault: nr_queues=2(max 32), depth=128(max 1024), dev_id=-1(auto allocation)\n");
@@ -2260,6 +2261,7 @@ int main(int argc, char *argv[])
{ "batch", 0, NULL, 'b'},
{ "rotate_auto_buf", 0, NULL, 0 },
{ "no_auto_part_scan", 0, NULL, 0 },
+ { "rq_affinity", 0, NULL, 0 },
{ "shmem_zc", 0, NULL, 0 },
{ "htlb", 1, NULL, 0 },
{ "rdonly_shmem_buf", 0, NULL, 0 },
@@ -2407,6 +2409,8 @@ int main(int argc, char *argv[])
ctx.safe_stop = 1;
if (!strcmp(longopts[option_idx].name, "no_auto_part_scan"))
ctx.flags |= UBLK_F_NO_AUTO_PART_SCAN;
+ if (!strcmp(longopts[option_idx].name, "rq_affinity"))
+ ctx.flags |= UBLK_F_SUPPORT_RQ_AFFINITY;
if (!strcmp(longopts[option_idx].name, "shmem_zc"))
ctx.flags |= UBLK_F_SHMEM_ZC;
if (!strcmp(longopts[option_idx].name, "htlb"))
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..c899a5bf0364
--- /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 --rq_affinity "${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] 7+ messages in thread
* Re: [PATCH v2 1/2] ublk: honor rq_affinity on request completion
2026-10-06 19:03 ` [PATCH v2 1/2] " Yoav Cohen
@ 2026-10-06 21:53 ` Caleb Sander Mateos
2026-10-09 13:43 ` Ming Lei
2026-10-09 14:04 ` Ming Lei
1 sibling, 1 reply; 7+ messages in thread
From: Caleb Sander Mateos @ 2026-10-06 21:53 UTC (permalink / raw)
To: Yoav Cohen; +Cc: Ming Lei, Jens Axboe, linux-block, jholzman, omril
On Tue, Oct 6, 2026 at 12:04 PM Yoav Cohen <yoav@nvidia.com> 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(). This covers every completion
> outcome -- reads, writes, flush/discard, errors, and the zero-copy/
> user-copy/auto-buf-reg/shmem-zc paths that skip the copy-back step --
> not just the classic-copy-mode successful-read case, since
> ublk_end_rq() re-derives which of those __ublk_complete_rq() took from
> @req and @io state rather than assuming one.
>
> Gate it behind a new UBLK_F_SUPPORT_RQ_AFFINITY flag that a server
> must request at ADD_DEV time; servers that don't ask for it keep
> today's always-local completion unchanged. Add ublk_support_rq_affinity()
> alongside the existing ublk_support_*() helpers, and check it before
> calling blk_mq_complete_request_remote() at both call sites.
>
> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> ---
> drivers/block/ublk_drv.c | 98 +++++++++++++++++++++++++++--------
> include/uapi/linux/ublk_cmd.h | 8 +++
> 2 files changed, 84 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 66eb55e7162e..b94293296b73 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 \
> @@ -452,6 +453,11 @@ static inline bool ublk_support_user_copy(const struct ublk_queue *ubq)
> return ubq->flags & UBLK_F_USER_COPY;
> }
>
> +static inline bool ublk_support_rq_affinity(const struct ublk_queue *ubq)
> +{
> + return ubq->flags & UBLK_F_SUPPORT_RQ_AFFINITY;
> +}
> +
> static inline bool ublk_dev_support_user_copy(const struct ublk_device *ub)
> {
> return ub->dev_info.flags & UBLK_F_USER_COPY;
> @@ -1468,6 +1474,14 @@ static inline bool ublk_need_unmap_req(const struct request *req)
> (req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
> }
>
> +/* Whether __ublk_complete_rq() takes the copy-back/partial-completion path */
> +static inline bool ublk_rq_need_unmap(const struct ublk_queue *ubq,
> + struct request *req)
> +{
> + return ublk_need_map_io(ubq) && ublk_need_unmap_req(req) &&
> + !ublk_iod_is_shmem_zc(ubq, req->tag);
> +}
> +
> static unsigned int ublk_map_io(const struct request *req,
> const struct ublk_io *io)
> {
> @@ -1549,13 +1563,62 @@ 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];
> +
> + /* matches the two outcomes __ublk_complete_rq() may have redirected */
> + if (io->res >= 0 && ublk_rq_need_unmap(ubq, req)) {
> + if (!ublk_update_and_requeue(req, io) &&
> + likely(!blk_should_fake_timeout(req->q)))
> + __blk_mq_end_request(req, BLK_STS_OK);
> + return;
> + }
> +
> + ublk_end_request(req, io->res < 0 ? errno_to_blk_status(io->res) :
> + 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)
> {
> + struct ublk_queue *ubq = req->mq_hctx->driver_data;
mq_hctx->driver_data and ubq->flags are likely to both be cache misses
on a ublk server thread servicing many queues. req->mq_hctx is
probably a hit as it's in the same cache line as req->cmd_flags
accessed just before in ublk_ch_uring_cmd_local() for the req_op(req)
== REQ_OP_ZONE_APPEND check. It would be nice to avoid these
additional dereferences, can we prevent queue_rq_affinity_store() from
setting rq_affinity to a value other than 0 if
UBLK_F_SUPPORT_RQ_AFFINITY isn't enabled? Perhaps indicated via a new
BLK_FEAT_SUPPORTS_RQ_AFFINITY flag in struct queue_limits's features
field?
Best,
Caleb
> 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)
> @@ -1568,7 +1631,7 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
>
> /* shmem zero copy: no data to unmap, pages already shared */
> if (!need_map || !ublk_need_unmap_req(req) ||
> - ublk_iod_is_shmem_zc(req->mq_hctx->driver_data, req->tag))
> + ublk_iod_is_shmem_zc(ubq, req->tag))
> goto exit;
>
> /* for READ request, writing data in iod->addr to rq buffers */
> @@ -1582,31 +1645,18 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
> if (unlikely(unmapped_bytes < io->res)) {
> if (unlikely(!unmapped_bytes)) {
> res = BLK_STS_IOERR;
> + io->res = -EIO;
> goto exit;
> }
>
> 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 (ublk_support_rq_affinity(ubq) && 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);
> @@ -1614,6 +1664,8 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
>
> return;
> exit:
> + if (ublk_support_rq_affinity(ubq) && blk_mq_complete_request_remote(req))
> + return;
> ublk_end_request(req, res);
> }
>
> @@ -2347,6 +2399,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 +2408,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 [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] ublk: honor rq_affinity on request completion
2026-10-06 21:53 ` Caleb Sander Mateos
@ 2026-10-09 13:43 ` Ming Lei
2026-10-09 15:25 ` Caleb Sander Mateos
0 siblings, 1 reply; 7+ messages in thread
From: Ming Lei @ 2026-10-09 13:43 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Yoav Cohen, Jens Axboe, linux-block, jholzman, omril
On Tue, Oct 06, 2026 at 02:53:47PM -0700, Caleb Sander Mateos wrote:
> On Tue, Oct 6, 2026 at 12:04 PM Yoav Cohen <yoav@nvidia.com> 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(). This covers every completion
> > outcome -- reads, writes, flush/discard, errors, and the zero-copy/
> > user-copy/auto-buf-reg/shmem-zc paths that skip the copy-back step --
> > not just the classic-copy-mode successful-read case, since
> > ublk_end_rq() re-derives which of those __ublk_complete_rq() took from
> > @req and @io state rather than assuming one.
> >
> > Gate it behind a new UBLK_F_SUPPORT_RQ_AFFINITY flag that a server
> > must request at ADD_DEV time; servers that don't ask for it keep
> > today's always-local completion unchanged. Add ublk_support_rq_affinity()
> > alongside the existing ublk_support_*() helpers, and check it before
> > calling blk_mq_complete_request_remote() at both call sites.
> >
> > Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> > ---
> > drivers/block/ublk_drv.c | 98 +++++++++++++++++++++++++++--------
> > include/uapi/linux/ublk_cmd.h | 8 +++
> > 2 files changed, 84 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 66eb55e7162e..b94293296b73 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 \
> > @@ -452,6 +453,11 @@ static inline bool ublk_support_user_copy(const struct ublk_queue *ubq)
> > return ubq->flags & UBLK_F_USER_COPY;
> > }
> >
> > +static inline bool ublk_support_rq_affinity(const struct ublk_queue *ubq)
> > +{
> > + return ubq->flags & UBLK_F_SUPPORT_RQ_AFFINITY;
> > +}
> > +
> > static inline bool ublk_dev_support_user_copy(const struct ublk_device *ub)
> > {
> > return ub->dev_info.flags & UBLK_F_USER_COPY;
> > @@ -1468,6 +1474,14 @@ static inline bool ublk_need_unmap_req(const struct request *req)
> > (req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
> > }
> >
> > +/* Whether __ublk_complete_rq() takes the copy-back/partial-completion path */
> > +static inline bool ublk_rq_need_unmap(const struct ublk_queue *ubq,
> > + struct request *req)
> > +{
> > + return ublk_need_map_io(ubq) && ublk_need_unmap_req(req) &&
> > + !ublk_iod_is_shmem_zc(ubq, req->tag);
> > +}
> > +
> > static unsigned int ublk_map_io(const struct request *req,
> > const struct ublk_io *io)
> > {
> > @@ -1549,13 +1563,62 @@ 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];
> > +
> > + /* matches the two outcomes __ublk_complete_rq() may have redirected */
> > + if (io->res >= 0 && ublk_rq_need_unmap(ubq, req)) {
> > + if (!ublk_update_and_requeue(req, io) &&
> > + likely(!blk_should_fake_timeout(req->q)))
> > + __blk_mq_end_request(req, BLK_STS_OK);
> > + return;
> > + }
> > +
> > + ublk_end_request(req, io->res < 0 ? errno_to_blk_status(io->res) :
> > + 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)
> > {
> > + struct ublk_queue *ubq = req->mq_hctx->driver_data;
>
> mq_hctx->driver_data and ubq->flags are likely to both be cache misses
> on a ublk server thread servicing many queues. req->mq_hctx is
> probably a hit as it's in the same cache line as req->cmd_flags
> accessed just before in ublk_ch_uring_cmd_local() for the req_op(req)
> == REQ_OP_ZONE_APPEND check. It would be nice to avoid these
> additional dereferences, can we prevent queue_rq_affinity_store() from
> setting rq_affinity to a value other than 0 if
> UBLK_F_SUPPORT_RQ_AFFINITY isn't enabled? Perhaps indicated via a new
> BLK_FEAT_SUPPORTS_RQ_AFFINITY flag in struct queue_limits's features
> field?
ubq is only for ublk_support_rq_affinity(ubq), `maybe rq_affinity` can be
passed by __ublk_complete_rq() parameter via
ublk_dev_support_rq_affinity(ub).
BTW, the one for ublk_iod_is_shmem_zc() can't be avoided.
Thanks,
Ming
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] ublk: honor rq_affinity on request completion
2026-10-06 19:03 ` [PATCH v2 1/2] " Yoav Cohen
2026-10-06 21:53 ` Caleb Sander Mateos
@ 2026-10-09 14:04 ` Ming Lei
1 sibling, 0 replies; 7+ messages in thread
From: Ming Lei @ 2026-10-09 14:04 UTC (permalink / raw)
To: Yoav Cohen; +Cc: Jens Axboe, linux-block, csander, jholzman, omril
On Tue, Oct 06, 2026 at 10:03:47PM +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(). This covers every completion
> outcome -- reads, writes, flush/discard, errors, and the zero-copy/
> user-copy/auto-buf-reg/shmem-zc paths that skip the copy-back step --
> not just the classic-copy-mode successful-read case, since
> ublk_end_rq() re-derives which of those __ublk_complete_rq() took from
> @req and @io state rather than assuming one.
>
> Gate it behind a new UBLK_F_SUPPORT_RQ_AFFINITY flag that a server
> must request at ADD_DEV time; servers that don't ask for it keep
> today's always-local completion unchanged. Add ublk_support_rq_affinity()
> alongside the existing ublk_support_*() helpers, and check it before
> calling blk_mq_complete_request_remote() at both call sites.
>
> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> ---
> drivers/block/ublk_drv.c | 98 +++++++++++++++++++++++++++--------
> include/uapi/linux/ublk_cmd.h | 8 +++
> 2 files changed, 84 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 66eb55e7162e..b94293296b73 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 \
> @@ -452,6 +453,11 @@ static inline bool ublk_support_user_copy(const struct ublk_queue *ubq)
> return ubq->flags & UBLK_F_USER_COPY;
> }
>
> +static inline bool ublk_support_rq_affinity(const struct ublk_queue *ubq)
> +{
> + return ubq->flags & UBLK_F_SUPPORT_RQ_AFFINITY;
> +}
> +
> static inline bool ublk_dev_support_user_copy(const struct ublk_device *ub)
> {
> return ub->dev_info.flags & UBLK_F_USER_COPY;
> @@ -1468,6 +1474,14 @@ static inline bool ublk_need_unmap_req(const struct request *req)
> (req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
> }
>
> +/* Whether __ublk_complete_rq() takes the copy-back/partial-completion path */
> +static inline bool ublk_rq_need_unmap(const struct ublk_queue *ubq,
> + struct request *req)
> +{
> + return ublk_need_map_io(ubq) && ublk_need_unmap_req(req) &&
> + !ublk_iod_is_shmem_zc(ubq, req->tag);
> +}
> +
> static unsigned int ublk_map_io(const struct request *req,
> const struct ublk_io *io)
> {
> @@ -1549,13 +1563,62 @@ 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];
> +
> + /* matches the two outcomes __ublk_complete_rq() may have redirected */
> + if (io->res >= 0 && ublk_rq_need_unmap(ubq, req)) {
> + if (!ublk_update_and_requeue(req, io) &&
> + likely(!blk_should_fake_timeout(req->q)))
> + __blk_mq_end_request(req, BLK_STS_OK);
> + return;
> + }
ublk_end_rq() may be run from remove cpu, then it becomes fragile to
touch `io` here, and not efficient to touch `ubq` as Caleb mentioned.
It could be simplified a lot to just deal with normal io->res == blk_rq_bytes(req)
here, meantime keep everything else(short write, failure, ...) in __ublk_complete_rq().
Thanks,
Ming
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] ublk: honor rq_affinity on request completion
2026-10-09 13:43 ` Ming Lei
@ 2026-10-09 15:25 ` Caleb Sander Mateos
0 siblings, 0 replies; 7+ messages in thread
From: Caleb Sander Mateos @ 2026-10-09 15:25 UTC (permalink / raw)
To: Ming Lei; +Cc: Yoav Cohen, Jens Axboe, linux-block, jholzman, omril
On Fri, Oct 9, 2026 at 6:43 AM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Tue, Oct 06, 2026 at 02:53:47PM -0700, Caleb Sander Mateos wrote:
> > On Tue, Oct 6, 2026 at 12:04 PM Yoav Cohen <yoav@nvidia.com> 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(). This covers every completion
> > > outcome -- reads, writes, flush/discard, errors, and the zero-copy/
> > > user-copy/auto-buf-reg/shmem-zc paths that skip the copy-back step --
> > > not just the classic-copy-mode successful-read case, since
> > > ublk_end_rq() re-derives which of those __ublk_complete_rq() took from
> > > @req and @io state rather than assuming one.
> > >
> > > Gate it behind a new UBLK_F_SUPPORT_RQ_AFFINITY flag that a server
> > > must request at ADD_DEV time; servers that don't ask for it keep
> > > today's always-local completion unchanged. Add ublk_support_rq_affinity()
> > > alongside the existing ublk_support_*() helpers, and check it before
> > > calling blk_mq_complete_request_remote() at both call sites.
> > >
> > > Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> > > ---
> > > drivers/block/ublk_drv.c | 98 +++++++++++++++++++++++++++--------
> > > include/uapi/linux/ublk_cmd.h | 8 +++
> > > 2 files changed, 84 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index 66eb55e7162e..b94293296b73 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 \
> > > @@ -452,6 +453,11 @@ static inline bool ublk_support_user_copy(const struct ublk_queue *ubq)
> > > return ubq->flags & UBLK_F_USER_COPY;
> > > }
> > >
> > > +static inline bool ublk_support_rq_affinity(const struct ublk_queue *ubq)
> > > +{
> > > + return ubq->flags & UBLK_F_SUPPORT_RQ_AFFINITY;
> > > +}
> > > +
> > > static inline bool ublk_dev_support_user_copy(const struct ublk_device *ub)
> > > {
> > > return ub->dev_info.flags & UBLK_F_USER_COPY;
> > > @@ -1468,6 +1474,14 @@ static inline bool ublk_need_unmap_req(const struct request *req)
> > > (req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
> > > }
> > >
> > > +/* Whether __ublk_complete_rq() takes the copy-back/partial-completion path */
> > > +static inline bool ublk_rq_need_unmap(const struct ublk_queue *ubq,
> > > + struct request *req)
> > > +{
> > > + return ublk_need_map_io(ubq) && ublk_need_unmap_req(req) &&
> > > + !ublk_iod_is_shmem_zc(ubq, req->tag);
> > > +}
> > > +
> > > static unsigned int ublk_map_io(const struct request *req,
> > > const struct ublk_io *io)
> > > {
> > > @@ -1549,13 +1563,62 @@ 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];
> > > +
> > > + /* matches the two outcomes __ublk_complete_rq() may have redirected */
> > > + if (io->res >= 0 && ublk_rq_need_unmap(ubq, req)) {
> > > + if (!ublk_update_and_requeue(req, io) &&
> > > + likely(!blk_should_fake_timeout(req->q)))
> > > + __blk_mq_end_request(req, BLK_STS_OK);
> > > + return;
> > > + }
> > > +
> > > + ublk_end_request(req, io->res < 0 ? errno_to_blk_status(io->res) :
> > > + 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)
> > > {
> > > + struct ublk_queue *ubq = req->mq_hctx->driver_data;
> >
> > mq_hctx->driver_data and ubq->flags are likely to both be cache misses
> > on a ublk server thread servicing many queues. req->mq_hctx is
> > probably a hit as it's in the same cache line as req->cmd_flags
> > accessed just before in ublk_ch_uring_cmd_local() for the req_op(req)
> > == REQ_OP_ZONE_APPEND check. It would be nice to avoid these
> > additional dereferences, can we prevent queue_rq_affinity_store() from
> > setting rq_affinity to a value other than 0 if
> > UBLK_F_SUPPORT_RQ_AFFINITY isn't enabled? Perhaps indicated via a new
> > BLK_FEAT_SUPPORTS_RQ_AFFINITY flag in struct queue_limits's features
> > field?
>
> ubq is only for ublk_support_rq_affinity(ubq), `maybe rq_affinity` can be
> passed by __ublk_complete_rq() parameter via
> ublk_dev_support_rq_affinity(ub).
>
> BTW, the one for ublk_iod_is_shmem_zc() can't be avoided.
Yes, but ublk_iod_is_shmem_zc() is only called in the need_map case. I
would expect most modern ublk servers to enable user copy and/or zero
copy, so they wouldn't take that path.
Best,
Caleb
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-09 15:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 19:03 [PATCH v2 0/2] ublk: honor rq_affinity on request completion Yoav Cohen
2026-10-06 19:03 ` [PATCH v2 1/2] " Yoav Cohen
2026-10-06 21:53 ` Caleb Sander Mateos
2026-10-09 13:43 ` Ming Lei
2026-10-09 15:25 ` Caleb Sander Mateos
2026-10-09 14:04 ` Ming Lei
2026-10-06 19:03 ` [PATCH v2 2/2] selftests: ublk: add rq_affinity test Yoav Cohen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.