[RFC,5/8] vhost: allow userspace to bind vqs to CPUs

This patch allows user space to bind vqs to specific CPUs. Since
cgroups will not be supported this just uses the normal kernel
workqueues. I know you guys were thinking about userspace
initiating the threading and doing something like nbd's DO_IO
ioctl, or something like shared thread pools, etc. But workqueues
have had the benefits:

1. You can share the same thread with different devs and/or vqs
and there's almost no extra code in vhost.c.

2. If a work item blocks for too long, the workqueue code can
create new threads on demand for us. This ends up being helpful
for the scsi case, where we can block waiting for IO completions
when the queues get too full.

3. We get the workqueue's dynamic thread creation and destruction.
We don't have to add our own reaping code when the system becomes
less busy.

Some TODOs:

1. What about the default worker? I left this setup for now.
For vhost-scsi, we can have multple queues, and then also multple
LUNs under the same vhost-scsi device. So we might want some
lower perf LUNs to use the default worker and use the existing
cgroup settings.

2. I added the get_workqueue callout so drivers could pass in
their own workqueue incase some wanted to do a per device one.
We could also just one vhost workqueue. vhost-scsi can do a lot
of work that might block in its work struct so I didn't want
it to interfer with the other devs.

Some results:

With this patch and the patches for 5.11's target/lio layer that
fix up/remove the locking in the main IO path, I can get 920K IOPs
with the lio ram disk and 800K with the nullblk driver and iblock
backend. This is with 8 vqs, virtqueue_size/cmd_per_lun=1024 (in
some guests I had to set both really high to get the guest's
scsi_host can_queue high enough to allow IO from all vqs), and
a fio command like:

fio --filename=/dev/sda --direct=1 --rw=randrw --bs=4k --ioengine=libaio
--iodepth=128  --numjobs=8 --time_based --group_reporting --name=iops
--runtime=60 --eta-newline=1

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/vhost/net.c        |   6 ++-
 drivers/vhost/vhost.c      | 130 +++++++++++++++++++++++++++++++++++++--------
 drivers/vhost/vhost.h      |  15 +++++-
 include/uapi/linux/vhost.h |   5 ++
 4 files changed, 129 insertions(+), 27 deletions(-)

On 2020/12/5 上午12:32, Mike Christie wrote:
> On 12/4/20 2:09 AM, Jason Wang wrote:

>>

>> On 2020/12/4 下午3:56, Mike Christie wrote:

>>> +static long vhost_vring_set_cpu(struct vhost_dev *d, struct 

>>> vhost_virtqueue *vq,

>>> +                void __user *argp)

>>> +{

>>> +    struct vhost_vring_state s;

>>> +    int ret = 0;

>>> +

>>> +    if (vq->private_data)

>>> +        return -EBUSY;

>>> +

>>> +    if (copy_from_user(&s, argp, sizeof s))

>>> +        return -EFAULT;

>>> +

>>> +    if (s.num == -1) {

>>> +        vq->cpu = s.num;

>>> +        return 0;

>>> +    }

>>> +

>>> +    if (s.num >= nr_cpu_ids)

>>> +        return -EINVAL;

>>> +

>>> +    if (!d->ops || !d->ops->get_workqueue)

>>> +        return -EINVAL;

>>> +

>>> +    if (!d->wq)

>>> +        d->wq = d->ops->get_workqueue();

>>> +    if (!d->wq)

>>> +        return -EINVAL;

>>> +

>>> +    vq->cpu = s.num;

>>> +    return ret;

>>> +}

>>

>>

>> So one question here. Who is in charge of doing this set_cpu? Note 

>> that sched_setaffinity(2) requires CAP_SYS_NICE to work, so I wonder 

>> whether or not it's legal for unprivileged Qemu to do this.

>

>

> I was having qemu do it when it's setting up the vqs since it had the 

> info there already.

>

> Is it normally the tool that makes calls into qemu that does the 

> operations that require CAP_SYS_NICE? 



My understanding is that it only matter scheduling. And this patch wants 
to change the affinity which should check that capability.


> If so, then I see the interface needs to be changed.



Actually, if I read this patch correctly it requires e.g qemu to make 
the decision instead of the management layer. This may bring some 
troubles to for e.g the libvirt emulatorpin[1] implementation.

Thanks

[1] 
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/virtualization_tuning_and_optimization_guide/sect-virtualization_tuning_optimization_guide-numa-numa_and_libvirt

On 12/6/20 10:27 PM, Jason Wang wrote:
> 

> On 2020/12/5 上午12:32, Mike Christie wrote:

>> On 12/4/20 2:09 AM, Jason Wang wrote:

>>>

>>> On 2020/12/4 下午3:56, Mike Christie wrote:

>>>> +static long vhost_vring_set_cpu(struct vhost_dev *d, struct 

>>>> vhost_virtqueue *vq,

>>>> +                void __user *argp)

