Re: [PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

I like VHOST_FORK_OWNER_TASK, but I would rather avoid treating worker 
creation as non-fatal error. This will go on silently and we will never 
know.
Let's just fail when any of the IOCTLs do.

Also let's set this mode for any amount of workers. The difference is 
more or less negligible but we will have uniformity across nworkers and 
better worker isolation.

On 10/29/26 10:23, Alexander Ivanov wrote:
> num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl. The
> kernel drops it with the revert of "drivers/vhost: add ioctl to
> increase the number of workers" in favour of the mainstream worker
> ioctls, which vhost-blk now supports. Create num-threads - 1 workers
> with VHOST_NEW_WORKER and attach them to the virtqueues round-robin
> with VHOST_ATTACH_VRING_WORKER, the layout VHOST_SET_NWORKERS produced.
> 
> Our kernel, unlike mainline, defaults to kthread mode, where
> VHOST_NEW_WORKER fails with -EFAULT rather than -ENOTTY. So switch the
> device to vhost_task mode with VHOST_SET_FORK_FROM_OWNER before
> VHOST_SET_OWNER. With num-threads=1 nothing changes: no worker ioctls,
> kthread mode.
> 
> Released vz10 kernels have VHOST_SET_FORK_FROM_OWNER, but their
> vhost-blk does not handle the worker ioctls and fails VHOST_NEW_WORKER
> with -ENOTTY. Other errors are possible as well, so do not key on the
> errno: if VHOST_SET_FORK_FROM_OWNER fails, keep all virtqueues on the
> default thread; if VHOST_NEW_WORKER fails partway, spread the
> remaining virtqueues over the threads created so far; if attaching a
> virtqueue fails, it stays on the default thread. Either way warn rather
> than fail to start vhost-blk. Unless VHOST_SET_FORK_FROM_OWNER failed,
> the default thread stays a vhost_task, as the mode cannot be changed
> after VHOST_SET_OWNER.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   hw/block/vhost-blk.c | 64 +++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> index b3eb181ceae..9416c3ac07e 100644
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
> @@ -33,6 +33,53 @@
>   #include "migration/cpr.h"
>   #include "system/runstate.h"
>   
> +/*
> + * 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 void vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> +{
> +    uint32_t ids[VHOST_BLK_MAX_QUEUES];
> +    int i, ret, err = 0, nfailed = 0;
> +
> +    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;
> +        }
> +
> +        if (i < nworkers) {
> +            struct vhost_worker_state worker = { };
> +
> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> +            if (ret < 0) {
> +                warn_report("vhost-blk: unable to create kernel thread: %s",
> +                            strerror(-ret));
> +                nworkers = i;
> +                continue;
> +            }
> +            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) {
> +            err = ret;
> +            nfailed++;
> +        }
> +    }
> +
> +    if (nfailed) {
> +        warn_report("vhost-blk: unable to attach kernel threads to %d "
> +                    "queues: %s", nfailed, strerror(-err));
> +    }
> +}
> +
>   static int vhost_blk_start(VirtIODevice *vdev)
>   {
>       VHostBlk *s = VHOST_BLK(vdev);
> @@ -52,15 +99,24 @@ static int vhost_blk_start(VirtIODevice *vdev)
>           return 0;
>       }
>   
> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> +    if (nworkers > 1) {
> +        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> +                                                         VHOST_FORK_OWNER_TASK);
> +        if (ret < 0) {
> +            warn_report("vhost-blk: unable to use multiple kernel threads: "
> +                        "%s", strerror(-ret));
> +            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) {
> +        vhost_blk_set_workers(hdev, nworkers);
>       }
>   
>       if (s->conf.serial) {