* [PATCH hci-8.1 v2 0/2] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls
@ 2026-10-27 12:47 Alexander Ivanov
2026-10-27 12:47 ` [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with " Alexander Ivanov
2026-10-27 12:47 ` [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
0 siblings, 2 replies; 9+ messages in thread
From: Alexander Ivanov @ 2026-10-27 12:47 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core
The kernel drops our VHOST_SET_NWORKERS ioctl in favour of the
mainstream VHOST_NEW_WORKER/VHOST_ATTACH_VRING_WORKER, which vhost-blk
now supports.
Patch 1 makes vhost-blk create its num-threads workers with the
mainstream ioctls, keeping the same worker layout, and fall back to a
single thread on a kernel without them. Patch 2 drops
VHOST_SET_NWORKERS from linux-headers.
v2:
- patch 1: clamp num-threads to VHOST_BLK_MAX_QUEUES
- patch 1: note in the commit message that our kernel, unlike
mainline, defaults to kthreads
https://virtuozzo.atlassian.net/browse/VSTOR-107537
Alexander Ivanov (2):
vhost-blk: create kernel threads with the mainstream worker ioctls
linux-headers: drop VHOST_SET_NWORKERS
hw/block/vhost-blk.c | 76 +++++++++++++++++++++++++++++++++++--
linux-headers/linux/vhost.h | 9 -----
2 files changed, 72 insertions(+), 13 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls
2026-10-27 12:47 [PATCH hci-8.1 v2 0/2] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
@ 2026-10-27 12:47 ` Alexander Ivanov
2026-10-27 12:52 ` no-reply
2026-10-27 14:22 ` Andrey Drobyshev
2026-10-27 12:47 ` [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
1 sibling, 2 replies; 9+ messages in thread
From: Alexander Ivanov @ 2026-10-27 12:47 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core
num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl.
The kernel is dropping it in favour of the mainstream worker ioctls,
which vhost-blk now supports, so use those: create num-threads - 1
workers with VHOST_NEW_WORKER and attach them to the virtqueues
round-robin with VHOST_ATTACH_VRING_WORKER, the default worker being
the first one. This is the layout VHOST_SET_NWORKERS produced.
VHOST_NEW_WORKER works only in vhost_task mode, while our kernel,
unlike mainline, defaults to kthreads, so switch the device to
vhost_task mode with VHOST_SET_FORK_FROM_OWNER before VHOST_SET_OWNER.
VHOST_RESET_OWNER restores the default, hence this is done on every
start. With num-threads=1 nothing changes: no worker ioctls, kthread
mode.
On a kernel without these ioctls warn and run a single thread rather
than fail to start vhost-blk.
https://virtuozzo.atlassian.net/browse/VSTOR-107537
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
hw/block/vhost-blk.c | 76 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 72 insertions(+), 4 deletions(-)
diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
index abddff02c6d..7b427083478 100644
--- a/hw/block/vhost-blk.c
+++ b/hw/block/vhost-blk.c
@@ -33,6 +33,52 @@
#include "migration/cpr.h"
#include "system/runstate.h"
+/*
+ * Spread the virtqueues round-robin over num-threads workers, the first one
+ * being the default worker that VHOST_SET_OWNER created.
+ */
+static int vhost_blk_set_workers(VHostBlk *s, int nworkers)
+{
+ uint32_t ids[VHOST_BLK_MAX_QUEUES];
+ struct vhost_worker_state worker;
+ struct vhost_vring_worker vq_worker;
+ int ret, i;
+
+ nworkers = MIN(nworkers, VHOST_BLK_MAX_QUEUES);
+ for (i = 1; i < nworkers; i++) {
+ memset(&worker, 0, sizeof(worker));
+ if (ioctl(s->vhostfd, VHOST_NEW_WORKER, &worker)) {
+ ret = -errno;
+ if (ret == -ENOTTY && i == 1) {
+ warn_report_once("vhost-blk: kernel does not support worker "
+ "ioctls, using a single kernel thread");
+ return 0;
+ }
+ error_report("vhost-blk: unable to create kernel thread: %s",
+ strerror(-ret));
+ return ret;
+ }
+ ids[i] = worker.worker_id;
+ }
+
+ for (i = 0; i < s->dev.nvqs; i++) {
+ if (i % nworkers == 0) {
+ continue;
+ }
+ memset(&vq_worker, 0, sizeof(vq_worker));
+ vq_worker.index = i;
+ vq_worker.worker_id = ids[i % nworkers];
+ if (ioctl(s->vhostfd, VHOST_ATTACH_VRING_WORKER, &vq_worker)) {
+ ret = -errno;
+ error_report("vhost-blk: unable to attach kernel thread to "
+ "queue %d: %s", i, strerror(-ret));
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int vhost_blk_start(VirtIODevice *vdev)
{
VHostBlk *s = VHOST_BLK(vdev);
@@ -51,15 +97,37 @@ static int vhost_blk_start(VirtIODevice *vdev)
return 0;
}
+ nworkers = MIN(s->conf.num_threads, s->dev.nvqs);
+ if (nworkers > 1) {
+ /*
+ * VHOST_NEW_WORKER works only in vhost_task mode, and
+ * VHOST_RESET_OWNER brings the kthread default back.
+ */
+ uint8_t fork_owner = VHOST_FORK_OWNER_TASK;
+
+ if (ioctl(s->vhostfd, VHOST_SET_FORK_FROM_OWNER, &fork_owner)) {
+ ret = -errno;
+ if (ret != -ENOTTY) {
+ error_report("vhost-blk: unable to set vhost task mode: %s",
+ strerror(-ret));
+ return ret;
+ }
+ warn_report_once("vhost-blk: kernel does not support vhost "
+ "tasks, using a single kernel thread");
+ nworkers = 1;
+ }
+ }
+
if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) {
error_report("vhost-blk: unable to set owner");
return -ENOSYS;
}
- nworkers = s->conf.num_threads;
- if (nworkers != 1 && ioctl(s->vhostfd, VHOST_SET_NWORKERS, &nworkers)) {
- error_report("vhost-blk: unable to set number of kernel threads");
- return -EINVAL;
+ if (nworkers > 1) {
+ ret = vhost_blk_set_workers(s, nworkers);
+ if (ret < 0) {
+ return ret;
+ }
}
if (s->conf.serial) {
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS
2026-10-27 12:47 [PATCH hci-8.1 v2 0/2] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
2026-10-27 12:47 ` [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with " Alexander Ivanov
@ 2026-10-27 12:47 ` Alexander Ivanov
2026-10-27 12:50 ` no-reply
2026-10-27 14:22 ` Andrey Drobyshev
1 sibling, 2 replies; 9+ messages in thread
From: Alexander Ivanov @ 2026-10-27 12:47 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core
vhost-blk no longer uses it, and the kernel drops the ioctl.
https://virtuozzo.atlassian.net/browse/VSTOR-107537
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
linux-headers/linux/vhost.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index baad06412d4..b6eb628d973 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -102,15 +102,6 @@
#define VHOST_GET_VRING_WORKER _IOWR(VHOST_VIRTIO, 0x16, \
struct vhost_vring_worker)
-/* Set number of vhost workers
- * Currently number of vhost workers can only be increased.
- * All workers are freed upon reset.
- * If the value is too big it is silently truncated to the maximum number of
- * supported vhost workers
- * Even if the error is returned it is possible that some workers were created
- */
-#define VHOST_SET_NWORKERS _IOW(VHOST_VIRTIO, 0x1F, int)
-
/* The following ioctls use eventfd file descriptors to signal and poll
* for events. */
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS
2026-10-27 12:47 ` [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
@ 2026-10-27 12:50 ` no-reply
2026-10-27 14:22 ` Andrey Drobyshev
1 sibling, 0 replies; 9+ messages in thread
From: no-reply @ 2026-10-27 12:50 UTC (permalink / raw)
To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core, andrey.drobyshev
> vhost-blk no longer uses it, and the kernel drops the ioctl.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027124759.2979043-1-alexander.ivanov@virtuozzo.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls
2026-10-27 12:47 ` [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with " Alexander Ivanov
@ 2026-10-27 12:52 ` no-reply
2026-10-27 13:05 ` Alexander Ivanov
2026-10-27 14:22 ` Andrey Drobyshev
1 sibling, 1 reply; 9+ messages in thread
From: no-reply @ 2026-10-27 12:52 UTC (permalink / raw)
To: andrey.zhadchenko, Alexander Ivanov; +Cc: andrey.drobyshev, svt-core
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vhost-blk: num-threads=0 silently accepted as one thread, no validation
--
Commit b390c3229288ae4b409cac07cdeba78f79d9b1e7
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
vhost-blk: create kernel threads with the mainstream worker ioctls
This patch replaces the VHOST_SET_NWORKERS ioctl in vhost_blk_start() with
VHOST_SET_FORK_FROM_OWNER, VHOST_NEW_WORKER and VHOST_ATTACH_VRING_WORKER.
It creates num-threads - 1 extra workers and attaches them to the virtqueues
round-robin. If the kernel lacks these ioctls, it warns and runs a single
thread.
Link: https://virtuozzo.atlassian.net/browse/VSTOR-107537
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
[ ... ]
> @@ -51,15 +97,37 @@ static int vhost_blk_start(VirtIODevice *vdev)
> return 0;
> }
>
> + nworkers = MIN(s->conf.num_threads, s->dev.nvqs);
> + if (nworkers > 1) {
[Severity: Low]
This isn't a crash, but the new code changes how num-threads=0 and values
above the queue count are handled. Is that intended?
Before this patch, vhost_blk_start() passed nworkers to VHOST_SET_NWORKERS
whenever it was not 1. A value of 0 was therefore passed to the kernel,
and as far as I can tell start failed with "unable to set number of kernel
threads". I could not check how the kernel handled 0.
Now nworkers = MIN(num_threads, nvqs) evaluates to 0 for num-threads=0, so
"nworkers > 1" is false. The device starts silently with a single kernel
thread, as if num-threads=1 had been given.
The same happens when num-threads is larger than the number of queues. The
count is clamped to nvqs and no diagnostic is printed.
The num-threads property is DEFINE_PROP_UINT16 with a default of 1, and
vhost_blk_device_realize() only validates num_queues. Nothing checks
num_threads there either.
Should realize reject num-threads=0, and should it warn or fail when
num-threads exceeds num-queues?
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027124759.2979043-1-alexander.ivanov@virtuozzo.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls
2026-10-27 12:52 ` no-reply
@ 2026-10-27 13:05 ` Alexander Ivanov
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Ivanov @ 2026-10-27 13:05 UTC (permalink / raw)
To: no-reply, andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core
>> @@ -51,15 +97,37 @@ static int vhost_blk_start(VirtIODevice *vdev)
>> return 0;
>> }
>>
>> + nworkers = MIN(s->conf.num_threads, s->dev.nvqs);
>> + if (nworkers > 1) {
> [Severity: Low]
> This isn't a crash, but the new code changes how num-threads=0 and values
> above the queue count are handled. Is that intended?
>
> Before this patch, vhost_blk_start() passed nworkers to VHOST_SET_NWORKERS
> whenever it was not 1. A value of 0 was therefore passed to the kernel,
> and as far as I can tell start failed with "unable to set number of kernel
> threads". I could not check how the kernel handled 0.
>
> Now nworkers = MIN(num_threads, nvqs) evaluates to 0 for num-threads=0, so
> "nworkers > 1" is false. The device starts silently with a single kernel
> thread, as if num-threads=1 had been given.
>
> The same happens when num-threads is larger than the number of queues. The
> count is clamped to nvqs and no diagnostic is printed.
>
> The num-threads property is DEFINE_PROP_UINT16 with a default of 1, and
> vhost_blk_device_realize() only validates num_queues. Nothing checks
> num_threads there either.
>
> Should realize reject num-threads=0, and should it warn or fail when
> num-threads exceeds num-queues?
This matches the old behaviour. The kernel's VHOST_SET_NWORKERS handler
clamped n to nvqs and, for n=0, created no extra workers and returned
success, so num-threads=0 already ran a single thread and values above
num-queues were already clamped silently. The patch keeps those semantics.
Rejecting num-threads=0 in realize would break configurations that work
today, so I'd leave validation out of this series.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls
2026-10-27 12:47 ` [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with " Alexander Ivanov
2026-10-27 12:52 ` no-reply
@ 2026-10-27 14:22 ` Andrey Drobyshev
2026-10-28 11:48 ` Alexander Ivanov
1 sibling, 1 reply; 9+ messages in thread
From: Andrey Drobyshev @ 2026-10-27 14:22 UTC (permalink / raw)
To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core
On 10/27/26 2:47 PM, Alexander Ivanov wrote:
> num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl.
> The kernel is dropping it in favour of the mainstream worker ioctls,
> which vhost-blk now supports, so use those: create num-threads - 1
> workers with VHOST_NEW_WORKER and attach them to the virtqueues
> round-robin with VHOST_ATTACH_VRING_WORKER, the default worker being
> the first one. This is the layout VHOST_SET_NWORKERS produced.
>
> VHOST_NEW_WORKER works only in vhost_task mode, while our kernel,
> unlike mainline, defaults to kthreads, so switch the device to
> vhost_task mode with VHOST_SET_FORK_FROM_OWNER before VHOST_SET_OWNER.
> VHOST_RESET_OWNER restores the default, hence this is done on every
> start. With num-threads=1 nothing changes: no worker ioctls, kthread
> mode.
>
> On a kernel without these ioctls warn and run a single thread rather
> than fail to start vhost-blk.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> [...]
The overall logic is fine, but I'd like patch to be simplified.
First, there're dedicated vhost_ops->vhost_new_worker() and
vhost_ops->vhost_attach_vring_worker() calls. We should reuse them.
For consistency, let's add another patch "vhost-backend: add
vhost_set_fork_from_owner op". Smth like:
> /* hw/virtio/vhost-backend.c */
> static int vhost_kernel_set_fork_from_owner(struct vhost_dev *dev,
> uint8_t fork_owner)
> {
> return vhost_kernel_call(dev, VHOST_SET_FORK_FROM_OWNER, &fork_owner);
> }
Other issues (by Claude):
1. Dead fallback in vhost_blk_set_workers(). The ENOTTY && i == 1 branch
handles a kernel that has VHOST_SET_FORK_FROM_OWNER (6.17 / vz10) but
not VHOST_NEW_WORKER (6.4) — no such kernel exists. Once the fork-mode
ioctl succeeded, a worker-ioctl failure is a real error. Drop, do a
single fallback point in start().
2. Fork-mode block in vhost_blk_start() is hard to read. A declaration
inside the if, the ENOTTY test nested under 'ret != -ENOTTY', the
'nworkers = 1' fall-through at the bottom. Switch to flat three-way if
(ret == -ENOTTY) … else if (ret < 0) … at the call site, ioctl plumbing
in the backend.
3. Redundant clamp. MIN(nworkers, VHOST_BLK_MAX_QUEUES) is already
guaranteed by nworkers ≤ nvqs ≤ VHOST_BLK_MAX_QUEUES (realize, :666); a
clamp hides an invariant. Switch to assert(nworkers <= ARRAY_SIZE(ids)).
4. memset() + field stores for two structs per iteration. Switch to
designated initializers, and the two loops merged into one: thread w is
created by its first user vq (i < nworkers), then attached — same ioctl
count, one error path per ioctl, loop body reads as "which thread,
create if first, attach".
5. s->dev. repeated throughout. Use struct vhost_dev *hdev = &s->dev;
once, as vhost-scsi does; vhost_blk_set_workers() takes hdev and no
longer needs VHostBlk.
6. Commit message nits (1/2). "The kernel is dropping it" needs the
kernel change/ticket; say that VHOST_NEW_WORKER returns -EFAULT (not
ENOTTY) in kthread mode, which is why the mode switch — not the worker
ioctl — is the capability probe; "default worker being the first one"
belongs in the code comment, where it already is.
Here's what the changed version might look like:
> /*
> * Spread the virtqueues round-robin over nworkers kernel threads. Thread 0
> * is the default worker VHOST_SET_OWNER created; vq 0 and every nworkers-th
> * vq after it stay on it.
> */
> static int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> {
> uint32_t ids[VHOST_BLK_MAX_QUEUES]; /* ids[0]: default worker, unused */
> int i, ret;
>
> assert(nworkers <= ARRAY_SIZE(ids));
>
> for (i = 1; i < hdev->nvqs; i++) {
> int w = i % nworkers;
> struct vhost_vring_worker vq_worker = { .index = i };
>
> if (w == 0) {
> continue;
> }
>
> /* vq i < nworkers is the first user of thread i: create it here */
> if (i < nworkers) {
> struct vhost_worker_state worker = { };
>
> ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> if (ret < 0) {
> error_report("vhost-blk: unable to create kernel thread: %s",
> strerror(-ret));
> return ret;
> }
> ids[w] = worker.worker_id;
> }
>
> vq_worker.worker_id = ids[w];
> ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker);
> if (ret < 0) {
> error_report("vhost-blk: unable to attach kernel thread to "
> "queue %d: %s", i, strerror(-ret));
> return ret;
> }
> }
>
> return 0;
> }
>
> static int vhost_blk_start(VirtIODevice *vdev)
> {
> VHostBlk *s = VHOST_BLK(vdev);
> struct vhost_dev *hdev = &s->dev;
> ...
> int ret, i, nworkers;
> ...
> if (s->vhost_started) {
> return 0;
> }
>
> nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> if (nworkers > 1) {
> /*
> * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to
> * kthreads and VHOST_RESET_OWNER brings that default back, so ask
> * on every start. Must precede VHOST_SET_OWNER.
> */
> ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> VHOST_FORK_OWNER_TASK);
> if (ret == -ENOTTY) {
> warn_report_once("vhost-blk: kernel does not support vhost tasks, "
> "using a single kernel thread");
> nworkers = 1;
> } else if (ret < 0) {
> error_report("vhost-blk: unable to set vhost task mode: %s",
> strerror(-ret));
> return ret;
> }
> }
>
> if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) {
> error_report("vhost-blk: unable to set owner");
> return -ENOSYS;
> }
>
> if (nworkers > 1) {
> ret = vhost_blk_set_workers(hdev, nworkers);
> if (ret < 0) {
> return ret;
> }
> }
>
> if (s->conf.serial) {
> ...
Andrey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS
2026-10-27 12:47 ` [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
2026-10-27 12:50 ` no-reply
@ 2026-10-27 14:22 ` Andrey Drobyshev
1 sibling, 0 replies; 9+ messages in thread
From: Andrey Drobyshev @ 2026-10-27 14:22 UTC (permalink / raw)
To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core
On 10/27/26 2:47 PM, Alexander Ivanov wrote:
> vhost-blk no longer uses it, and the kernel drops the ioctl.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> [...]
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with the mainstream worker ioctls
2026-10-27 14:22 ` Andrey Drobyshev
@ 2026-10-28 11:48 ` Alexander Ivanov
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Ivanov @ 2026-10-28 11:48 UTC (permalink / raw)
To: Andrey Drobyshev, andrey.zhadchenko; +Cc: svt-core
> The overall logic is fine, but I'd like patch to be simplified.
>
> First, there're dedicated vhost_ops->vhost_new_worker() and
> vhost_ops->vhost_attach_vring_worker() calls. We should reuse them.
> For consistency, let's add another patch "vhost-backend: add
> vhost_set_fork_from_owner op". Smth like:
>
>> /* hw/virtio/vhost-backend.c */
>> static int vhost_kernel_set_fork_from_owner(struct vhost_dev *dev,
>> uint8_t fork_owner)
>> {
>> return vhost_kernel_call(dev, VHOST_SET_FORK_FROM_OWNER, &fork_owner);
>> }
Done.
> Other issues (by Claude):
>
> 1. Dead fallback in vhost_blk_set_workers(). The ENOTTY && i == 1 branch
> handles a kernel that has VHOST_SET_FORK_FROM_OWNER (6.17 / vz10) but
> not VHOST_NEW_WORKER (6.4) — no such kernel exists. Once the fork-mode
> ioctl succeeded, a worker-ioctl failure is a real error. Drop, do a
> single fallback point in start().
The fallback is not dead. VHOST_SET_FORK_FROM_OWNER is in our
6.12 kernel as a RHEL 10 backport. The worker ioctls are generic,
but each vhost driver has to route them to vhost_worker_ioctl(),
and vhost-blk only does that with "drivers/vhost/blk: add support
for worker ioctls" (VSTOR-107537), which is not merged yet.
> 2. Fork-mode block in vhost_blk_start() is hard to read. A declaration
> inside the if, the ENOTTY test nested under 'ret != -ENOTTY', the
> 'nworkers = 1' fall-through at the bottom. Switch to flat three-way if
> (ret == -ENOTTY) … else if (ret < 0) … at the call site, ioctl plumbing
> in the backend.
Done.
> 3. Redundant clamp. MIN(nworkers, VHOST_BLK_MAX_QUEUES) is already
> guaranteed by nworkers ≤ nvqs ≤ VHOST_BLK_MAX_QUEUES (realize, :666); a
> clamp hides an invariant. Switch to assert(nworkers <= ARRAY_SIZE(ids)).
Done, switched to the assert. To make it hold, I added a patch that rejects
num-queues above VHOST_BLK_MAX_QUEUES at realize: before, only the
auto value was capped there.
> 4. memset() + field stores for two structs per iteration. Switch to
> designated initializers, and the two loops merged into one: thread w is
> created by its first user vq (i < nworkers), then attached — same ioctl
> count, one error path per ioctl, loop body reads as "which thread,
> create if first, attach".
Done. The structs use designated initializers, and the two loops
are merged into one: a thread is created by its first vq, then attached.
> 5. s->dev. repeated throughout. Use struct vhost_dev *hdev = &s->dev;
> once, as vhost-scsi does; vhost_blk_set_workers() takes hdev and no
> longer needs VHostBlk.
Done. vhost_blk_start() uses hdev throughout, and vhost_blk_set_workers()
takes hdev instead of VHostBlk.
> 6. Commit message nits (1/2). "The kernel is dropping it" needs the
> kernel change/ticket; say that VHOST_NEW_WORKER returns -EFAULT (not
> ENOTTY) in kthread mode, which is why the mode switch — not the worker
> ioctl — is the capability probe; "default worker being the first one"
> belongs in the code comment, where it already is.
>
> Here's what the changed version might look like:
>
>> /*
>> * Spread the virtqueues round-robin over nworkers kernel threads. Thread 0
>> * is the default worker VHOST_SET_OWNER created; vq 0 and every nworkers-th
>> * vq after it stay on it.
>> */
>> static int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
>> {
>> uint32_t ids[VHOST_BLK_MAX_QUEUES]; /* ids[0]: default worker, unused */
>> int i, ret;
>>
>> assert(nworkers <= ARRAY_SIZE(ids));
>>
>> for (i = 1; i < hdev->nvqs; i++) {
>> int w = i % nworkers;
>> struct vhost_vring_worker vq_worker = { .index = i };
>>
>> if (w == 0) {
>> continue;
>> }
>>
>> /* vq i < nworkers is the first user of thread i: create it here */
>> if (i < nworkers) {
>> struct vhost_worker_state worker = { };
>>
>> ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
>> if (ret < 0) {
>> error_report("vhost-blk: unable to create kernel thread: %s",
>> strerror(-ret));
>> return ret;
>> }
>> ids[w] = worker.worker_id;
>> }
>>
>> vq_worker.worker_id = ids[w];
>> ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker);
>> if (ret < 0) {
>> error_report("vhost-blk: unable to attach kernel thread to "
>> "queue %d: %s", i, strerror(-ret));
>> return ret;
>> }
>> }
>>
>> return 0;
>> }
>>
>> static int vhost_blk_start(VirtIODevice *vdev)
>> {
>> VHostBlk *s = VHOST_BLK(vdev);
>> struct vhost_dev *hdev = &s->dev;
>> ...
>> int ret, i, nworkers;
>> ...
>> if (s->vhost_started) {
>> return 0;
>> }
>>
>> nworkers = MIN(s->conf.num_threads, hdev->nvqs);
>> if (nworkers > 1) {
>> /*
>> * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to
>> * kthreads and VHOST_RESET_OWNER brings that default back, so ask
>> * on every start. Must precede VHOST_SET_OWNER.
>> */
>> ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
>> VHOST_FORK_OWNER_TASK);
>> if (ret == -ENOTTY) {
>> warn_report_once("vhost-blk: kernel does not support vhost tasks, "
>> "using a single kernel thread");
>> nworkers = 1;
>> } else if (ret < 0) {
>> error_report("vhost-blk: unable to set vhost task mode: %s",
>> strerror(-ret));
>> return ret;
>> }
>> }
>>
>> if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) {
>> error_report("vhost-blk: unable to set owner");
>> return -ENOSYS;
>> }
>>
>> if (nworkers > 1) {
>> ret = vhost_blk_set_workers(hdev, nworkers);
>> if (ret < 0) {
>> return ret;
>> }
>> }
>>
>> if (s->conf.serial) {
>> ...
Done.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-28 11:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 12:47 [PATCH hci-8.1 v2 0/2] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
2026-10-27 12:47 ` [PATCH hci-8.1 v2 1/2] vhost-blk: create kernel threads with " Alexander Ivanov
2026-10-27 12:52 ` no-reply
2026-10-27 13:05 ` Alexander Ivanov
2026-10-27 14:22 ` Andrey Drobyshev
2026-10-28 11:48 ` Alexander Ivanov
2026-10-27 12:47 ` [PATCH hci-8.1 v2 2/2] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
2026-10-27 12:50 ` no-reply
2026-10-27 14:22 ` Andrey Drobyshev
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.