Re: [RESEND PATCH v1 0/7]vhost: Add support of kthread API

[Michael S. Tsirkin]

On Mon, Sep 09, 2024 at 10:00:38AM +0800, Cindy Lu wrote:
> In commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads"),
> vhost removed the support for the kthread API. However, there are
> still situations where there is a request to use kthread.
> In this PATCH, the support of kthread is added back. Additionally,
> a module_param is added to enforce which mode we are using, and
> a new UAPI is introduced to allow the userspace app to set the
> mode they want to use.
> 
> Tested the user application with QEMU.

Cindy, the APIs do not make sense, security does not make sense,
and you are not describing the issue and the fix.


The name should reflect what this does from userspace POV, not from
kernel API POV.  kthread and task mode is not something that any users
have any business even to consider.


To help you out, some ideas:

I think the issue is something like "vhost is now a child of the
owner thread. While this makes sense from containerization
POV, some old userspace is confused, as previously vhost not
and so was allowed to steal cpu resources from outside the container."

Now, what can be done? Given we already released a secure kernel,
I am not inclined to revert it back to be insecure by default.
But I'm fine with a modparam to allow userspace to get insecure
behaviour.


Now, a modparam is annoying in that it affects all userspace,
and people might be running a mix of old and new userspace.
So if we do that, we also want a flag that will get
safe behaviour even if unsafe one is allowed.


Is this clear enough, or do I need to elaborate more?

Thanks!

> Cindy Lu (7):
>   vhost: Add a new module_param for enable kthread
>   vhost: Add kthread support in function vhost_worker_queue()
>   vhost: Add kthread support in function vhost_workers_free()
>   vhost: Add the vhost_worker to support kthread
>   vhost: Add the cgroup related function
>   vhost: Add kthread support in function vhost_worker_create
>   vhost: Add new UAPI to support change to task mode
> 
>  drivers/vhost/vhost.c      | 246 +++++++++++++++++++++++++++++++++++--
>  drivers/vhost/vhost.h      |   1 +
>  include/uapi/linux/vhost.h |   2 +
>  3 files changed, 240 insertions(+), 9 deletions(-)
> 
> -- 
> 2.45.0