[PATCH v4 0/8] vhost: Add support of kthread API

[PATCH v4 0/8] vhost: Add support of kthread API

[Cindy Lu]

In commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads"),

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.

In this patch, 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

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()
  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

[PATCH v4 1/8] vhost: Add a new parameter in vhost_dev to allow user select kthread

[Cindy Lu]

The vhost now uses vhost_task and workers as a child of the owner thread.
While this aligns with containerization principles,it confuses some legacy
userspace app, Therefore, we are reintroducing kthread API support.

Introduce a new parameter to enable users to choose between
kthread and task mode.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 1 +
 drivers/vhost/vhost.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9ac25d08f473..eaddbd39c29b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -552,6 +552,7 @@ void vhost_dev_init(struct vhost_dev *dev,
 	dev->byte_weight = byte_weight;
 	dev->use_worker = use_worker;
 	dev->msg_handler = msg_handler;
+	dev->inherit_owner = true;
 	init_waitqueue_head(&dev->wait);
 	INIT_LIST_HEAD(&dev->read_list);
 	INIT_LIST_HEAD(&dev->pending_list);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index bb75a292d50c..c650c4506c70 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -176,6 +176,7 @@ struct vhost_dev {
 	int byte_weight;
 	struct xarray worker_xa;
 	bool use_worker;
+	bool inherit_owner;
 	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 			   struct vhost_iotlb_msg *msg);
 };
-- 
2.45.0

Re: [PATCH v4 1/8] vhost: Add a new parameter in vhost_dev to allow user select kthread

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:40AM +0800, Cindy Lu wrote:
>The vhost now uses vhost_task and workers as a child of the owner thread.
>While this aligns with containerization principles,it confuses some legacy

nit: missing space in "principles,it"

>userspace app, Therefore, we are reintroducing kthread API support.

nit: "app, therefore" or "app. Therefore"

>
>Introduce a new parameter to enable users to choose between
>kthread and task mode.
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c | 1 +
> drivers/vhost/vhost.h | 1 +
> 2 files changed, 2 insertions(+)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 9ac25d08f473..eaddbd39c29b 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -552,6 +552,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> 	dev->byte_weight = byte_weight;
> 	dev->use_worker = use_worker;
> 	dev->msg_handler = msg_handler;
>+	dev->inherit_owner = true;
> 	init_waitqueue_head(&dev->wait);
> 	INIT_LIST_HEAD(&dev->read_list);
> 	INIT_LIST_HEAD(&dev->pending_list);
>diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>index bb75a292d50c..c650c4506c70 100644
>--- a/drivers/vhost/vhost.h
>+++ b/drivers/vhost/vhost.h
>@@ -176,6 +176,7 @@ struct vhost_dev {
> 	int byte_weight;
> 	struct xarray worker_xa;
> 	bool use_worker;
>+	bool inherit_owner;
> 	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
> 			   struct vhost_iotlb_msg *msg);
> };
>-- 
>2.45.0
>

Re: [PATCH v4 1/8] vhost: Add a new parameter in vhost_dev to allow user select kthread

[Cindy Lu]

On Wed, Dec 11, 2024 at 1:52 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Wed, Dec 11, 2024 at 12:41:40AM +0800, Cindy Lu wrote:
> >The vhost now uses vhost_task and workers as a child of the owner thread.
> >While this aligns with containerization principles,it confuses some legacy
>
> nit: missing space in "principles,it"
>
Thanks Stefano, will fix this
> >userspace app, Therefore, we are reintroducing kthread API support.
>
> nit: "app, therefore" or "app. Therefore"
>
sure, will fix this
> >
> >Introduce a new parameter to enable users to choose between
> >kthread and task mode.
> >
> >Signed-off-by: Cindy Lu <lulu@redhat.com>
> >---
> > drivers/vhost/vhost.c | 1 +
> > drivers/vhost/vhost.h | 1 +
> > 2 files changed, 2 insertions(+)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index 9ac25d08f473..eaddbd39c29b 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -552,6 +552,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> >       dev->byte_weight = byte_weight;
> >       dev->use_worker = use_worker;
> >       dev->msg_handler = msg_handler;
> >+      dev->inherit_owner = true;
> >       init_waitqueue_head(&dev->wait);
> >       INIT_LIST_HEAD(&dev->read_list);
> >       INIT_LIST_HEAD(&dev->pending_list);
> >diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >index bb75a292d50c..c650c4506c70 100644
> >--- a/drivers/vhost/vhost.h
> >+++ b/drivers/vhost/vhost.h
> >@@ -176,6 +176,7 @@ struct vhost_dev {
> >       int byte_weight;
> >       struct xarray worker_xa;
> >       bool use_worker;
> >+      bool inherit_owner;
> >       int (*msg_handler)(struct vhost_dev *dev, u32 asid,
> >                          struct vhost_iotlb_msg *msg);
> > };
> >--
> >2.45.0
> >
>

[PATCH v4 2/8] vhost: Add the vhost_worker to support kthread

[Cindy Lu]

Add the previously removed function vhost_worker() back
to support the kthread and rename it to vhost_run_work_kthread_list.

The old function vhost_worker was change to support task in
commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
change to xarray in
commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index eaddbd39c29b..1feba29abf95 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -388,6 +388,44 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	__vhost_vq_meta_reset(vq);
 }
 
