Re: [PATCH 3/9] vhost: modify internal functions to take a vhost_worker
flat view
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2021-06-03 10:45:13
Also in:
linux-scsi, target-devel
On Tue, May 25, 2021 at 01:05:54PM -0500, Mike Christie wrote:
quoted hunk ↗ jump to hunk
-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].
quoted hunk ↗ jump to hunk
/* 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.
quoted hunk ↗ jump to hunk
/* 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().