>>>> +{

>>>> +    struct vhost_vring_state s;

>>>> +    int ret = 0;

>>>> +

>>>> +    if (vq->private_data)

>>>> +        return -EBUSY;

>>>> +

>>>> +    if (copy_from_user(&s, argp, sizeof s))

>>>> +        return -EFAULT;

>>>> +

>>>> +    if (s.num == -1) {

>>>> +        vq->cpu = s.num;

>>>> +        return 0;

>>>> +    }

>>>> +

>>>> +    if (s.num >= nr_cpu_ids)

>>>> +        return -EINVAL;

>>>> +

>>>> +    if (!d->ops || !d->ops->get_workqueue)

>>>> +        return -EINVAL;

>>>> +

>>>> +    if (!d->wq)

>>>> +        d->wq = d->ops->get_workqueue();

>>>> +    if (!d->wq)

>>>> +        return -EINVAL;

>>>> +

>>>> +    vq->cpu = s.num;

>>>> +    return ret;

>>>> +}

>>>

>>>

>>> So one question here. Who is in charge of doing this set_cpu? Note 

>>> that sched_setaffinity(2) requires CAP_SYS_NICE to work, so I wonder 

>>> whether or not it's legal for unprivileged Qemu to do this.

>>

>>

>> I was having qemu do it when it's setting up the vqs since it had the 

>> info there already.

>>

>> Is it normally the tool that makes calls into qemu that does the 

>> operations that require CAP_SYS_NICE? 

> 

> 

> My understanding is that it only matter scheduling. And this patch wants 

> to change the affinity which should check that capability.

> 

> 

>> If so, then I see the interface needs to be changed.

> 

> 

> Actually, if I read this patch correctly it requires e.g qemu to make 

> the decision instead of the management layer. This may bring some 

> troubles to for e.g the libvirt emulatorpin[1] implementation.

> 


Let me make sure I understood you.

I thought qemu would just have a new property, and users would pass that 
in like they do for the number of queues setting. Then qemu would pass 
that to the kernel. The primary user I have to support at work does not 
use libvirt based tools so I thought that was a common point that would 
work for everyone.

For my work use requirement, your emulatorpin and CAP_SYS_NICE comment 
then that means we want an interface that something other than qemu can 
use right? So the tools would call directly into the kernel and not go 
through qemu right?

On 2020/12/8 上午2:31, Mike Christie wrote:
> On 12/6/20 10:27 PM, Jason Wang wrote:

>>

>> On 2020/12/5 上午12:32, Mike Christie wrote:

>>> On 12/4/20 2:09 AM, Jason Wang wrote:

>>>>

>>>> On 2020/12/4 下午3:56, Mike Christie wrote:

>>>>> +static long vhost_vring_set_cpu(struct vhost_dev *d, struct 

>>>>> vhost_virtqueue *vq,

>>>>> +                void __user *argp)

>>>>> +{

>>>>> +    struct vhost_vring_state s;

>>>>> +    int ret = 0;

>>>>> +

>>>>> +    if (vq->private_data)

>>>>> +        return -EBUSY;

>>>>> +

>>>>> +    if (copy_from_user(&s, argp, sizeof s))

>>>>> +        return -EFAULT;

>>>>> +

>>>>> +    if (s.num == -1) {

>>>>> +        vq->cpu = s.num;

>>>>> +        return 0;

>>>>> +    }

>>>>> +

>>>>> +    if (s.num >= nr_cpu_ids)

>>>>> +        return -EINVAL;

>>>>> +

>>>>> +    if (!d->ops || !d->ops->get_workqueue)

>>>>> +        return -EINVAL;

>>>>> +

>>>>> +    if (!d->wq)

>>>>> +        d->wq = d->ops->get_workqueue();

>>>>> +    if (!d->wq)

>>>>> +        return -EINVAL;

>>>>> +

>>>>> +    vq->cpu = s.num;

>>>>> +    return ret;

>>>>> +}

>>>>

>>>>

>>>> So one question here. Who is in charge of doing this set_cpu? Note 

>>>> that sched_setaffinity(2) requires CAP_SYS_NICE to work, so I 

>>>> wonder whether or not it's legal for unprivileged Qemu to do this.

>>>

>>>

>>> I was having qemu do it when it's setting up the vqs since it had 

>>> the info there already.

>>>

>>> Is it normally the tool that makes calls into qemu that does the 

>>> operations that require CAP_SYS_NICE? 

>>

>>

>> My understanding is that it only matter scheduling. And this patch 

>> wants to change the affinity which should check that capability.

>>

>>

>>> If so, then I see the interface needs to be changed.

>>

>>

>> Actually, if I read this patch correctly it requires e.g qemu to make 

>> the decision instead of the management layer. This may bring some 

>> troubles to for e.g the libvirt emulatorpin[1] implementation.

>>

>

> Let me make sure I understood you.

>

> I thought qemu would just have a new property, and users would pass 

> that in like they do for the number of queues setting. Then qemu would 

> pass that to the kernel. The primary user I have to support at work 

> does not use libvirt based tools so I thought that was a common point 

> that would work for everyone.



I think we need talk with libvirt guys to see if it works for them. My 
understanding is the scheduling should be the charge of them not qemu.


>

> For my work use requirement, your emulatorpin and CAP_SYS_NICE comment 

> then that means we want an interface that something other than qemu 

> can use right? So the tools would call directly into the kernel and 

> not go through qemu right?



Yes, usually qemu runs without any privilege. So could it be e.g a sysfs 
interface or other?

Thanks


>

>