On 11/29/23 3:30 AM, Stefano Garzarella wrote:
> On Sun, Nov 26, 2023 at 06:28:34PM -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, workers_per_virtqueue, which can be set
>> to:
>>
>> false: Existing behavior where we get the single worker thread.
>> true: Create a worker per IO virtqueue.
>>
>> Signed-off-by: Mike Christie <michael.chris...@oracle.com>
>> ---
>> hw/scsi/vhost-scsi.c | 60 +++++++++++++++++++++++++++++++++
>> include/hw/virtio/virtio-scsi.h | 1 +
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
>> index 3126df9e1d9d..77eef9474c23 100644
>> --- a/hw/scsi/vhost-scsi.c
>> +++ b/hw/scsi/vhost-scsi.c
>> @@ -165,6 +165,57 @@ static const VMStateDescription
>> vmstate_virtio_vhost_scsi = {
>> .pre_save = vhost_scsi_pre_save,
>> };
>>
>> +static int vhost_scsi_set_workers(VHostSCSICommon *vsc, bool per_virtqueue)
>> +{
>> + struct vhost_dev *dev = &vsc->dev;
>> + struct vhost_vring_worker vq_worker;
>> + struct vhost_worker_state worker;
>> + int i, ret;
>> +
>> + /* Use default worker */
>> + if (!per_virtqueue || dev->nvqs == VHOST_SCSI_VQ_NUM_FIXED + 1) {
>> + return 0;
>> + }
>> +
>> + /*
>> + * 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);
>> + if (ret == -ENOTTY) {
>> + /*
>> + * worker ioctls are not implemented so just ignore and
>> + * and continue device setup.
>> + */
>
> IIUC here the user has asked to use a worker for each virtqueue, but the
> kernel does not support it so we ignore it.
>
> Should we at least print a warning?
>
We should. I'll add it.