+static int vhost_run_work_kthread_list(void *data)
+{
+	struct vhost_worker *worker = data;
+	struct vhost_work *work, *work_next;
+	struct vhost_dev *dev = worker->dev;
+	struct llist_node *node;
+
+	kthread_use_mm(dev->mm);
+
+	for (;;) {
+		/* mb paired w/ kthread_stop */
+		set_current_state(TASK_INTERRUPTIBLE);
+
+		if (kthread_should_stop()) {
+			__set_current_state(TASK_RUNNING);
+			break;
+		}
+		node = llist_del_all(&worker->work_list);
+		if (!node)
+			schedule();
+
+		node = llist_reverse_order(node);
+		/* make sure flag is seen after deletion */
+		smp_wmb();
+		llist_for_each_entry_safe(work, work_next, node, node) {
+			clear_bit(VHOST_WORK_QUEUED, &work->flags);
+			__set_current_state(TASK_RUNNING);
+			kcov_remote_start_common(worker->kcov_handle);
+			work->fn(work);
+			kcov_remote_stop();
+			cond_resched();
+		}
+	}
+	kthread_unuse_mm(dev->mm);
+
+	return 0;
+}
+
 static bool vhost_run_work_list(void *data)
 {
 	struct vhost_worker *worker = data;
-- 
2.45.0

Re: [PATCH v4 2/8] vhost: Add the vhost_worker to support kthread

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:41AM +0800, Cindy Lu wrote:
>Add the previously removed function vhost_worker() back
>to support the kthread and rename it to vhost_run_work_kthread_list.
>
>The old function vhost_worker was change to support task in

s/change/changed

>commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
>change to xarray in

"and to support multiple workers per device using xarray in"

>commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index eaddbd39c29b..1feba29abf95 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -388,6 +388,44 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> 	__vhost_vq_meta_reset(vq);
> }
>
>+static int vhost_run_work_kthread_list(void *data)
>+{
>+	struct vhost_worker *worker = data;
>+	struct vhost_work *work, *work_next;
>+	struct vhost_dev *dev = worker->dev;
>+	struct llist_node *node;
>+
>+	kthread_use_mm(dev->mm);
>+
>+	for (;;) {
>+		/* mb paired w/ kthread_stop */
>+		set_current_state(TASK_INTERRUPTIBLE);
>+
>+		if (kthread_should_stop()) {
>+			__set_current_state(TASK_RUNNING);
>+			break;
>+		}
>+		node = llist_del_all(&worker->work_list);
>+		if (!node)
>+			schedule();
>+
>+		node = llist_reverse_order(node);
>+		/* make sure flag is seen after deletion */
>+		smp_wmb();
>+		llist_for_each_entry_safe(work, work_next, node, node) {
>+			clear_bit(VHOST_WORK_QUEUED, &work->flags);
>+			__set_current_state(TASK_RUNNING);
>+			kcov_remote_start_common(worker->kcov_handle);
>+			work->fn(work);
>+			kcov_remote_stop();
>+			cond_resched();
>+		}
>+	}
>+	kthread_unuse_mm(dev->mm);
>+
>+	return 0;
>+}
>+
> static bool vhost_run_work_list(void *data)
> {
> 	struct vhost_worker *worker = data;
>-- 
>2.45.0
>

Re: [PATCH v4 2/8] vhost: Add the vhost_worker to support kthread

[Cindy Lu]

On Wed, Dec 11, 2024 at 1:52 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Wed, Dec 11, 2024 at 12:41:41AM +0800, Cindy Lu wrote:
> >Add the previously removed function vhost_worker() back
> >to support the kthread and rename it to vhost_run_work_kthread_list.
> >
> >The old function vhost_worker was change to support task in
>
> s/change/changed
>
will fix this
> >commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
> >change to xarray in
>
> "and to support multiple workers per device using xarray in"
>
will fix this
Thanks
Cindy
> >commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")

> >
> >Signed-off-by: Cindy Lu <lulu@redhat.com>
> >---
> > drivers/vhost/vhost.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index eaddbd39c29b..1feba29abf95 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -388,6 +388,44 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> >       __vhost_vq_meta_reset(vq);
> > }
> >
> >+static int vhost_run_work_kthread_list(void *data)
> >+{
> >+      struct vhost_worker *worker = data;
> >+      struct vhost_work *work, *work_next;
> >+      struct vhost_dev *dev = worker->dev;
> >+      struct llist_node *node;
> >+
> >+      kthread_use_mm(dev->mm);
> >+
> >+      for (;;) {
> >+              /* mb paired w/ kthread_stop */
> >+              set_current_state(TASK_INTERRUPTIBLE);
> >+
> >+              if (kthread_should_stop()) {
> >+                      __set_current_state(TASK_RUNNING);
> >+                      break;
> >+              }
> >+              node = llist_del_all(&worker->work_list);
> >+              if (!node)
> >+                      schedule();
> >+
> >+              node = llist_reverse_order(node);
> >+              /* make sure flag is seen after deletion */
> >+              smp_wmb();
> >+              llist_for_each_entry_safe(work, work_next, node, node) {
> >+                      clear_bit(VHOST_WORK_QUEUED, &work->flags);
> >+                      __set_current_state(TASK_RUNNING);
> >+                      kcov_remote_start_common(worker->kcov_handle);
> >+                      work->fn(work);
> >+                      kcov_remote_stop();
> >+                      cond_resched();
> >+              }
> >+      }
> >+      kthread_unuse_mm(dev->mm);
> >+
> >+      return 0;
> >+}
> >+
> > static bool vhost_run_work_list(void *data)
> > {
> >       struct vhost_worker *worker = data;
> >--
> >2.45.0
> >
>

[PATCH v4 3/8] vhost: Add the cgroup related function

[Cindy Lu]

Add back the previously removed cgroup function to support the kthread
The biggest change for this part is in vhost_attach_cgroups() and
vhost_attach_task_to_cgroups().

The old function was remove in
commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1feba29abf95..812dfd218bc2 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/kthread.h>
+#include <linux/cgroup.h>
 #include <linux/module.h>
 #include <linux/sort.h>
 #include <linux/sched/mm.h>
@@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
 }
 EXPORT_SYMBOL_GPL(vhost_dev_check_owner);
 
+struct vhost_attach_cgroups_struct {
+	struct vhost_work work;
+	struct task_struct *owner;
+	int ret;
+};
+
+static void vhost_attach_cgroups_work(struct vhost_work *work)
+{
+	struct vhost_attach_cgroups_struct *s;
+
+	s = container_of(work, struct vhost_attach_cgroups_struct, work);
+	s->ret = cgroup_attach_task_all(s->owner, current);
+}
+
+static int vhost_attach_task_to_cgroups(struct vhost_worker *worker)
+{
+	struct vhost_flush_struct flush;
+	struct vhost_attach_cgroups_struct attach;
+
+	attach.owner = current;
+
+	vhost_work_init(&attach.work, vhost_attach_cgroups_work);
+	vhost_worker_queue(worker, &attach.work);
+
+	init_completion(&flush.wait_event);
+	vhost_work_init(&flush.work, vhost_flush_work);
+	vhost_worker_queue(worker, &flush.work);
+	wait_for_completion(&flush.wait_event);
+
+	return attach.ret;
+}
+
 /* Caller should have device mutex */
 bool vhost_dev_has_owner(struct vhost_dev *dev)
 {
-- 
2.45.0

Re: [PATCH v4 3/8] vhost: Add the cgroup related function

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:42AM +0800, Cindy Lu wrote:
>Add back the previously removed cgroup function to support the kthread

nit: missing . at the end

>The biggest change for this part is in vhost_attach_cgroups() and
>vhost_attach_task_to_cgroups().

It's not clear what the big change is, is there a piece missing?

>
>The old function was remove in

s/remove/removed

BTW which is the old function?

>commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 1feba29abf95..812dfd218bc2 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -22,6 +22,7 @@
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
> #include <linux/kthread.h>
>+#include <linux/cgroup.h>
> #include <linux/module.h>
> #include <linux/sort.h>
> #include <linux/sched/mm.h>
>@@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
> }
> EXPORT_SYMBOL_GPL(vhost_dev_check_owner);
>
>+struct vhost_attach_cgroups_struct {
>+	struct vhost_work work;
>+	struct task_struct *owner;
>+	int ret;
>+};
>+
>+static void vhost_attach_cgroups_work(struct vhost_work *work)
>+{
>+	struct vhost_attach_cgroups_struct *s;
>+
>+	s = container_of(work, struct vhost_attach_cgroups_struct, work);
>+	s->ret = cgroup_attach_task_all(s->owner, current);
>+}
>+
>+static int vhost_attach_task_to_cgroups(struct vhost_worker *worker)

