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 | 24 ++++++++++++++++++++++--
include/uapi/linux/vhost.h | 18 ++++++++++++++++++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d8c0ea118bb1..45d8f5c5bca9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
int i;
vhost_dev_cleanup(dev);
-
+ dev->inherit_owner = true;
dev->umem = umem;
/* We don't need VQ locks below since vhost_dev_cleanup makes sure
* VQs aren't running.
@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
+
#endif
--
2.45.0On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index d8c0ea118bb1..45d8f5c5bca9 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> int i;
>
> vhost_dev_cleanup(dev);
>-
>+ dev->inherit_owner = true;
> dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
>@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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.
Should we document that this (inherit_owner = 1) is the default, so if
the user doesn't call VHOST_FORK_FROM_OWNER, this mode will be
automatically selected?
Thanks,
Stefano
>+ *
>+ * 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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
>+
> #endif
>--
>2.45.0
>
On Wed, Feb 26, 2025 at 5:05 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > 2 files changed, 40 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index d8c0ea118bb1..45d8f5c5bca9 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > int i;
> >
> > vhost_dev_cleanup(dev);
> >-
> >+ dev->inherit_owner = true;
> > dev->umem = umem;
> > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > * VQs aren't running.
> >@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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.
>
> Should we document that this (inherit_owner = 1) is the default, so if
> the user doesn't call VHOST_FORK_FROM_OWNER, this mode will be
> automatically selected?
>
> Thanks,
> Stefano
sure, I will add 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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> >+
> > #endif
> >--
> >2.45.0
> >
>
On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index d8c0ea118bb1..45d8f5c5bca9 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> int i;
>
> vhost_dev_cleanup(dev);
>-
>+ dev->inherit_owner = true;
> dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
>@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
I don't think we really care of the size of the parameter, so can we
just use `bool` or `unsigned int` or `int` for this IOCTL?
As we did for other IOCTLs where we had to enable/disable something (e.g
VHOST_VSOCK_SET_RUNNING, VHOST_VDPA_SET_VRING_ENABLE).
Thanks,
Stefano
On Tue, Feb 25, 2025 at 7:31 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > 2 files changed, 40 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index d8c0ea118bb1..45d8f5c5bca9 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > int i;
> >
> > vhost_dev_cleanup(dev);
> >-
> >+ dev->inherit_owner = true;
> > dev->umem = umem;
> > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > * VQs aren't running.
> >@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
>
> I don't think we really care of the size of the parameter, so can we
> just use `bool` or `unsigned int` or `int` for this IOCTL?
>
> As we did for other IOCTLs where we had to enable/disable something (e.g
> VHOST_VSOCK_SET_RUNNING, VHOST_VDPA_SET_VRING_ENABLE).
>
hi Stefano
I initially used it as a boolean, but during the code review, the
maintainers considered it was unsuitable for the bool use as the
interface in ioctl (I think in version 3 ?). So I changed it to u8,
then will check if this is 1/0 in ioctl and the u8 should be
sufficient for us to use
Thanks
cindy
> Thanks,
> Stefano
>
Hi Cindy,
On Wed, 26 Feb 2025 at 07:14, Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, Feb 25, 2025 at 7:31 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> > > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > > 2 files changed, 40 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > >index d8c0ea118bb1..45d8f5c5bca9 100644
> > >--- a/drivers/vhost/vhost.c
> > >+++ b/drivers/vhost/vhost.c
> > >@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > > int i;
> > >
> > > vhost_dev_cleanup(dev);
> > >-
> > >+ dev->inherit_owner = true;
> > > dev->umem = umem;
> > > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > > * VQs aren't running.
> > >@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> >
> > I don't think we really care of the size of the parameter, so can we
> > just use `bool` or `unsigned int` or `int` for this IOCTL?
> >
> > As we did for other IOCTLs where we had to enable/disable something (e.g
> > VHOST_VSOCK_SET_RUNNING, VHOST_VDPA_SET_VRING_ENABLE).
> >
> hi Stefano
> I initially used it as a boolean, but during the code review, the
> maintainers considered it was unsuitable for the bool use as the
I see, indeed I found only 1 case of bool:
include/uapi/misc/xilinx_sdfec.h:#define XSDFEC_SET_BYPASS
_IOW(XSDFEC_MAGIC, 9, bool)
> interface in ioctl (I think in version 3 ?). So I changed it to u8,
> then will check if this is 1/0 in ioctl and the u8 should be
> sufficient for us to use
Okay, if Michael and Jason are happy with it, it's fine.
It just seemed strange to me that for other IOCTLs we use int or
unsigned int when we need a boolean instead of a sized type.
Thanks for looking at it,
Stefano
On Wed, Feb 26, 2025 at 09:46:06AM +0100, Stefano Garzarella wrote:
> Hi Cindy,
>
> On Wed, 26 Feb 2025 at 07:14, Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Tue, Feb 25, 2025 at 7:31 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > >
> > > On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> > > > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > > > 2 files changed, 40 insertions(+), 2 deletions(-)
> > > >
> > > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > >index d8c0ea118bb1..45d8f5c5bca9 100644
> > > >--- a/drivers/vhost/vhost.c
> > > >+++ b/drivers/vhost/vhost.c
> > > >@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > > > int i;
> > > >
> > > > vhost_dev_cleanup(dev);
> > > >-
> > > >+ dev->inherit_owner = true;
> > > > dev->umem = umem;
> > > > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > > > * VQs aren't running.
> > > >@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > >
> > > I don't think we really care of the size of the parameter, so can we
> > > just use `bool` or `unsigned int` or `int` for this IOCTL?
> > >
> > > As we did for other IOCTLs where we had to enable/disable something (e.g
> > > VHOST_VSOCK_SET_RUNNING, VHOST_VDPA_SET_VRING_ENABLE).
> > >
> > hi Stefano
> > I initially used it as a boolean, but during the code review, the
> > maintainers considered it was unsuitable for the bool use as the
>
> I see, indeed I found only 1 case of bool:
>
> include/uapi/misc/xilinx_sdfec.h:#define XSDFEC_SET_BYPASS
> _IOW(XSDFEC_MAGIC, 9, bool)
>
> > interface in ioctl (I think in version 3 ?). So I changed it to u8,
> > then will check if this is 1/0 in ioctl and the u8 should be
> > sufficient for us to use
>
> Okay, if Michael and Jason are happy with it, it's fine.
> It just seemed strange to me that for other IOCTLs we use int or
> unsigned int when we need a boolean instead of a sized type.
I only found VHOST_VSOCK_SET_RUNNING. which other ioctls?
> Thanks for looking at it,
> Stefano
On Wed, 26 Feb 2025 at 09:50, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Feb 26, 2025 at 09:46:06AM +0100, Stefano Garzarella wrote:
> > Hi Cindy,
> >
> > On Wed, 26 Feb 2025 at 07:14, Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > On Tue, Feb 25, 2025 at 7:31 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > > >
> > > > On Sun, Feb 23, 2025 at 11:36:20PM +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 | 24 ++++++++++++++++++++++--
> > > > > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > > > > 2 files changed, 40 insertions(+), 2 deletions(-)
> > > > >
> > > > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > >index d8c0ea118bb1..45d8f5c5bca9 100644
> > > > >--- a/drivers/vhost/vhost.c
> > > > >+++ b/drivers/vhost/vhost.c
> > > > >@@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > > > > int i;
> > > > >
> > > > > vhost_dev_cleanup(dev);
> > > > >-
> > > > >+ dev->inherit_owner = true;
> > > > > dev->umem = umem;
> > > > > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > > > > * VQs aren't running.
> > > > >@@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > > >
> > > > I don't think we really care of the size of the parameter, so can we
> > > > just use `bool` or `unsigned int` or `int` for this IOCTL?
> > > >
> > > > As we did for other IOCTLs where we had to enable/disable something (e.g
> > > > VHOST_VSOCK_SET_RUNNING, VHOST_VDPA_SET_VRING_ENABLE).
> > > >
> > > hi Stefano
> > > I initially used it as a boolean, but during the code review, the
> > > maintainers considered it was unsuitable for the bool use as the
> >
> > I see, indeed I found only 1 case of bool:
> >
> > include/uapi/misc/xilinx_sdfec.h:#define XSDFEC_SET_BYPASS
> > _IOW(XSDFEC_MAGIC, 9, bool)
> >
> > > interface in ioctl (I think in version 3 ?). So I changed it to u8,
> > > then will check if this is 1/0 in ioctl and the u8 should be
> > > sufficient for us to use
> >
> > Okay, if Michael and Jason are happy with it, it's fine.
> > It just seemed strange to me that for other IOCTLs we use int or
> > unsigned int when we need a boolean instead of a sized type.
>
> I only found VHOST_VSOCK_SET_RUNNING. which other ioctls?
VHOST_VDPA_SET_VRING_ENABLE use the `struct vhost_vring_state` where we
use the `unsigned int num` field as boolean, but I see that this is a
special case where we re-use the same struct for multiple ioctls.
Thanks,
Stefano
better subject:
vhost: uapi to control task mode (owner vs kthread)
On Sun, Feb 23, 2025 at 11:36:20PM +0800, Cindy Lu wrote:
> Add a new UAPI to enable setting the vhost device to task mode.
better:
Add a new UAPI to configure the vhost device to use the kthread mode
> The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> to configure the mode
... to either owner or kthread.
> 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 | 24 ++++++++++++++++++++++--
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d8c0ea118bb1..45d8f5c5bca9 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> int i;
>
> vhost_dev_cleanup(dev);
> -
> + dev->inherit_owner = true;
> dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
> @@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> +
> #endif
> --
> 2.45.0
On Tue, Feb 25, 2025 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> better subject:
>
> vhost: uapi to control task mode (owner vs kthread)
>
>
> On Sun, Feb 23, 2025 at 11:36:20PM +0800, Cindy Lu wrote:
> > Add a new UAPI to enable setting the vhost device to task mode.
>
> better:
>
> Add a new UAPI to configure the vhost device to use the kthread mode
>
Thanks MST, will change this
>
> > The userspace application can use VHOST_SET_INHERIT_FROM_OWNER
> > to configure the mode
>
> ... to either owner or kthread.
>
sure, will change this
thanks
cindy
>
> > 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 | 24 ++++++++++++++++++++++--
> > include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> > 2 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index d8c0ea118bb1..45d8f5c5bca9 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> > int i;
> >
> > vhost_dev_cleanup(dev);
> > -
> > + dev->inherit_owner = true;
> > dev->umem = umem;
> > /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> > * VQs aren't running.
> > @@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> > +
> > #endif
> > --
> > 2.45.0
>
On Sun, Feb 23, 2025 at 11:36:20PM +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>
Thanks Cindy,
can we add a KConfig knob to disable legacy app support?
It can be handy for security.
Pls make it a patch on top.
> ---
> drivers/vhost/vhost.c | 24 ++++++++++++++++++++++--
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d8c0ea118bb1..45d8f5c5bca9 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> int i;
>
> vhost_dev_cleanup(dev);
> -
> + dev->inherit_owner = true;
> dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
> @@ -2278,15 +2278,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_FORK_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..8f558b433536 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_FORK_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_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> +
> #endif
> --
> 2.45.0
On Sun, Feb 23, 2025 at 11:41 PM Cindy Lu <lulu@redhat.com> 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 | 24 ++++++++++++++++++++++--
> include/uapi/linux/vhost.h | 18 ++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d8c0ea118bb1..45d8f5c5bca9 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1133,7 +1133,7 @@ void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_iotlb *umem)
> int i;
>
> vhost_dev_cleanup(dev);
> -
> + dev->inherit_owner = true;
Any reason this needs to be changed under reset_owner?
> dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
> @@ -2278,15 +2278,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_FORK_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 */
Code explains itself, let's just drop this comment.
> + 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..8f558b433536 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_FORK_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.
Since this is uAPI, it's better to avoid mentioning too many
implementation details. So I would tweak this as.
"Vhost will create tasks similar to processes forked from the owner,
inheriting all of the owner's attributes."
> + *
> + * 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.
"Vhost will create tasks as kernel thread."
> + */
> +#define VHOST_FORK_FROM_OWNER _IOW(VHOST_VIRTIO, 0x83, __u8)
> +
> #endif
> --
> 2.45.0
>
Thanks
© 2016 - 2025 Red Hat, Inc.