linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/3] vhost: Add support of kthread API
@ 2025-06-09  7:33 Cindy Lu
  2025-06-09  7:33 ` [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread Cindy Lu
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Cindy Lu @ 2025-06-09  7:33 UTC (permalink / raw)
  To: lulu, jasowang, mst, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

In this series, a new UAPI is implemented to allow   
userspace applications to configure their thread 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

Changelog v5:
 1. Change wakeup and stop function pointers in struct vhost_worker to void.
 2. merging patches 4, 5, 6 in a single patch
 3. Fix spelling issues and address other comments.

Changelog v6:
 1. move the check of VHOST_NEW_WORKER from vhost_scsi to vhost
 2. Change the ioctl name VHOST_SET_INHERIT_FROM_OWNER to VHOST_FORK_FROM_OWNER
 3. reuse the function __vhost_worker_flush
 4. use a ops sturct to support worker relates function
 5. reset the value of inherit_owner in vhost_dev_reset_owner.
 
Changelog v7: 
 1. add a KConfig knob to disable legacy app support
 2. Split the changes into two patches to separately introduce the ops and add kthread support.
 3. Utilized INX_MAX to avoid modifications in __vhost_worker_flush
 4. Rebased on the latest kernel
 5. Address other comments
 
Changelog v8: 
 1. Rebased on the latest kernel
 2. Address some other comments 
 
Changelog v9:
 1. Rebased on the latest kernel. 
 2. Squashed patches 6‑7. 
 3. Squashed patches 2‑4. 
 4. Minor fixes in commit log
 
Changelog v10:
 1.Add support for the module_param.
 2.Squash patches 3 and 4.
 3.Make minor fixes in the commit log.
 4.Fix the mismatched tabs in Kconfig.
 5.Rebase on the latest kernel.

Changelog v11:
 1.make the module_param under Kconfig
 2.Make minor fixes in the commit log.
 3.change the name inherit_owner to fork_owner
 4.add NEW ioctl VHOST_GET_FORK_FROM_OWNER
 5.Rebase on the latest kernel
     
Tested with QEMU with kthread mode/task mode/kthread+task mode

Cindy Lu (3):
  vhost: Add a new parameter in vhost_dev to allow user select kthread
  vhost: Reintroduce kthread mode support in vhost
  vhost: Add configuration controls for vhost worker's mode

 drivers/vhost/Kconfig      |  17 +++
 drivers/vhost/vhost.c      | 234 ++++++++++++++++++++++++++++++++++---
 drivers/vhost/vhost.h      |  22 ++++
 include/uapi/linux/vhost.h |  25 ++++
 4 files changed, 280 insertions(+), 18 deletions(-)

-- 
2.45.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread
  2025-06-09  7:33 [PATCH v11 0/3] vhost: Add support of kthread API Cindy Lu
@ 2025-06-09  7:33 ` Cindy Lu
  2025-06-12  6:12   ` Jason Wang
  2025-06-09  7:33 ` [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost Cindy Lu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Cindy Lu @ 2025-06-09  7:33 UTC (permalink / raw)
  To: lulu, jasowang, mst, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

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 applications, 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 |  2 ++
 drivers/vhost/vhost.h | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3a5ebb973dba..ff3052858308 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -41,6 +41,7 @@ static int max_iotlb_entries = 2048;
 module_param(max_iotlb_entries, int, 0444);
 MODULE_PARM_DESC(max_iotlb_entries,
 	"Maximum number of iotlb entries. (default: 2048)");
+static bool fork_from_owner_default = true;
 
 enum {
 	VHOST_MEMORY_F_LOG = 0x1,
@@ -552,6 +553,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->fork_owner = fork_from_owner_default;
 	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..e3d4732883af 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -176,6 +176,16 @@ struct vhost_dev {
 	int byte_weight;
 	struct xarray worker_xa;
 	bool use_worker;
+	/*
+	 * If fork_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.
+	 * here we use true as default value.
+	 * The default value is set by fork_from_owner_default
+	 */
+	bool fork_owner;
 	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 			   struct vhost_iotlb_msg *msg);
 };
-- 
2.45.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost
  2025-06-09  7:33 [PATCH v11 0/3] vhost: Add support of kthread API Cindy Lu
  2025-06-09  7:33 ` [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread Cindy Lu
@ 2025-06-09  7:33 ` Cindy Lu
  2025-06-12  6:13   ` Jason Wang
  2025-06-09  7:33 ` [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode Cindy Lu
  2025-06-12  6:20 ` [PATCH v11 0/3] vhost: Add support of kthread API Michael S. Tsirkin
  3 siblings, 1 reply; 11+ messages in thread
From: Cindy Lu @ 2025-06-09  7:33 UTC (permalink / raw)
  To: lulu, jasowang, mst, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

This patch reintroduces kthread mode support in vhost,
It also introduces struct vhost_worker_ops to abstract
worker create/stop/wakeup operations.

* Bring back the original vhost_worker() implementation,
  and renamed to vhost_run_work_kthread_list().

* Add cgroup support for the kthread

* Introduce struct vhost_worker_ops:
  - Encapsulates create / stop / wake‑up callbacks.
  - vhost_worker_create() selects the proper ops according to
    inherit_owner.

This partially reverts or improves upon:
commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")

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

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ff3052858308..37d3ed8be822 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>
@@ -243,7 +244,7 @@ static void vhost_worker_queue(struct vhost_worker *worker,
 		 * test_and_set_bit() implies a memory barrier.
 		 */
 		llist_add(&work->node, &worker->work_list);
-		vhost_task_wake(worker->vtsk);
+		worker->ops->wakeup(worker);
 	}
 }
 
@@ -389,6 +390,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;
@@ -583,6 +622,46 @@ 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_attach_cgroups_struct attach;
+	int saved_cnt;
+
+	attach.owner = current;
+
+	vhost_work_init(&attach.work, vhost_attach_cgroups_work);
+	vhost_worker_queue(worker, &attach.work);
+
+	mutex_lock(&worker->mutex);
+
+	/*
+	 * Bypass attachment_cnt check in __vhost_worker_flush:
+	 * Temporarily change it to INT_MAX to bypass the check
+	 */
+	saved_cnt = worker->attachment_cnt;
+	worker->attachment_cnt = INT_MAX;
+	__vhost_worker_flush(worker);
+	worker->attachment_cnt = saved_cnt;
+
+	mutex_unlock(&worker->mutex);
+
+	return attach.ret;
+}
+
 /* Caller should have device mutex */
 bool vhost_dev_has_owner(struct vhost_dev *dev)
 {
@@ -628,7 +707,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->ops->stop(worker);
 	kfree(worker);
 }
 
@@ -651,42 +730,115 @@ static void vhost_workers_free(struct vhost_dev *dev)
 	xa_destroy(&dev->worker_xa);
 }
 
+static void vhost_task_wakeup(struct vhost_worker *worker)
+{
+	return vhost_task_wake(worker->vtsk);
+}
+
+static void vhost_kthread_wakeup(struct vhost_worker *worker)
+{
+	wake_up_process(worker->kthread_task);
+}
+
+static void vhost_task_do_stop(struct vhost_worker *worker)
+{
+	return vhost_task_stop(worker->vtsk);
+}
+
+static void vhost_kthread_do_stop(struct vhost_worker *worker)
+{
+	kthread_stop(worker->kthread_task);
+}
+
+static int vhost_task_worker_create(struct vhost_worker *worker,
+				    struct vhost_dev *dev, const char *name)
+{
+	struct vhost_task *vtsk;
+	u32 id;
+	int ret;
+
+	vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
+				 worker, name);
+	if (IS_ERR(vtsk))
+		return PTR_ERR(vtsk);
+
+	worker->vtsk = vtsk;
+	vhost_task_start(vtsk);
+	ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
+	if (ret < 0) {
+		vhost_task_do_stop(worker);
+		return ret;
+	}
+	worker->id = id;
+	return 0;
+}
+
+static int vhost_kthread_worker_create(struct vhost_worker *worker,
+				       struct vhost_dev *dev, const char *name)
+{
+	struct task_struct *task;
+	u32 id;
+	int ret;
+
+	task = kthread_create(vhost_run_work_kthread_list, worker, "%s", name);
+	if (IS_ERR(task))
+		return PTR_ERR(task);
+
+	worker->kthread_task = task;
+	wake_up_process(task);
+	ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
+	if (ret < 0)
+		goto stop_worker;
+
+	ret = vhost_attach_task_to_cgroups(worker);
+	if (ret)
+		goto stop_worker;
+
+	worker->id = id;
+	return 0;
+
+stop_worker:
+	vhost_kthread_do_stop(worker);
+	return ret;
+}
+
+static const struct vhost_worker_ops kthread_ops = {
+	.create = vhost_kthread_worker_create,
+	.stop = vhost_kthread_do_stop,
+	.wakeup = vhost_kthread_wakeup,
+};
+
+static const struct vhost_worker_ops vhost_task_ops = {
+	.create = vhost_task_worker_create,
+	.stop = vhost_task_do_stop,
+	.wakeup = vhost_task_wakeup,
+};
+
 static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
 {
 	struct vhost_worker *worker;
-	struct vhost_task *vtsk;
 	char name[TASK_COMM_LEN];
 	int ret;
-	u32 id;
+	const struct vhost_worker_ops *ops = dev->fork_owner ? &vhost_task_ops :
+							       &kthread_ops;
 
 	worker = kzalloc(sizeof(*worker), GFP_KERNEL_ACCOUNT);
 	if (!worker)
 		return NULL;
 
 	worker->dev = dev;
+	worker->ops = ops;
 	snprintf(name, sizeof(name), "vhost-%d", current->pid);
 
-	vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
-				 worker, name);
-	if (IS_ERR(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);
-
-	ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
+	ret = ops->create(worker, dev, name);
 	if (ret < 0)
-		goto stop_worker;
-	worker->id = id;
+		goto free_worker;
 
 	return worker;
 
-stop_worker:
-	vhost_task_stop(vtsk);
 free_worker:
 	kfree(worker);
 	return NULL;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index e3d4732883af..ab704d84fb34 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -26,7 +26,18 @@ struct vhost_work {
 	unsigned long		flags;
 };
 
+struct vhost_worker;
+struct vhost_dev;
+
+struct vhost_worker_ops {
+	int (*create)(struct vhost_worker *worker, struct vhost_dev *dev,
+		      const char *name);
+	void (*stop)(struct vhost_worker *worker);
+	void (*wakeup)(struct vhost_worker *worker);
+};
+
 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 +47,7 @@ struct vhost_worker {
 	u32			id;
 	int			attachment_cnt;
 	bool			killed;
+	const struct vhost_worker_ops *ops;
 };
 
 /* Poll a file (eventfd or socket) */
-- 
2.45.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode
  2025-06-09  7:33 [PATCH v11 0/3] vhost: Add support of kthread API Cindy Lu
  2025-06-09  7:33 ` [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread Cindy Lu
  2025-06-09  7:33 ` [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost Cindy Lu
@ 2025-06-09  7:33 ` Cindy Lu
  2025-06-12  6:13   ` Jason Wang
  2025-06-12  6:31   ` Michael S. Tsirkin
  2025-06-12  6:20 ` [PATCH v11 0/3] vhost: Add support of kthread API Michael S. Tsirkin
  3 siblings, 2 replies; 11+ messages in thread
From: Cindy Lu @ 2025-06-09  7:33 UTC (permalink / raw)
  To: lulu, jasowang, mst, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

This patch introduces functionality to control the vhost worker mode:

- Add two new IOCTLs:
  * VHOST_SET_FORK_FROM_OWNER: Allows userspace to select between
    task mode (fork_owner=1) and kthread mode (fork_owner=0)
  * VHOST_GET_FORK_FROM_OWNER: Retrieves the current thread mode
    setting

- Expose module parameter 'fork_from_owner_default' to allow system
  administrators to configure the default mode for vhost workers

- Add KConfig option CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL to
  control the availability of these IOCTLs and parameter, allowing
  distributions to disable them if not needed

- The VHOST_NEW_WORKER functionality requires fork_owner to be set
  to true, with validation added to ensure proper configuration

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vhost/Kconfig      | 17 +++++++++++++++
 drivers/vhost/vhost.c      | 44 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vhost.h | 25 ++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 020d4fbb947c..49e1d9dc92b7 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -96,3 +96,20 @@ config VHOST_CROSS_ENDIAN_LEGACY
 	  If unsure, say "N".
 
 endif
+
+config CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
+	bool "Enable CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL"
+	default n
+	help
+	  This option enables two IOCTLs: VHOST_SET_FORK_FROM_OWNER and
+	  VHOST_GET_FORK_FROM_OWNER. These allow userspace applications
+	  to modify the vhost worker mode for vhost devices.
+
+	  Also expose module parameter 'fork_from_owner_default' to allow users
+	  to configure the default mode for vhost workers.
+
+	  By default, `CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL` is set to `n`,
+	  which disables the IOCTLs and parameter.
+	  When enabled (y), users can change the worker thread mode as needed.
+
+	  If unsure, say "N".
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 37d3ed8be822..903d9c3f6784 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -43,6 +43,11 @@ module_param(max_iotlb_entries, int, 0444);
 MODULE_PARM_DESC(max_iotlb_entries,
 	"Maximum number of iotlb entries. (default: 2048)");
 static bool fork_from_owner_default = true;
+#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
+module_param(fork_from_owner_default, bool, 0444);
+MODULE_PARM_DESC(fork_from_owner_default,
+		 "Set task mode as the default(default: Y)");
+#endif
 
 enum {
 	VHOST_MEMORY_F_LOG = 0x1,
@@ -1019,6 +1024,13 @@ long vhost_worker_ioctl(struct vhost_dev *dev, unsigned int ioctl,
 	switch (ioctl) {
 	/* dev worker ioctls */
 	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 fork_owner must be true.
+		 */
+		if (!dev->fork_owner)
+			return -EFAULT;
 		ret = vhost_new_worker(dev, &state);
 		if (!ret && copy_to_user(argp, &state, sizeof(state)))
 			ret = -EFAULT;
@@ -1136,6 +1148,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
 
 	vhost_dev_cleanup(dev);
 
+	dev->fork_owner = fork_from_owner_default;
 	dev->umem = umem;
 	/* We don't need VQ locks below since vhost_dev_cleanup makes sure
 	 * VQs aren't running.
@@ -2289,6 +2302,37 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
 		goto done;
 	}
 
+#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
+	u8 fork_owner;
+
+	if (ioctl == VHOST_SET_FORK_FROM_OWNER) {
+		/*fork_owner can only be modified before owner is set*/
+		if (vhost_dev_has_owner(d)) {
+			r = -EBUSY;
+			goto done;
+		}
+		if (copy_from_user(&fork_owner, argp, sizeof(u8))) {
+			r = -EFAULT;
+			goto done;
+		}
+		if (fork_owner > 1) {
+			r = -EINVAL;
+			goto done;
+		}
+		d->fork_owner = (bool)fork_owner;
+		r = 0;
+		goto done;
+	}
+	if (ioctl == VHOST_GET_FORK_FROM_OWNER) {
+		fork_owner = d->fork_owner;
+		if (copy_to_user(argp, &fork_owner, sizeof(u8))) {
+			r = -EFAULT;
+			goto done;
+		}
+		r = 0;
+		goto done;
+	}
+#endif
 	/* 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 d4b3e2ae1314..e51d6a347607 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -235,4 +235,29 @@
  */
 #define VHOST_VDPA_GET_VRING_SIZE	_IOWR(VHOST_VIRTIO, 0x82,	\
 					      struct vhost_vring_state)
+
+/**
+ * VHOST_SET_FORK_FROM_OWNER - Set the fork_owner flag for the vhost device,
+ * This ioctl must called before VHOST_SET_OWNER.
+ * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
+ *
+ * @param fork_owner: An 8-bit value that determines the vhost thread mode
+ *
+ * When fork_owner is set to 1(default value):
+ *   - Vhost will create vhost worker as tasks forked from the owner,
+ *     inheriting all of the owner's attributes.
+ *
+ * When fork_owner is set to 0:
+ *   - Vhost will create vhost workers as kernel threads.
+ */
+#define VHOST_SET_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
+
+/**
+ * VHOST_GET_FORK_OWNER - Get the current fork_owner flag for the vhost device.
+ * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
+ *
+ * @return: An 8-bit value indicating the current thread mode.
+ */
+#define VHOST_GET_FORK_FROM_OWNER _IOR(VHOST_VIRTIO, 0x84, __u8)
+
 #endif
-- 
2.45.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread
  2025-06-09  7:33 ` [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread Cindy Lu
@ 2025-06-12  6:12   ` Jason Wang
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Wang @ 2025-06-12  6:12 UTC (permalink / raw)
  To: Cindy Lu
  Cc: mst, michael.christie, sgarzare, linux-kernel, virtualization,
	netdev

On Mon, Jun 9, 2025 at 3:34 PM Cindy Lu <lulu@redhat.com> 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 userspace applications, 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>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost
  2025-06-09  7:33 ` [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost Cindy Lu
@ 2025-06-12  6:13   ` Jason Wang
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Wang @ 2025-06-12  6:13 UTC (permalink / raw)
  To: Cindy Lu
  Cc: mst, michael.christie, sgarzare, linux-kernel, virtualization,
	netdev

On Mon, Jun 9, 2025 at 3:34 PM Cindy Lu <lulu@redhat.com> wrote:
>
> This patch reintroduces kthread mode support in vhost,
> It also introduces struct vhost_worker_ops to abstract
> worker create/stop/wakeup operations.
>
> * Bring back the original vhost_worker() implementation,
>   and renamed to vhost_run_work_kthread_list().
>
> * Add cgroup support for the kthread
>
> * Introduce struct vhost_worker_ops:
>   - Encapsulates create / stop / wake‑up callbacks.
>   - vhost_worker_create() selects the proper ops according to
>     inherit_owner.
>
> This partially reverts or improves upon:
> commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
> commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode
  2025-06-09  7:33 ` [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode Cindy Lu
@ 2025-06-12  6:13   ` Jason Wang
  2025-06-12  6:31   ` Michael S. Tsirkin
  1 sibling, 0 replies; 11+ messages in thread
From: Jason Wang @ 2025-06-12  6:13 UTC (permalink / raw)
  To: Cindy Lu
  Cc: mst, michael.christie, sgarzare, linux-kernel, virtualization,
	netdev

On Mon, Jun 9, 2025 at 3:34 PM Cindy Lu <lulu@redhat.com> wrote:
>
> This patch introduces functionality to control the vhost worker mode:
>
> - Add two new IOCTLs:
>   * VHOST_SET_FORK_FROM_OWNER: Allows userspace to select between
>     task mode (fork_owner=1) and kthread mode (fork_owner=0)
>   * VHOST_GET_FORK_FROM_OWNER: Retrieves the current thread mode
>     setting
>
> - Expose module parameter 'fork_from_owner_default' to allow system
>   administrators to configure the default mode for vhost workers
>
> - Add KConfig option CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL to
>   control the availability of these IOCTLs and parameter, allowing
>   distributions to disable them if not needed
>
> - The VHOST_NEW_WORKER functionality requires fork_owner to be set
>   to true, with validation added to ensure proper configuration
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 0/3] vhost: Add support of kthread API
  2025-06-09  7:33 [PATCH v11 0/3] vhost: Add support of kthread API Cindy Lu
                   ` (2 preceding siblings ...)
  2025-06-09  7:33 ` [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode Cindy Lu
@ 2025-06-12  6:20 ` Michael S. Tsirkin
  2025-06-15 14:07   ` Cindy Lu
  3 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2025-06-12  6:20 UTC (permalink / raw)
  To: Cindy Lu
  Cc: jasowang, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

On Mon, Jun 09, 2025 at 03:33:06PM +0800, Cindy Lu wrote:
> In this series, a new UAPI is implemented to allow   
> userspace applications to configure their thread 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
> 
> Changelog v5:
>  1. Change wakeup and stop function pointers in struct vhost_worker to void.
>  2. merging patches 4, 5, 6 in a single patch
>  3. Fix spelling issues and address other comments.
> 
> Changelog v6:
>  1. move the check of VHOST_NEW_WORKER from vhost_scsi to vhost
>  2. Change the ioctl name VHOST_SET_INHERIT_FROM_OWNER to VHOST_FORK_FROM_OWNER
>  3. reuse the function __vhost_worker_flush
>  4. use a ops sturct to support worker relates function
>  5. reset the value of inherit_owner in vhost_dev_reset_owner.
>  
> Changelog v7: 
>  1. add a KConfig knob to disable legacy app support
>  2. Split the changes into two patches to separately introduce the ops and add kthread support.
>  3. Utilized INX_MAX to avoid modifications in __vhost_worker_flush
>  4. Rebased on the latest kernel
>  5. Address other comments
>  
> Changelog v8: 
>  1. Rebased on the latest kernel
>  2. Address some other comments 
>  
> Changelog v9:
>  1. Rebased on the latest kernel. 
>  2. Squashed patches 6‑7. 
>  3. Squashed patches 2‑4. 
>  4. Minor fixes in commit log
>  
> Changelog v10:
>  1.Add support for the module_param.
>  2.Squash patches 3 and 4.
>  3.Make minor fixes in the commit log.
>  4.Fix the mismatched tabs in Kconfig.
>  5.Rebase on the latest kernel.
> 
> Changelog v11:
>  1.make the module_param under Kconfig
>  2.Make minor fixes in the commit log.
>  3.change the name inherit_owner to fork_owner
>  4.add NEW ioctl VHOST_GET_FORK_FROM_OWNER
>  5.Rebase on the latest kernel
>      
> Tested with QEMU with kthread mode/task mode/kthread+task mode
> 
> Cindy Lu (3):
>   vhost: Add a new parameter in vhost_dev to allow user select kthread
>   vhost: Reintroduce kthread mode support in vhost
>   vhost: Add configuration controls for vhost worker's mode


All of this should be squashed in a single patch.

> 
>  drivers/vhost/Kconfig      |  17 +++
>  drivers/vhost/vhost.c      | 234 ++++++++++++++++++++++++++++++++++---
>  drivers/vhost/vhost.h      |  22 ++++
>  include/uapi/linux/vhost.h |  25 ++++
>  4 files changed, 280 insertions(+), 18 deletions(-)
> 
> -- 
> 2.45.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode
  2025-06-09  7:33 ` [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode Cindy Lu
  2025-06-12  6:13   ` Jason Wang
@ 2025-06-12  6:31   ` Michael S. Tsirkin
  2025-06-15 14:06     ` Cindy Lu
  1 sibling, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2025-06-12  6:31 UTC (permalink / raw)
  To: Cindy Lu
  Cc: jasowang, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

On Mon, Jun 09, 2025 at 03:33:09PM +0800, Cindy Lu wrote:
> This patch introduces functionality to control the vhost worker mode:
> 
> - Add two new IOCTLs:
>   * VHOST_SET_FORK_FROM_OWNER: Allows userspace to select between
>     task mode (fork_owner=1) and kthread mode (fork_owner=0)
>   * VHOST_GET_FORK_FROM_OWNER: Retrieves the current thread mode
>     setting
> 
> - Expose module parameter 'fork_from_owner_default' to allow system
>   administrators to configure the default mode for vhost workers
> 
> - Add KConfig option CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL to
>   control the availability of these IOCTLs and parameter, allowing
>   distributions to disable them if not needed
> 
> - The VHOST_NEW_WORKER functionality requires fork_owner to be set
>   to true, with validation added to ensure proper configuration
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>


getting there. yet something to improve.

> ---
>  drivers/vhost/Kconfig      | 17 +++++++++++++++
>  drivers/vhost/vhost.c      | 44 ++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vhost.h | 25 ++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
> 
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 020d4fbb947c..49e1d9dc92b7 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -96,3 +96,20 @@ config VHOST_CROSS_ENDIAN_LEGACY
>  	  If unsure, say "N".
>  
>  endif
> +
> +config CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> +	bool "Enable CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL"
> +	default n
> +	help
> +	  This option enables two IOCTLs: VHOST_SET_FORK_FROM_OWNER and
> +	  VHOST_GET_FORK_FROM_OWNER. These allow userspace applications
> +	  to modify the vhost worker mode for vhost devices.
> +
> +	  Also expose module parameter 'fork_from_owner_default' to allow users
> +	  to configure the default mode for vhost workers.
> +
> +	  By default, `CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL` is set to `n`,
> +	  which disables the IOCTLs and parameter.
> +	  When enabled (y), users can change the worker thread mode as needed.
> +
> +	  If unsure, say "N".
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 37d3ed8be822..903d9c3f6784 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -43,6 +43,11 @@ module_param(max_iotlb_entries, int, 0444);
>  MODULE_PARM_DESC(max_iotlb_entries,
>  	"Maximum number of iotlb entries. (default: 2048)");
>  static bool fork_from_owner_default = true;

Add empty lines around ifdef to make it clear what code do they
delimit.


> +#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> +module_param(fork_from_owner_default, bool, 0444);
> +MODULE_PARM_DESC(fork_from_owner_default,
> +		 "Set task mode as the default(default: Y)");
> +#endif
>  
>  enum {
>  	VHOST_MEMORY_F_LOG = 0x1,
> @@ -1019,6 +1024,13 @@ long vhost_worker_ioctl(struct vhost_dev *dev, unsigned int ioctl,
>  	switch (ioctl) {
>  	/* dev worker ioctls */
>  	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 fork_owner must be true.
> +		 */
> +		if (!dev->fork_owner)
> +			return -EFAULT;

An empty line here would make the code clearer.

>  		ret = vhost_new_worker(dev, &state);
>  		if (!ret && copy_to_user(argp, &state, sizeof(state)))
>  			ret = -EFAULT;
> @@ -1136,6 +1148,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
>  
>  	vhost_dev_cleanup(dev);
>  
> +	dev->fork_owner = fork_from_owner_default;
>  	dev->umem = umem;
>  	/* We don't need VQ locks below since vhost_dev_cleanup makes sure
>  	 * VQs aren't running.
> @@ -2289,6 +2302,37 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
>  		goto done;
>  	}
>  
> +#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> +	u8 fork_owner;

Do not declare variables in the middle of a scope please.
This one is not needed in this scope, so just move it down
to within if (yes you will repeat the declaration twice then).



> +
> +	if (ioctl == VHOST_SET_FORK_FROM_OWNER) {
> +		/*fork_owner can only be modified before owner is set*/

bad comment style.

> +		if (vhost_dev_has_owner(d)) {
> +			r = -EBUSY;
> +			goto done;
> +		}
> +		if (copy_from_user(&fork_owner, argp, sizeof(u8))) {

get_user is a better fit for this. In particular, typesafe.


> +			r = -EFAULT;
> +			goto done;
> +		}
> +		if (fork_owner > 1) {

so 0 and 1 are the only legal values?
maybe add an enum or defines in the header then.


> +			r = -EINVAL;
> +			goto done;
> +		}
> +		d->fork_owner = (bool)fork_owner;

		!!fork_owner is shorter and idiomatic.

> +		r = 0;
> +		goto done;
> +	}
> +	if (ioctl == VHOST_GET_FORK_FROM_OWNER) {
> +		fork_owner = d->fork_owner;
> +		if (copy_to_user(argp, &fork_owner, sizeof(u8))) {

put_user

> +			r = -EFAULT;
> +			goto done;
> +		}
> +		r = 0;
> +		goto done;
> +	}
> +#endif
>  	/* 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 d4b3e2ae1314..e51d6a347607 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -235,4 +235,29 @@
>   */
>  #define VHOST_VDPA_GET_VRING_SIZE	_IOWR(VHOST_VIRTIO, 0x82,	\
>  					      struct vhost_vring_state)
> +
> +/**
> + * VHOST_SET_FORK_FROM_OWNER - Set the fork_owner flag for the vhost device,
> + * This ioctl must called before VHOST_SET_OWNER.
> + * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
> + *
> + * @param fork_owner: An 8-bit value that determines the vhost thread mode
> + *
> + * When fork_owner is set to 1(default value):
> + *   - Vhost will create vhost worker as tasks forked from the owner,
> + *     inheriting all of the owner's attributes.
> + *
> + * When fork_owner is set to 0:
> + *   - Vhost will create vhost workers as kernel threads.
> + */
> +#define VHOST_SET_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> +
> +/**
> + * VHOST_GET_FORK_OWNER - Get the current fork_owner flag for the vhost device.
> + * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
> + *
> + * @return: An 8-bit value indicating the current thread mode.
> + */
> +#define VHOST_GET_FORK_FROM_OWNER _IOR(VHOST_VIRTIO, 0x84, __u8)
> +
>  #endif
> -- 
> 2.45.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode
  2025-06-12  6:31   ` Michael S. Tsirkin
@ 2025-06-15 14:06     ` Cindy Lu
  0 siblings, 0 replies; 11+ messages in thread
From: Cindy Lu @ 2025-06-15 14:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: jasowang, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

On Thu, Jun 12, 2025 at 2:31 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 09, 2025 at 03:33:09PM +0800, Cindy Lu wrote:
> > This patch introduces functionality to control the vhost worker mode:
> >
> > - Add two new IOCTLs:
> >   * VHOST_SET_FORK_FROM_OWNER: Allows userspace to select between
> >     task mode (fork_owner=1) and kthread mode (fork_owner=0)
> >   * VHOST_GET_FORK_FROM_OWNER: Retrieves the current thread mode
> >     setting
> >
> > - Expose module parameter 'fork_from_owner_default' to allow system
> >   administrators to configure the default mode for vhost workers
> >
> > - Add KConfig option CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL to
> >   control the availability of these IOCTLs and parameter, allowing
> >   distributions to disable them if not needed
> >
> > - The VHOST_NEW_WORKER functionality requires fork_owner to be set
> >   to true, with validation added to ensure proper configuration
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
>
> getting there. yet something to improve.
>
> > ---
> >  drivers/vhost/Kconfig      | 17 +++++++++++++++
> >  drivers/vhost/vhost.c      | 44 ++++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/vhost.h | 25 ++++++++++++++++++++++
> >  3 files changed, 86 insertions(+)
> >
> > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> > index 020d4fbb947c..49e1d9dc92b7 100644
> > --- a/drivers/vhost/Kconfig
> > +++ b/drivers/vhost/Kconfig
> > @@ -96,3 +96,20 @@ config VHOST_CROSS_ENDIAN_LEGACY
> >         If unsure, say "N".
> >
> >  endif
> > +
> > +config CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> > +     bool "Enable CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL"
> > +     default n
> > +     help
> > +       This option enables two IOCTLs: VHOST_SET_FORK_FROM_OWNER and
> > +       VHOST_GET_FORK_FROM_OWNER. These allow userspace applications
> > +       to modify the vhost worker mode for vhost devices.
> > +
> > +       Also expose module parameter 'fork_from_owner_default' to allow users
> > +       to configure the default mode for vhost workers.
> > +
> > +       By default, `CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL` is set to `n`,
> > +       which disables the IOCTLs and parameter.
> > +       When enabled (y), users can change the worker thread mode as needed.
> > +
> > +       If unsure, say "N".
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 37d3ed8be822..903d9c3f6784 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -43,6 +43,11 @@ module_param(max_iotlb_entries, int, 0444);
> >  MODULE_PARM_DESC(max_iotlb_entries,
> >       "Maximum number of iotlb entries. (default: 2048)");
> >  static bool fork_from_owner_default = true;
>
> Add empty lines around ifdef to make it clear what code do they
> delimit.
>
>
> > +#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> > +module_param(fork_from_owner_default, bool, 0444);
> > +MODULE_PARM_DESC(fork_from_owner_default,
> > +              "Set task mode as the default(default: Y)");
> > +#endif
> >
> >  enum {
> >       VHOST_MEMORY_F_LOG = 0x1,
> > @@ -1019,6 +1024,13 @@ long vhost_worker_ioctl(struct vhost_dev *dev, unsigned int ioctl,
> >       switch (ioctl) {
> >       /* dev worker ioctls */
> >       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 fork_owner must be true.
> > +              */
> > +             if (!dev->fork_owner)
> > +                     return -EFAULT;
>
> An empty line here would make the code clearer.
>
> >               ret = vhost_new_worker(dev, &state);
> >               if (!ret && copy_to_user(argp, &state, sizeof(state)))
> >                       ret = -EFAULT;
> > @@ -1136,6 +1148,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> >
> >       vhost_dev_cleanup(dev);
> >
> > +     dev->fork_owner = fork_from_owner_default;
> >       dev->umem = umem;
> >       /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> >        * VQs aren't running.
> > @@ -2289,6 +2302,37 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> >               goto done;
> >       }
> >
> > +#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL
> > +     u8 fork_owner;
>
> Do not declare variables in the middle of a scope please.
> This one is not needed in this scope, so just move it down
> to within if (yes you will repeat the declaration twice then).
>
>
>
> > +
> > +     if (ioctl == VHOST_SET_FORK_FROM_OWNER) {
> > +             /*fork_owner can only be modified before owner is set*/
>
> bad comment style.
>
> > +             if (vhost_dev_has_owner(d)) {
> > +                     r = -EBUSY;
> > +                     goto done;
> > +             }
> > +             if (copy_from_user(&fork_owner, argp, sizeof(u8))) {
>
> get_user is a better fit for this. In particular, typesafe.
>
>
> > +                     r = -EFAULT;
> > +                     goto done;
> > +             }
> > +             if (fork_owner > 1) {
>
> so 0 and 1 are the only legal values?
> maybe add an enum or defines in the header then.
>
>
> > +                     r = -EINVAL;
> > +                     goto done;
> > +             }
> > +             d->fork_owner = (bool)fork_owner;
>
>                 !!fork_owner is shorter and idiomatic.
>
> > +             r = 0;
> > +             goto done;
> > +     }
> > +     if (ioctl == VHOST_GET_FORK_FROM_OWNER) {
> > +             fork_owner = d->fork_owner;
> > +             if (copy_to_user(argp, &fork_owner, sizeof(u8))) {
>
> put_user
>
Thanks, Micheal. I'll address all above comments and send a revised version.
Thanks
cindy
> > +                     r = -EFAULT;
> > +                     goto done;
> > +             }
> > +             r = 0;
> > +             goto done;
> > +     }
> > +#endif
> >       /* 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 d4b3e2ae1314..e51d6a347607 100644
> > --- a/include/uapi/linux/vhost.h
> > +++ b/include/uapi/linux/vhost.h
> > @@ -235,4 +235,29 @@
> >   */
> >  #define VHOST_VDPA_GET_VRING_SIZE    _IOWR(VHOST_VIRTIO, 0x82,       \
> >                                             struct vhost_vring_state)
> > +
> > +/**
> > + * VHOST_SET_FORK_FROM_OWNER - Set the fork_owner flag for the vhost device,
> > + * This ioctl must called before VHOST_SET_OWNER.
> > + * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
> > + *
> > + * @param fork_owner: An 8-bit value that determines the vhost thread mode
> > + *
> > + * When fork_owner is set to 1(default value):
> > + *   - Vhost will create vhost worker as tasks forked from the owner,
> > + *     inheriting all of the owner's attributes.
> > + *
> > + * When fork_owner is set to 0:
> > + *   - Vhost will create vhost workers as kernel threads.
> > + */
> > +#define VHOST_SET_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > +
> > +/**
> > + * VHOST_GET_FORK_OWNER - Get the current fork_owner flag for the vhost device.
> > + * Only available when CONFIG_VHOST_ENABLE_FORK_OWNER_CONTROL=y
> > + *
> > + * @return: An 8-bit value indicating the current thread mode.
> > + */
> > +#define VHOST_GET_FORK_FROM_OWNER _IOR(VHOST_VIRTIO, 0x84, __u8)
> > +
> >  #endif
> > --
> > 2.45.0
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v11 0/3] vhost: Add support of kthread API
  2025-06-12  6:20 ` [PATCH v11 0/3] vhost: Add support of kthread API Michael S. Tsirkin
@ 2025-06-15 14:07   ` Cindy Lu
  0 siblings, 0 replies; 11+ messages in thread
From: Cindy Lu @ 2025-06-15 14:07 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: jasowang, michael.christie, sgarzare, linux-kernel,
	virtualization, netdev

On Thu, Jun 12, 2025 at 2:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 09, 2025 at 03:33:06PM +0800, Cindy Lu wrote:
> > In this series, a new UAPI is implemented to allow
> > userspace applications to configure their thread 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
> >
> > Changelog v5:
> >  1. Change wakeup and stop function pointers in struct vhost_worker to void.
> >  2. merging patches 4, 5, 6 in a single patch
> >  3. Fix spelling issues and address other comments.
> >
> > Changelog v6:
> >  1. move the check of VHOST_NEW_WORKER from vhost_scsi to vhost
> >  2. Change the ioctl name VHOST_SET_INHERIT_FROM_OWNER to VHOST_FORK_FROM_OWNER
> >  3. reuse the function __vhost_worker_flush
> >  4. use a ops sturct to support worker relates function
> >  5. reset the value of inherit_owner in vhost_dev_reset_owner.
> >
> > Changelog v7:
> >  1. add a KConfig knob to disable legacy app support
> >  2. Split the changes into two patches to separately introduce the ops and add kthread support.
> >  3. Utilized INX_MAX to avoid modifications in __vhost_worker_flush
> >  4. Rebased on the latest kernel
> >  5. Address other comments
> >
> > Changelog v8:
> >  1. Rebased on the latest kernel
> >  2. Address some other comments
> >
> > Changelog v9:
> >  1. Rebased on the latest kernel.
> >  2. Squashed patches 6‑7.
> >  3. Squashed patches 2‑4.
> >  4. Minor fixes in commit log
> >
> > Changelog v10:
> >  1.Add support for the module_param.
> >  2.Squash patches 3 and 4.
> >  3.Make minor fixes in the commit log.
> >  4.Fix the mismatched tabs in Kconfig.
> >  5.Rebase on the latest kernel.
> >
> > Changelog v11:
> >  1.make the module_param under Kconfig
> >  2.Make minor fixes in the commit log.
> >  3.change the name inherit_owner to fork_owner
> >  4.add NEW ioctl VHOST_GET_FORK_FROM_OWNER
> >  5.Rebase on the latest kernel
> >
> > Tested with QEMU with kthread mode/task mode/kthread+task mode
> >
> > Cindy Lu (3):
> >   vhost: Add a new parameter in vhost_dev to allow user select kthread
> >   vhost: Reintroduce kthread mode support in vhost
> >   vhost: Add configuration controls for vhost worker's mode
>
>
> All of this should be squashed in a single patch.
>
Sure will do
Thanks
cindy
> >
> >  drivers/vhost/Kconfig      |  17 +++
> >  drivers/vhost/vhost.c      | 234 ++++++++++++++++++++++++++++++++++---
> >  drivers/vhost/vhost.h      |  22 ++++
> >  include/uapi/linux/vhost.h |  25 ++++
> >  4 files changed, 280 insertions(+), 18 deletions(-)
> >
> > --
> > 2.45.0
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-06-15 14:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-09  7:33 [PATCH v11 0/3] vhost: Add support of kthread API Cindy Lu
2025-06-09  7:33 ` [PATCH v11 1/3] vhost: Add a new parameter in vhost_dev to allow user select kthread Cindy Lu
2025-06-12  6:12   ` Jason Wang
2025-06-09  7:33 ` [PATCH v11 2/3] vhost: Reintroduce kthread mode support in vhost Cindy Lu
2025-06-12  6:13   ` Jason Wang
2025-06-09  7:33 ` [PATCH v11 3/3] vhost: Add configuration controls for vhost worker's mode Cindy Lu
2025-06-12  6:13   ` Jason Wang
2025-06-12  6:31   ` Michael S. Tsirkin
2025-06-15 14:06     ` Cindy Lu
2025-06-12  6:20 ` [PATCH v11 0/3] vhost: Add support of kthread API Michael S. Tsirkin
2025-06-15 14:07   ` Cindy Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).