This function looks renamed, should we mention in the commit 
description?

>+{
>+	struct vhost_flush_struct flush;
>+	struct vhost_attach_cgroups_struct attach;
>+
>+	attach.owner = current;
>+
>+	vhost_work_init(&attach.work, vhost_attach_cgroups_work);
>+	vhost_worker_queue(worker, &attach.work);
>+
>+	init_completion(&flush.wait_event);
>+	vhost_work_init(&flush.work, vhost_flush_work);
>+	vhost_worker_queue(worker, &flush.work);
>+	wait_for_completion(&flush.wait_event);

IIUC this block is the old "vhost_dev_flush", right?

Maybe we should mention also that in the commit description.

>+
>+	return attach.ret;
>+}
>+
> /* Caller should have device mutex */
> bool vhost_dev_has_owner(struct vhost_dev *dev)
> {
>-- 
>2.45.0
>

Re: [PATCH v4 3/8] vhost: Add the cgroup related function

[Cindy Lu]

On Wed, Dec 11, 2024 at 1:53 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Wed, Dec 11, 2024 at 12:41:42AM +0800, Cindy Lu wrote:
> >Add back the previously removed cgroup function to support the kthread
>
> nit: missing . at the end
>
will fix this
> >The biggest change for this part is in vhost_attach_cgroups() and
> >vhost_attach_task_to_cgroups().
>
> It's not clear what the big change is, is there a piece missing?
>
sure, I will rewrite this part to make it more clear
Thanks
cindy
> >
> >The old function was remove in
>
> s/remove/removed
>
> BTW which is the old function?
>
I will add this information
Thanks
cindy
> >commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
> >
> >Signed-off-by: Cindy Lu <lulu@redhat.com>
> >---
> > drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index 1feba29abf95..812dfd218bc2 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -22,6 +22,7 @@
> > #include <linux/slab.h>
> > #include <linux/vmalloc.h>
> > #include <linux/kthread.h>
> >+#include <linux/cgroup.h>
> > #include <linux/module.h>
> > #include <linux/sort.h>
> > #include <linux/sched/mm.h>
> >@@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
> > }
> > EXPORT_SYMBOL_GPL(vhost_dev_check_owner);
> >
> >+struct vhost_attach_cgroups_struct {
> >+      struct vhost_work work;
> >+      struct task_struct *owner;
> >+      int ret;
> >+};
> >+
> >+static void vhost_attach_cgroups_work(struct vhost_work *work)
> >+{
> >+      struct vhost_attach_cgroups_struct *s;
> >+
> >+      s = container_of(work, struct vhost_attach_cgroups_struct, work);
> >+      s->ret = cgroup_attach_task_all(s->owner, current);
> >+}
> >+
> >+static int vhost_attach_task_to_cgroups(struct vhost_worker *worker)
>
> This function looks renamed, should we mention in the commit
> description?
>
> >+{
> >+      struct vhost_flush_struct flush;
> >+      struct vhost_attach_cgroups_struct attach;
> >+
> >+      attach.owner = current;
> >+
> >+      vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> >+      vhost_worker_queue(worker, &attach.work);
> >+
> >+      init_completion(&flush.wait_event);
> >+      vhost_work_init(&flush.work, vhost_flush_work);
> >+      vhost_worker_queue(worker, &flush.work);
> >+      wait_for_completion(&flush.wait_event);
>
> IIUC this block is the old "vhost_dev_flush", right?
>
> Maybe we should mention also that in the commit description.
>
sure, will add these informations
Thanks
cindy
> >+
> >+      return attach.ret;
> >+}
> >+
> > /* Caller should have device mutex */
> > bool vhost_dev_has_owner(struct vhost_dev *dev)
> > {
> >--
> >2.45.0
> >
>

[PATCH v4 4/8] vhost: Add kthread support in function vhost_worker_create

[Cindy Lu]

Restored the previous functions kthread_wakeup and kthread_stop.
Also add 2 new function pointer. The function vhost_worker_create
Will initializes this pointer based on the value of inherit_owner.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 84 +++++++++++++++++++++++++++++++++++--------
 drivers/vhost/vhost.h |  3 ++
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 812dfd218bc2..0175bbf4d8b3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -721,14 +721,38 @@ static void vhost_workers_free(struct vhost_dev *dev)
 	xa_destroy(&dev->worker_xa);
 }
 
+static int vhost_task_wakeup_fn(struct vhost_worker *worker)
+{
+	vhost_task_wake(worker->vtsk);
+	return 0;
+}
+
+static int vhost_kthread_wakeup_fn(struct vhost_worker *worker)
+{
+	return wake_up_process(worker->kthread_task);
+}
+
+static int vhost_task_stop_fn(struct vhost_worker *worker)
+{
+	vhost_task_stop(worker->vtsk);
+	return 0;
+}
+
+static int vhost_kthread_stop_fn(struct vhost_worker *worker)
+{
+	return kthread_stop(worker->kthread_task);
+}
+
 static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
 {
 	struct vhost_worker *worker;
-	struct vhost_task *vtsk;
+	struct vhost_task *vtsk = NULL;
+	struct task_struct *task = NULL;
 	char name[TASK_COMM_LEN];
 	int ret;
 	u32 id;
 
+	/* Allocate resources for the worker */
 	worker = kzalloc(sizeof(*worker), GFP_KERNEL_ACCOUNT);
 	if (!worker)
 		return NULL;
@@ -736,27 +760,59 @@ static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
 	worker->dev = dev;
 	snprintf(name, sizeof(name), "vhost-%d", current->pid);
 
-	vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
-				 worker, name);
-	if (!vtsk)
-		goto free_worker;
-
 	mutex_init(&worker->mutex);
 	init_llist_head(&worker->work_list);
 	worker->kcov_handle = kcov_common_handle();
-	worker->vtsk = vtsk;
 
