Message-ID: <CACGkMEtyX6n9uLMmo7X08tFS-V6QZoDVTxhE53h9sLDPNBKnKw@mail.gmail.com>
Date: Tue, 28 Oct 2025 08:42:32 +0800
From: Jason Wang <jasowang@...hat.com>
To: Nick Hudson <nhudson@...mai.com>
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Eugenio Pérez <eperezma@...hat.com>, 
	Max Tottenham <mtottenh@...mai.com>, kvm@...r.kernel.org, virtualization@...ts.linux.dev, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vhost: add a new ioctl VHOST_GET_VRING_WORKER_INFO and
 use in net.c

On Mon, Oct 27, 2025 at 6:27 PM Nick Hudson <nhudson@...mai.com> wrote:
>
> The vhost_net (and vhost_sock) drivers create worker tasks to handle
> the virtual queues. Provide a new ioctl VHOST_GET_VRING_WORKER_INFO that
> can be used to determine the PID of these tasks so that, for example,
> they can be pinned to specific CPU(s).
>
> Signed-off-by: Nick Hudson <nhudson@...mai.com>
> Reviewed-by: Max Tottenham <mtottenh@...mai.com>
> ---
>  drivers/vhost/net.c              |  5 +++++
>  drivers/vhost/vhost.c            | 16 ++++++++++++++++
>  include/uapi/linux/vhost.h       |  3 +++
>  include/uapi/linux/vhost_types.h | 13 +++++++++++++
>  kernel/vhost_task.c              | 12 ++++++++++++
>  5 files changed, 49 insertions(+)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 35ded4330431..e86bd5d7d202 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1804,6 +1804,11 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
>                 return vhost_net_reset_owner(n);
>         case VHOST_SET_OWNER:
>                 return vhost_net_set_owner(n);
> +       case VHOST_GET_VRING_WORKER_INFO:
> +               mutex_lock(&n->dev.mutex);
> +               r = vhost_worker_ioctl(&n->dev, ioctl, argp);
> +               mutex_unlock(&n->dev.mutex);
> +               return r;
>         default:
>                 mutex_lock(&n->dev.mutex);
>                 r = vhost_dev_ioctl(&n->dev, ioctl, argp);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 8570fdf2e14a..8b52fd5723c3 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2399,6 +2399,22 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>                 if (ctx)
>                         eventfd_ctx_put(ctx);
>                 break;
> +       case VHOST_GET_VRING_WORKER_INFO:
> +               worker = rcu_dereference_check(vq->worker,
> +                                              lockdep_is_held(&dev->mutex));
> +               if (!worker) {
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +
> +               memset(&ring_worker_info, 0, sizeof(ring_worker_info));
> +               ring_worker_info.index = idx;
> +               ring_worker_info.worker_id = worker->id;
> +               ring_worker_info.worker_pid = task_pid_vnr(vhost_get_task(worker->vtsk));
> +
> +               if (copy_to_user(argp, &ring_worker_info, sizeof(ring_worker_info)))
> +                       ret = -EFAULT;
> +               break;
>         default:
>                 r = -ENOIOCTLCMD;
>                 break;
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c57674a6aa0d..c32aa8c71952 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -101,6 +101,9 @@
>  /* Return the vring worker's ID */
>  #define VHOST_GET_VRING_WORKER _IOWR(VHOST_VIRTIO, 0x16,               \
>                                      struct vhost_vring_worker)
> +/* Return the vring worker's ID and PID */
> +#define VHOST_GET_VRING_WORKER_INFO _IOWR(VHOST_VIRTIO, 0x17,  \
> +                                    struct vhost_vring_worker_info)
>
>  /* The following ioctls use eventfd file descriptors to signal and poll
>   * for events. */
> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
> index 1c39cc5f5a31..28e00f8ade85 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -63,6 +63,19 @@ struct vhost_vring_worker {
>         unsigned int worker_id;
>  };
>
> +/* Per-virtqueue worker mapping entry */
> +struct vhost_vring_worker_info {
> +       /* vring index */
> +       unsigned int index;
> +       /*
> +        * The id of the vhost_worker returned from VHOST_NEW_WORKER or
> +        * allocated as part of vhost_dev_set_owner.
> +        */
> +       unsigned int worker_id;

I'm not sure the above two are a must and exposing internal workd_id
seems not like a good idea.

> +
> +       __kernel_pid_t worker_pid;  /* PID/TID of worker thread, -1 if none */

Instead of exposing the worker PID, I wonder if it's simple to just
having a better naming of the worker instead of a simple:

        snprintf(name, sizeof(name), "vhost-%d", current->pid);

> +};
> +
>  /* no alignment requirement */
>  struct vhost_iotlb_msg {
>         __u64 iova;
> diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
> index 27107dcc1cbf..aa87a7f0c98a 100644
> --- a/kernel/vhost_task.c
> +++ b/kernel/vhost_task.c
> @@ -67,6 +67,18 @@ static int vhost_task_fn(void *data)
>         do_exit(0);
>  }
>
> +/**
> + * vhost_get_task - get a pointer to the vhost_task's task_struct
> + * @vtsk: vhost_task to return the task for
> + *
> + * return the vhost_task's task.
> + */
> +struct task_struct *vhost_get_task(struct vhost_task *vtsk)
> +{
> +       return vtsk->task;
> +}
> +EXPORT_SYMBOL_GPL(vhost_get_task);
> +
>  /**
>   * vhost_task_wake - wakeup the vhost_task
>   * @vtsk: vhost_task to wake
> --
> 2.34.1
>

Thanks

Powered by blists - more mailing lists