linux-kernel - Re: [PATCH v4 0/8] vhost: Add support of kthread API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <wxfwmdn73heh5k7dnbaqt5iq23qwz4ltlapetvsqlryhz7mhpt@4e3pw5qm7yom>
Date: Tue, 10 Dec 2024 18:51:54 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Cindy Lu <lulu@...hat.com>
Cc: jasowang@...hat.com, mst@...hat.com, michael.christie@...cle.com, 
	linux-kernel@...r.kernel.org, virtualization@...ts.linux-foundation.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v4 0/8] vhost: Add support of kthread API

On Wed, Dec 11, 2024 at 12:41:39AM +0800, Cindy Lu wrote:
>In commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads"),

missing something here?

>
>The vhost now uses vhost_task and operates as a child of the owner thread.
>This aligns with containerization principles, But it has confused some legacy
>userspace applications. Therefore, we are reintroducing support
>for the kthread API.
>
>In commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads"),
>The vhost now use vhost_task and workers working as a child of the owner thread,
>which aligns with containerization principles. However, this change has caused
>confusion for some legacy userspace applications.
>Therefore, we are reintroducing support for the kthread API.

This paragraph seems duplicated.

If you have to resend a v5, recheck the cover for a moment because it's 
not easy to follow.

>
>In this patch,

s/patch/series

> a new User API is implemented to allow userspace applications to
>configure their request mode.
>
>Changelog v2:
> 1. Change the module_param's name to enforce_inherit_owner, and the default value is true.
> 2. Change the UAPI's name to VHOST_SET_INHERIT_FROM_OWNER.
>
>Changelog v3:
> 1. Change the module_param's name to inherit_owner_default, and the default value is true.
> 2. Add a structure for task function; the worker will select a different mode based on the value inherit_owner.
> 3. device will have their own inherit_owner in struct vhost_dev
> 4. Address other comments
>
>Changelog v4:
> 1. remove the module_param, only keep the UAPI
> 2. remove the structure for task function; change to use the function pointer in vhost_worker
> 3. fix the issue in vhost_worker_create and vhost_dev_ioctl
> 4. Address other comments
>
>Tested with QEMU with kthread mode/task mode/kthread+task mode

A link to QEMU patches will be nice.

>
>Cindy Lu (8):
>  vhost: Add a new parameter in vhost_dev to allow user select kthread
>  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 kthread support in function vhost_worker_queue()
>  vhost: Add kthread support in function vhost_worker_destroy()

What about merging patches 4, 5, 6 in a single patch?

Thanks,
Stefano

>  vhost: Add new UAPI to support change to task mode
>  vhost_scsi: Add check for inherit_owner status
>
> drivers/vhost/scsi.c       |   8 ++
> drivers/vhost/vhost.c      | 185 +++++++++++++++++++++++++++++++++----
> drivers/vhost/vhost.h      |   4 +
> include/uapi/linux/vhost.h |  18 ++++
> 4 files changed, 198 insertions(+), 17 deletions(-)
>
>-- 
>2.45.0
>