-	vhost_task_start(vtsk);
+	if (dev->inherit_owner) {
+		/*
+		 * If inherit_owner is true we use vhost_tasks to create
+		 * the worker so all settings/limits like cgroups, NPROC,
+		 * scheduler, etc are inherited from the owner. If false,
+		 * we use kthreads and only attach to the same cgroups
+		 * as the owner for compat with older kernels.
+		 */
+		vtsk = vhost_task_create(vhost_run_work_list,
+					 vhost_worker_killed, worker, name);
+		if (!vtsk)
+			goto free_worker;
+
+		worker->vtsk = vtsk;
+		worker->task_wakeup = vhost_task_wakeup_fn;
+		worker->task_stop = vhost_task_stop_fn;
+
+		vhost_task_start(vtsk);
+		ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b,
+			       GFP_KERNEL);
+		if (ret < 0)
+			goto stop_worker;
+	} else {
+		/* Create and start a kernel thread */
+		task = kthread_create(vhost_run_work_kthread_list, worker,
+				      "vhost-%d", current->pid);
+		if (IS_ERR(task)) {
+			ret = PTR_ERR(task);
+			goto free_worker;
+		}
+		worker->kthread_task = task;
+		worker->task_wakeup = vhost_kthread_wakeup_fn;
+		worker->task_stop = vhost_kthread_stop_fn;
 
-	ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
-	if (ret < 0)
-		goto stop_worker;
-	worker->id = id;
+		wake_up_process(task);
+		ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b,
+			       GFP_KERNEL);
+		if (ret < 0)
+			goto stop_worker;
 
-	return worker;
+		ret = vhost_attach_task_to_cgroups(worker);
+		if (ret)
+			goto stop_worker;
+	}
 
+	worker->id = id;
+	return worker;
 stop_worker:
-	vhost_task_stop(vtsk);
+	worker->task_stop(worker);
 free_worker:
 	kfree(worker);
 	return NULL;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index c650c4506c70..a7dc6e168753 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -27,6 +27,7 @@ struct vhost_work {
 };
 
 struct vhost_worker {
+	struct task_struct *kthread_task;
 	struct vhost_task	*vtsk;
 	struct vhost_dev	*dev;
 	/* Used to serialize device wide flushing with worker swapping. */
@@ -36,6 +37,8 @@ struct vhost_worker {
 	u32			id;
 	int			attachment_cnt;
 	bool			killed;
+	int (*task_wakeup)(struct vhost_worker *worker);
+	int (*task_stop)(struct vhost_worker *worker);
 };
 
 /* Poll a file (eventfd or socket) */
-- 
2.45.0

Re: [PATCH v4 4/8] vhost: Add kthread support in function vhost_worker_create

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:43AM +0800, Cindy Lu wrote:
>Restored the previous functions kthread_wakeup and kthread_stop.

nit: "Add back the previously removed"

>Also add 2 new function pointer. 

"Also add 2 new function pointers to wakeup and stop the workers."

> The function vhost_worker_create
>Will initializes this pointer based on the value of inherit_owner.

nit: s/Will/will

>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c | 84 +++++++++++++++++++++++++++++++++++--------
> drivers/vhost/vhost.h |  3 ++
> 2 files changed, 73 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 812dfd218bc2..0175bbf4d8b3 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -721,14 +721,38 @@ static void vhost_workers_free(struct vhost_dev *dev)
> 	xa_destroy(&dev->worker_xa);
> }
>
>+static int vhost_task_wakeup_fn(struct vhost_worker *worker)
>+{
>+	vhost_task_wake(worker->vtsk);
>+	return 0;
>+}
>+
>+static int vhost_kthread_wakeup_fn(struct vhost_worker *worker)
>+{
>+	return wake_up_process(worker->kthread_task);
>+}
>+
>+static int vhost_task_stop_fn(struct vhost_worker *worker)
>+{
>+	vhost_task_stop(worker->vtsk);
>+	return 0;
>+}
>+
>+static int vhost_kthread_stop_fn(struct vhost_worker *worker)
>+{
>+	return kthread_stop(worker->kthread_task);
>+}
>+
> static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
> {
> 	struct vhost_worker *worker;
>-	struct vhost_task *vtsk;
>+	struct vhost_task *vtsk = NULL;
>+	struct task_struct *task = NULL;
> 	char name[TASK_COMM_LEN];
> 	int ret;
> 	u32 id;
>
>+	/* Allocate resources for the worker */
> 	worker = kzalloc(sizeof(*worker), GFP_KERNEL_ACCOUNT);
> 	if (!worker)
> 		return NULL;
>@@ -736,27 +760,59 @@ static struct vhost_worker 
>*vhost_worker_create(struct vhost_dev *dev)
> 	worker->dev = dev;
> 	snprintf(name, sizeof(name), "vhost-%d", current->pid);
>
>-	vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
>-				 worker, name);
>-	if (!vtsk)
>-		goto free_worker;
>-
> 	mutex_init(&worker->mutex);
> 	init_llist_head(&worker->work_list);
> 	worker->kcov_handle = kcov_common_handle();
>-	worker->vtsk = vtsk;
>
>-	vhost_task_start(vtsk);
>+	if (dev->inherit_owner) {
>+		/*
>+		 * If inherit_owner is true we use vhost_tasks to create
>+		 * the worker so all settings/limits like cgroups, NPROC,
>+		 * scheduler, etc are inherited from the owner. If false,
>+		 * we use kthreads and only attach to the same cgroups
>+		 * as the owner for compat with older kernels.
>+		 */
>+		vtsk = vhost_task_create(vhost_run_work_list,
>+					 vhost_worker_killed, worker, name);
>+		if (!vtsk)
>+			goto free_worker;
>+
>+		worker->vtsk = vtsk;
>+		worker->task_wakeup = vhost_task_wakeup_fn;
>+		worker->task_stop = vhost_task_stop_fn;
>+
>+		vhost_task_start(vtsk);
>+		ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b,
>+			       GFP_KERNEL);
>+		if (ret < 0)
>+			goto stop_worker;
>+	} else {
>+		/* Create and start a kernel thread */

I would move here the comment included in the previous branch:
"If false we use kthreads and only attach to... "

Or move the entire comment block before the if.

>+		task = kthread_create(vhost_run_work_kthread_list, worker,
>+				      "vhost-%d", current->pid);
>+		if (IS_ERR(task)) {
>+			ret = PTR_ERR(task);
>+			goto free_worker;
>+		}
>+		worker->kthread_task = task;
>+		worker->task_wakeup = vhost_kthread_wakeup_fn;
>+		worker->task_stop = vhost_kthread_stop_fn;
>
>-	ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
>-	if (ret < 0)
>-		goto stop_worker;
>-	worker->id = id;
>+		wake_up_process(task);
>+		ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b,
>+			       GFP_KERNEL);
>+		if (ret < 0)
>+			goto stop_worker;
>
>-	return worker;
>+		ret = vhost_attach_task_to_cgroups(worker);
>+		if (ret)
>+			goto stop_worker;
>+	}
>
>+	worker->id = id;
>+	return worker;
> stop_worker:
>-	vhost_task_stop(vtsk);
>+	worker->task_stop(worker);
> free_worker:
> 	kfree(worker);
> 	return NULL;
>diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>index c650c4506c70..a7dc6e168753 100644
>--- a/drivers/vhost/vhost.h
>+++ b/drivers/vhost/vhost.h
>@@ -27,6 +27,7 @@ struct vhost_work {
> };
>
> struct vhost_worker {
>+	struct task_struct *kthread_task;
> 	struct vhost_task	*vtsk;
> 	struct vhost_dev	*dev;
> 	/* Used to serialize device wide flushing with worker swapping. */
>@@ -36,6 +37,8 @@ struct vhost_worker {
> 	u32			id;
> 	int			attachment_cnt;
> 	bool			killed;
>+	int (*task_wakeup)(struct vhost_worker *worker);
>+	int (*task_stop)(struct vhost_worker *worker);

We never read the return values of these functions, so either mark them 
both void or check their return value.

What about renaming in worker_wakeup and worker_stop?
Or even better since they are part of vhost_worker, just wakeup and 
stop.

Thanks,
Stefano

> };
>
> /* Poll a file (eventfd or socket) */
>-- 
>2.45.0
>

[PATCH v4 5/8] vhost: Add kthread support in function vhost_worker_queue()

[Cindy Lu]

The function vhost_worker_queue() uses a function pointer in
vhost_worker, which is initialized based on the inherit_owner
value.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 0175bbf4d8b3..d1aec41bcd56 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -237,13 +237,16 @@ EXPORT_SYMBOL_GPL(vhost_poll_stop);
 static void vhost_worker_queue(struct vhost_worker *worker,
 			       struct vhost_work *work)
 {
+	if (!worker)
+		return;
+
 	if (!test_and_set_bit(VHOST_WORK_QUEUED, &work->flags)) {
 		/* We can only add the work to the list after we're
 		 * sure it was not in the list.
 		 * test_and_set_bit() implies a memory barrier.
 		 */
 		llist_add(&work->node, &worker->work_list);
-		vhost_task_wake(worker->vtsk);
+		worker->task_wakeup(worker);
 	}
 }
 
-- 
2.45.0

Re: [PATCH v4 5/8] vhost: Add kthread support in function vhost_worker_queue()

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:44AM +0800, Cindy Lu wrote:
>The function vhost_worker_queue() uses a function pointer in
>vhost_worker, which is initialized based on the inherit_owner
>value.
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 0175bbf4d8b3..d1aec41bcd56 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -237,13 +237,16 @@ EXPORT_SYMBOL_GPL(vhost_poll_stop);
> static void vhost_worker_queue(struct vhost_worker *worker,
> 			       struct vhost_work *work)
> {
>+	if (!worker)
>+		return;
>+

In which scenario can `worker` be NULL?

I would like to better understand why it couldn't happen before and now 
it can.

Thanks,
Stefano

> 	if (!test_and_set_bit(VHOST_WORK_QUEUED, &work->flags)) {
> 		/* We can only add the work to the list after we're
> 		 * sure it was not in the list.
> 		 * test_and_set_bit() implies a memory barrier.
> 		 */
> 		llist_add(&work->node, &worker->work_list);
>-		vhost_task_wake(worker->vtsk);
>+		worker->task_wakeup(worker);
> 	}
> }
>
>-- 
>2.45.0
>

[PATCH v4 6/8] vhost: Add kthread support in function vhost_worker_destroy()

[Cindy Lu]

The function vhost_worker_destroy() uses a function pointer in
vhost_worker, which is initialized based on the inherit_owner
value.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d1aec41bcd56..3e9cb99da1b5 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -701,7 +701,7 @@ static void vhost_worker_destroy(struct vhost_dev *dev,
 
 	WARN_ON(!llist_empty(&worker->work_list));
 	xa_erase(&dev->worker_xa, worker->id);
-	vhost_task_stop(worker->vtsk);
+	worker->task_stop(worker);
 	kfree(worker);
 }
 
-- 
2.45.0

[PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Cindy Lu]

Add a new UAPI to enable setting the vhost device to task mode.
The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
to configure the mode if necessary.
This setting must be applied before VHOST_SET_OWNER, as the worker
will be created in the VHOST_SET_OWNER function

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
 include/uapi/linux/vhost.h | 18 ++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3e9cb99da1b5..12c3bf3d1ed4 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
 {
 	struct eventfd_ctx *ctx;
 	u64 p;
-	long r;
+	long r = 0;
 	int i, fd;
+	u8 inherit_owner;
 
 	/* If you are not the owner, you can become one */
 	if (ioctl == VHOST_SET_OWNER) {
 		r = vhost_dev_set_owner(d);
 		goto done;
 	}
+	if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
+		/*inherit_owner can only be modified before owner is set*/
+		if (vhost_dev_has_owner(d)) {
+			r = -EBUSY;
+			goto done;
+		}
+		if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
+			r = -EFAULT;
+			goto done;
+		}
+		/* Validate the inherit_owner value, ensuring it is either 0 or 1 */
+		if (inherit_owner > 1) {
+			r = -EINVAL;
+			goto done;
+		}
+
+		d->inherit_owner = (bool)inherit_owner;
 
+		goto done;
+	}
 	/* You must be the owner to do anything else */
 	r = vhost_dev_check_owner(d);
 	if (r)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index b95dd84eef2d..d7564d62b76d 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -235,4 +235,22 @@
  */
 #define VHOST_VDPA_GET_VRING_SIZE	_IOWR(VHOST_VIRTIO, 0x82,	\
 					      struct vhost_vring_state)
