On 11/15/23 5:43 AM, Stefano Garzarella wrote:
> On Mon, Nov 13, 2023 at 06:36:44PM -0600, Mike Christie wrote:
>> This adds support for vhost-scsi to be able to create a worker thread
>> per virtqueue. Right now for vhost-net we get a worker thread per
>> tx/rx virtqueue pair which scales nicely as we add more virtqueues and
>> CPUs, but for scsi we get the single worker thread that's shared by all
>> virtqueues. When trying to send IO to more than 2 virtqueues the single
>> thread becomes a bottlneck.
>>
>> This patch adds a new setting, virtqueue_workers, which can be set to:
>>
>> 1: Existing behavior whre we get the single thread.
>> -1: Create a worker per IO virtqueue.
>
> I find this setting a bit odd. What about a boolean instead?
>
> `per_virtqueue_workers`:
> false: Existing behavior whre we get the single thread.
> true: Create a worker per IO virtqueue.
Sound good.
>
>>
>> Signed-off-by: Mike Christie <michael.chris...@oracle.com>
>> ---
>> hw/scsi/vhost-scsi.c | 68 +++++++++++++++++++++++++++++++++
>> include/hw/virtio/virtio-scsi.h | 1 +
>> 2 files changed, 69 insertions(+)
>>
>> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
>> index 3126df9e1d9d..5cf669b6563b 100644
>> --- a/hw/scsi/vhost-scsi.c
>> +++ b/hw/scsi/vhost-scsi.c
>> @@ -31,6 +31,9 @@
>> #include "qemu/cutils.h"
>> #include "sysemu/sysemu.h"
>>
>> +#define VHOST_SCSI_WORKER_PER_VQ -1
>> +#define VHOST_SCSI_WORKER_DEF 1
>> +
>> /* Features supported by host kernel. */
>> static const int kernel_feature_bits[] = {
>> VIRTIO_F_NOTIFY_ON_EMPTY,
>> @@ -165,6 +168,62 @@ static const VMStateDescription
>> vmstate_virtio_vhost_scsi = {
>> .pre_save = vhost_scsi_pre_save,
>> };
>>
>> +static int vhost_scsi_set_workers(VHostSCSICommon *vsc, int workers_cnt)
>> +{
>> + struct vhost_dev *dev = &vsc->dev;
>> + struct vhost_vring_worker vq_worker;
>> + struct vhost_worker_state worker;
>> + int i, ret;
>> +
>> + /* Use default worker */
>> + if (workers_cnt == VHOST_SCSI_WORKER_DEF ||
>> + dev->nvqs == VHOST_SCSI_VQ_NUM_FIXED + 1) {
>> + return 0;
>> + }
>> +
>> + if (workers_cnt != VHOST_SCSI_WORKER_PER_VQ) {
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * ctl/evt share the first worker since it will be rare for them
>> + * to send cmds while IO is running.
>> + */
>> + for (i = VHOST_SCSI_VQ_NUM_FIXED + 1; i < dev->nvqs; i++) {
>> + memset(&worker, 0, sizeof(worker));
>> +
>> + ret = dev->vhost_ops->vhost_new_worker(dev, &worker);
>
> Should we call vhost_free_worker() in the vhost_scsi_unrealize() or are
> workers automatically freed when `vhostfd` is closed?
>
All worker threads are freed automatically like how the default worker
created from VHOST_SET_OWNER is freed on close.
