On Tue, May 25, 2021 at 01:05:54PM -0500, Mike Christie wrote:
> -void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)
> +static void vhost_work_queue_on(struct vhost_work *work,
> +                             struct vhost_worker *worker)
>  {
> -     if (!dev->worker)
> -             return;
> -
>       if (!test_and_set_bit(VHOST_WORK_QUEUED, &work->flags)) {
>               /* We can only add the work to the list after we're
>                * sure it was not in the list.
>                * test_and_set_bit() implies a memory barrier.
>                */
> -             llist_add(&work->node, &dev->worker->work_list);
> -             wake_up_process(dev->worker->task);
> +             llist_add(&work->node, &worker->work_list);
> +             wake_up_process(worker->task);
>       }
>  }
> +
> +void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)

When should this function still be used? A doc comment contrasting it to
vhost_work_queue_on() would be helpful. I would expect callers to switch
to that instead of queuing work on dev->workers[0].

>  /* A lockless hint for busy polling code to exit the loop */
>  bool vhost_has_work(struct vhost_dev *dev)
>  {
> -     return dev->worker && !llist_empty(&dev->worker->work_list);
> +     int i;
> +
> +     for (i = 0; i < dev->num_workers; i++) {
> +             if (!llist_empty(&dev->workers[i]->work_list))
> +                     return true;
> +     }
> +
> +     return false;
>  }
>  EXPORT_SYMBOL_GPL(vhost_has_work);

It's probably not necessary to poll all workers:

drivers/vhost/net.c calls vhost_has_work() to busy poll a specific
virtqueue. If the vq:worker mapping is 1:1 or N:1 then vhost_has_work()
should be extended to include the struct vhost_virtqueue so we can poll
just that vq worker's work_list.
>  /* Caller must have device mutex */
>  static int vhost_worker_try_create_def(struct vhost_dev *dev)
>  {
> -     if (!dev->use_worker || dev->worker)
> +     struct vhost_worker *worker;
> +
> +     if (!dev->use_worker || dev->workers)
>               return 0;
>  
> -     return vhost_worker_create(dev);
> +     dev->workers = kcalloc(1, sizeof(struct vhost_worker *), GFP_KERNEL);

GFP_KERNEL_ACCOUNT so that vhost memory associated with a process (the
one that invoked the ioctl) is accounted? This may get trickier if the
workers are shared between processes.

The same applies for struct vhost_worker in vhost_worker_create().

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to