+
+/**
+ * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
+ *
+ * @param inherit_owner: An 8-bit value that determines the vhost thread mode
+ *
+ * When inherit_owner is set to 1:
+ *   - The VHOST worker threads inherit its values/checks from
+ *     the thread that owns the VHOST device, The vhost threads will
+ *     be counted in the nproc rlimits.
+ *
+ * When inherit_owner is set to 0:
+ *   - The VHOST worker threads will use the traditional kernel thread (kthread)
+ *     implementation, which may be preferred by older userspace applications that
+ *     do not utilize the newer vhost_task concept.
+ */
+#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
+
 #endif
-- 
2.45.0

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Stefano Garzarella]

On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
>Add a new UAPI to enable setting the vhost device to task mode.
>The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
>to configure the mode if necessary.
>This setting must be applied before VHOST_SET_OWNER, as the worker
>will be created in the VHOST_SET_OWNER function
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 3e9cb99da1b5..12c3bf3d1ed4 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> {
> 	struct eventfd_ctx *ctx;
> 	u64 p;
>-	long r;
>+	long r = 0;
> 	int i, fd;
>+	u8 inherit_owner;
>
> 	/* If you are not the owner, you can become one */
> 	if (ioctl == VHOST_SET_OWNER) {
> 		r = vhost_dev_set_owner(d);
> 		goto done;
> 	}
>+	if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
>+		/*inherit_owner can only be modified before owner is set*/
>+		if (vhost_dev_has_owner(d)) {
>+			r = -EBUSY;
>+			goto done;
>+		}
>+		if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
>+			r = -EFAULT;
>+			goto done;
>+		}
>+		/* Validate the inherit_owner value, ensuring it is either 0 or 1 */
>+		if (inherit_owner > 1) {
>+			r = -EINVAL;
>+			goto done;
>+		}
>+
>+		d->inherit_owner = (bool)inherit_owner;
>
>+		goto done;
>+	}
> 	/* You must be the owner to do anything else */
> 	r = vhost_dev_check_owner(d);
> 	if (r)
>diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
>index b95dd84eef2d..d7564d62b76d 100644
>--- a/include/uapi/linux/vhost.h
>+++ b/include/uapi/linux/vhost.h
>@@ -235,4 +235,22 @@
>  */
> #define VHOST_VDPA_GET_VRING_SIZE	_IOWR(VHOST_VIRTIO, 0x82,	
> \
> 					      struct vhost_vring_state)
>+
>+/**
>+ * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
>+ *
>+ * @param inherit_owner: An 8-bit value that determines the vhost thread mode
>+ *
>+ * When inherit_owner is set to 1:
>+ *   - The VHOST worker threads inherit its values/checks from
>+ *     the thread that owns the VHOST device, The vhost threads will
>+ *     be counted in the nproc rlimits.

We should mention that this is the default behaviour, so the user does 
not need to call VHOST_SET_INHERIT_FROM_OWNER if the default is okay.

>+ *
>+ * When inherit_owner is set to 0:
>+ *   - The VHOST worker threads will use the traditional kernel thread (kthread)
>+ *     implementation, which may be preferred by older userspace applications that
>+ *     do not utilize the newer vhost_task concept.
>+ */
>+#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)

Do we really need a parameter? I mean could we just have an IOCTL to set 
the old behavior, since the new one is enabled by default?

Not a strong opinion on that, but just an idea to reduce confusion in 
the user. Anyway, if we want the parameter, maybe we can use int instead 
of u8, since we don't particularly care about the length.

Thanks,
Stefano

>+
> #endif
>-- 
>2.45.0
>

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Cindy Lu]

On Wed, Dec 11, 2024 at 1:55 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
> >Add a new UAPI to enable setting the vhost device to task mode.
> >The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> >to configure the mode if necessary.
> >This setting must be applied before VHOST_SET_OWNER, as the worker
> >will be created in the VHOST_SET_OWNER function
> >
> >Signed-off-by: Cindy Lu <lulu@redhat.com>
> >---
> > drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
> > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > 2 files changed, 39 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index 3e9cb99da1b5..12c3bf3d1ed4 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> > {
> >       struct eventfd_ctx *ctx;
> >       u64 p;
> >-      long r;
> >+      long r = 0;
> >       int i, fd;
> >+      u8 inherit_owner;
> >
> >       /* If you are not the owner, you can become one */
> >       if (ioctl == VHOST_SET_OWNER) {
> >               r = vhost_dev_set_owner(d);
> >               goto done;
> >       }
> >+      if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
> >+              /*inherit_owner can only be modified before owner is set*/
> >+              if (vhost_dev_has_owner(d)) {
> >+                      r = -EBUSY;
> >+                      goto done;
> >+              }
> >+              if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
> >+                      r = -EFAULT;
> >+                      goto done;
> >+              }
> >+              /* Validate the inherit_owner value, ensuring it is either 0 or 1 */
> >+              if (inherit_owner > 1) {
> >+                      r = -EINVAL;
> >+                      goto done;
> >+              }
> >+
> >+              d->inherit_owner = (bool)inherit_owner;
> >
> >+              goto done;
> >+      }
> >       /* You must be the owner to do anything else */
> >       r = vhost_dev_check_owner(d);
> >       if (r)
> >diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> >index b95dd84eef2d..d7564d62b76d 100644
> >--- a/include/uapi/linux/vhost.h
> >+++ b/include/uapi/linux/vhost.h
> >@@ -235,4 +235,22 @@
> >  */
> > #define VHOST_VDPA_GET_VRING_SIZE     _IOWR(VHOST_VIRTIO, 0x82,
> > \
> >                                             struct vhost_vring_state)
> >+
> >+/**
> >+ * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
> >+ *
> >+ * @param inherit_owner: An 8-bit value that determines the vhost thread mode
> >+ *
> >+ * When inherit_owner is set to 1:
> >+ *   - The VHOST worker threads inherit its values/checks from
> >+ *     the thread that owns the VHOST device, The vhost threads will
> >+ *     be counted in the nproc rlimits.
>
> We should mention that this is the default behaviour, so the user does
> not need to call VHOST_SET_INHERIT_FROM_OWNER if the default is okay.
>
sure will fix this
thanks
cindy
> >+ *
> >+ * When inherit_owner is set to 0:
> >+ *   - The VHOST worker threads will use the traditional kernel thread (kthread)
> >+ *     implementation, which may be preferred by older userspace applications that
> >+ *     do not utilize the newer vhost_task concept.
> >+ */
> >+#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
>
> Do we really need a parameter? I mean could we just have an IOCTL to set
> the old behavior, since the new one is enabled by default?
>
> Not a strong opinion on that, but just an idea to reduce confusion in
> the user. Anyway, if we want the parameter, maybe we can use int instead
> of u8, since we don't particularly care about the length.
>
> Thanks,
> Stefano
>
I think we could keep this, just in case the userspace app needs to
switch back from kthread mode.
Thanks
Cindy
> >+
> > #endif
> >--
> >2.45.0
> >
>

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Michael S. Tsirkin]

On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
> Add a new UAPI to enable setting the vhost device to task mode.
> The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> to configure the mode if necessary.
> This setting must be applied before VHOST_SET_OWNER, as the worker
> will be created in the VHOST_SET_OWNER function
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>

Good.  I would like to see an option to allow/block this ioctl,
to prevent exceeding nproc limits. A Kconfig option is probably
sufficient.  "allow legacy threading mode" and default to yes?


> ---
>  drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
>  include/uapi/linux/vhost.h | 18 ++++++++++++++++++
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 3e9cb99da1b5..12c3bf3d1ed4 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>  {
>  	struct eventfd_ctx *ctx;
>  	u64 p;
> -	long r;
> +	long r = 0;
>  	int i, fd;
> +	u8 inherit_owner;
>  
>  	/* If you are not the owner, you can become one */
>  	if (ioctl == VHOST_SET_OWNER) {
>  		r = vhost_dev_set_owner(d);
>  		goto done;
>  	}
> +	if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
> +		/*inherit_owner can only be modified before owner is set*/
> +		if (vhost_dev_has_owner(d)) {
> +			r = -EBUSY;
> +			goto done;
> +		}
> +		if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
> +			r = -EFAULT;
> +			goto done;
> +		}
> +		/* Validate the inherit_owner value, ensuring it is either 0 or 1 */
> +		if (inherit_owner > 1) {
> +			r = -EINVAL;
> +			goto done;
> +		}
> +
> +		d->inherit_owner = (bool)inherit_owner;
>  
> +		goto done;
> +	}
>  	/* You must be the owner to do anything else */
>  	r = vhost_dev_check_owner(d);
>  	if (r)
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index b95dd84eef2d..d7564d62b76d 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -235,4 +235,22 @@
>   */
>  #define VHOST_VDPA_GET_VRING_SIZE	_IOWR(VHOST_VIRTIO, 0x82,	\
>  					      struct vhost_vring_state)
> +
> +/**
> + * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
> + *
> + * @param inherit_owner: An 8-bit value that determines the vhost thread mode
> + *
> + * When inherit_owner is set to 1:
> + *   - The VHOST worker threads inherit its values/checks from
> + *     the thread that owns the VHOST device, The vhost threads will
> + *     be counted in the nproc rlimits.
> + *
> + * When inherit_owner is set to 0:
> + *   - The VHOST worker threads will use the traditional kernel thread (kthread)
> + *     implementation, which may be preferred by older userspace applications that
> + *     do not utilize the newer vhost_task concept.
> + */
> +#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> +
>  #endif
> -- 
> 2.45.0

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Cindy Lu]

On Wed, Jan 8, 2025 at 8:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
> > Add a new UAPI to enable setting the vhost device to task mode.
> > The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> > to configure the mode if necessary.
> > This setting must be applied before VHOST_SET_OWNER, as the worker
> > will be created in the VHOST_SET_OWNER function
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
> Good.  I would like to see an option to allow/block this ioctl,
> to prevent exceeding nproc limits. A Kconfig option is probably
> sufficient.  "allow legacy threading mode" and default to yes?
>
sure I will add this in the new version, the legacy thread mode here
is  kthread mode
I will change these commit comments and make it more clear
Thanks
Cindy

>
> > ---
> >  drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
> >  include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 3e9cb99da1b5..12c3bf3d1ed4 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> >  {
> >       struct eventfd_ctx *ctx;
> >       u64 p;
> > -     long r;
> > +     long r = 0;
> >       int i, fd;
> > +     u8 inherit_owner;
> >
> >       /* If you are not the owner, you can become one */
> >       if (ioctl == VHOST_SET_OWNER) {
> >               r = vhost_dev_set_owner(d);
> >               goto done;
> >       }
> > +     if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
> > +             /*inherit_owner can only be modified before owner is set*/
> > +             if (vhost_dev_has_owner(d)) {
> > +                     r = -EBUSY;
> > +                     goto done;
> > +             }
> > +             if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
> > +                     r = -EFAULT;
> > +                     goto done;
> > +             }
> > +             /* Validate the inherit_owner value, ensuring it is either 0 or 1 */
> > +             if (inherit_owner > 1) {
> > +                     r = -EINVAL;
> > +                     goto done;
> > +             }
> > +
> > +             d->inherit_owner = (bool)inherit_owner;
> >
> > +             goto done;
> > +     }
> >       /* You must be the owner to do anything else */
> >       r = vhost_dev_check_owner(d);
> >       if (r)
> > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > index b95dd84eef2d..d7564d62b76d 100644
> > --- a/include/uapi/linux/vhost.h
> > +++ b/include/uapi/linux/vhost.h
> > @@ -235,4 +235,22 @@
> >   */
> >  #define VHOST_VDPA_GET_VRING_SIZE    _IOWR(VHOST_VIRTIO, 0x82,       \
> >                                             struct vhost_vring_state)
> > +
> > +/**
> > + * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
> > + *
> > + * @param inherit_owner: An 8-bit value that determines the vhost thread mode
> > + *
> > + * When inherit_owner is set to 1:
> > + *   - The VHOST worker threads inherit its values/checks from
> > + *     the thread that owns the VHOST device, The vhost threads will
> > + *     be counted in the nproc rlimits.
> > + *
> > + * When inherit_owner is set to 0:
> > + *   - The VHOST worker threads will use the traditional kernel thread (kthread)
> > + *     implementation, which may be preferred by older userspace applications that
> > + *     do not utilize the newer vhost_task concept.
> > + */
> > +#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > +
> >  #endif
> > --
> > 2.45.0
>

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Jason Wang]

On Mon, Jan 13, 2025 at 10:36 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Wed, Jan 8, 2025 at 8:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
> > > Add a new UAPI to enable setting the vhost device to task mode.
> > > The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> > > to configure the mode if necessary.
> > > This setting must be applied before VHOST_SET_OWNER, as the worker
> > > will be created in the VHOST_SET_OWNER function
> > >
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> >
> > Good.  I would like to see an option to allow/block this ioctl,
> > to prevent exceeding nproc limits.

VHOST_SET_INHERIT_FROM_OWNER is for fork() alike behaviour. Without
this ioctl we will go for kthread.

> A Kconfig option is probably
> > sufficient.  "allow legacy threading mode" and default to yes?

How could we block legacy threading mode in this case?

> >
> sure I will add this in the new version, the legacy thread mode here
> is  kthread mode
> I will change these commit comments and make it more clear
> Thanks
> Cindy

Thanks

>
> >
> > > ---
> > >  drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
> > >  include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index 3e9cb99da1b5..12c3bf3d1ed4 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> > >  {
> > >       struct eventfd_ctx *ctx;
> > >       u64 p;
> > > -     long r;
> > > +     long r = 0;
> > >       int i, fd;
> > > +     u8 inherit_owner;
> > >
> > >       /* If you are not the owner, you can become one */
> > >       if (ioctl == VHOST_SET_OWNER) {
> > >               r = vhost_dev_set_owner(d);
> > >               goto done;
> > >       }
> > > +     if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
> > > +             /*inherit_owner can only be modified before owner is set*/
> > > +             if (vhost_dev_has_owner(d)) {
> > > +                     r = -EBUSY;
> > > +                     goto done;
> > > +             }
> > > +             if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
> > > +                     r = -EFAULT;
> > > +                     goto done;
> > > +             }
> > > +             /* Validate the inherit_owner value, ensuring it is either 0 or 1 */
> > > +             if (inherit_owner > 1) {
> > > +                     r = -EINVAL;
> > > +                     goto done;
> > > +             }
> > > +
> > > +             d->inherit_owner = (bool)inherit_owner;
> > >
> > > +             goto done;
> > > +     }
> > >       /* You must be the owner to do anything else */
> > >       r = vhost_dev_check_owner(d);
> > >       if (r)
> > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > > index b95dd84eef2d..d7564d62b76d 100644
> > > --- a/include/uapi/linux/vhost.h
> > > +++ b/include/uapi/linux/vhost.h
> > > @@ -235,4 +235,22 @@
> > >   */
> > >  #define VHOST_VDPA_GET_VRING_SIZE    _IOWR(VHOST_VIRTIO, 0x82,       \
> > >                                             struct vhost_vring_state)
> > > +
> > > +/**
> > > + * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
> > > + *
> > > + * @param inherit_owner: An 8-bit value that determines the vhost thread mode
> > > + *
> > > + * When inherit_owner is set to 1:
> > > + *   - The VHOST worker threads inherit its values/checks from
> > > + *     the thread that owns the VHOST device, The vhost threads will
> > > + *     be counted in the nproc rlimits.
> > > + *
> > > + * When inherit_owner is set to 0:
> > > + *   - The VHOST worker threads will use the traditional kernel thread (kthread)
> > > + *     implementation, which may be preferred by older userspace applications that
> > > + *     do not utilize the newer vhost_task concept.
> > > + */
> > > +#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > > +
> > >  #endif
> > > --
> > > 2.45.0
> >
>

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Cindy Lu]

Ping for review, Hi MST and Jason, do you have any comments on this?
Thanks
Cindy
On Mon, Jan 13, 2025 at 11:09 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Jan 13, 2025 at 10:36 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Wed, Jan 8, 2025 at 8:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Dec 11, 2024 at 12:41:46AM +0800, Cindy Lu wrote:
> > > > Add a new UAPI to enable setting the vhost device to task mode.
> > > > The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> > > > to configure the mode if necessary.
> > > > This setting must be applied before VHOST_SET_OWNER, as the worker
> > > > will be created in the VHOST_SET_OWNER function
> > > >
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > >
> > > Good.  I would like to see an option to allow/block this ioctl,
> > > to prevent exceeding nproc limits.
>
> VHOST_SET_INHERIT_FROM_OWNER is for fork() alike behaviour. Without
> this ioctl we will go for kthread.
>
> > A Kconfig option is probably
> > > sufficient.  "allow legacy threading mode" and default to yes?
>
> How could we block legacy threading mode in this case?
>
> > >
> > sure I will add this in the new version, the legacy thread mode here
> > is  kthread mode
> > I will change these commit comments and make it more clear
> > Thanks
> > Cindy
>
> Thanks
>
> >
> > >
> > > > ---
> > > >  drivers/vhost/vhost.c      | 22 +++++++++++++++++++++-
> > > >  include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index 3e9cb99da1b5..12c3bf3d1ed4 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -2257,15 +2257,35 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> > > >  {
> > > >       struct eventfd_ctx *ctx;
> > > >       u64 p;
> > > > -     long r;
> > > > +     long r = 0;
> > > >       int i, fd;
> > > > +     u8 inherit_owner;
> > > >
> > > >       /* If you are not the owner, you can become one */
> > > >       if (ioctl == VHOST_SET_OWNER) {
> > > >               r = vhost_dev_set_owner(d);
> > > >               goto done;
> > > >       }
> > > > +     if (ioctl == VHOST_SET_INHERIT_FROM_OWNER) {
> > > > +             /*inherit_owner can only be modified before owner is set*/
> > > > +             if (vhost_dev_has_owner(d)) {
> > > > +                     r = -EBUSY;
> > > > +                     goto done;
> > > > +             }
> > > > +             if (copy_from_user(&inherit_owner, argp, sizeof(u8))) {
> > > > +                     r = -EFAULT;
> > > > +                     goto done;
> > > > +             }
> > > > +             /* Validate the inherit_owner value, ensuring it is either 0 or 1 */
> > > > +             if (inherit_owner > 1) {
> > > > +                     r = -EINVAL;
> > > > +                     goto done;
> > > > +             }
> > > > +
> > > > +             d->inherit_owner = (bool)inherit_owner;
> > > >
> > > > +             goto done;
> > > > +     }
> > > >       /* You must be the owner to do anything else */
> > > >       r = vhost_dev_check_owner(d);
> > > >       if (r)
> > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > > > index b95dd84eef2d..d7564d62b76d 100644
> > > > --- a/include/uapi/linux/vhost.h
> > > > +++ b/include/uapi/linux/vhost.h
> > > > @@ -235,4 +235,22 @@
> > > >   */
> > > >  #define VHOST_VDPA_GET_VRING_SIZE    _IOWR(VHOST_VIRTIO, 0x82,       \
> > > >                                             struct vhost_vring_state)
> > > > +
> > > > +/**
> > > > + * VHOST_SET_INHERIT_FROM_OWNER - Set the inherit_owner flag for the vhost device
> > > > + *
> > > > + * @param inherit_owner: An 8-bit value that determines the vhost thread mode
> > > > + *
> > > > + * When inherit_owner is set to 1:
> > > > + *   - The VHOST worker threads inherit its values/checks from
> > > > + *     the thread that owns the VHOST device, The vhost threads will
> > > > + *     be counted in the nproc rlimits.
> > > > + *
> > > > + * When inherit_owner is set to 0:
> > > > + *   - The VHOST worker threads will use the traditional kernel thread (kthread)
> > > > + *     implementation, which may be preferred by older userspace applications that
> > > > + *     do not utilize the newer vhost_task concept.
> > > > + */
> > > > +#define VHOST_SET_INHERIT_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > > > +
> > > >  #endif
> > > > --
> > > > 2.45.0
> > >
> >
>

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Michael S. Tsirkin]

On Wed, Jan 22, 2025 at 10:07:19AM +0800, Cindy Lu wrote:
> Ping for review, Hi MST and Jason, do you have any comments on this?
> Thanks
> Cindy


I see there are unaddressed comments by Jason and Stefano.

Re: [PATCH v4 7/8] vhost: Add new UAPI to support change to task mode

[Cindy Lu]

On Wed, Jan 22, 2025 at 3:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jan 22, 2025 at 10:07:19AM +0800, Cindy Lu wrote:
> > Ping for review, Hi MST and Jason, do you have any comments on this?
> > Thanks
> > Cindy
>
>
> I see there are unaddressed comments by Jason and Stefano.
>
sure, thanks, Mst. I will submit a new version.
Thanks
Cindy

[PATCH v4 8/8] vhost_scsi: Add check for inherit_owner status

[Cindy Lu]

The vhost_scsi VHOST_NEW_WORKER requires the inherit_owner
setting to be true. So we need to implement a check for this.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/scsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 718fa4e0b31e..0d63b6b5c852 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2086,6 +2086,14 @@ vhost_scsi_ioctl(struct file *f,
 			return -EFAULT;
 		return vhost_scsi_set_features(vs, features);
 	case VHOST_NEW_WORKER:
+		/*
+		 * vhost_tasks will account for worker threads under the parent's
+		 * NPROC value but kthreads do not. To avoid userspace overflowing
+		 * the system with worker threads inherit_owner must be true.
+		 */
+		if (!vs->dev.inherit_owner)
+			return -EFAULT;
+		fallthrough;
 	case VHOST_FREE_WORKER:
 	case VHOST_ATTACH_VRING_WORKER:
 	case VHOST_GET_VRING_WORKER:
-- 
2.45.0

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

[Stefano Garzarella]

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
>