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.0On 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
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
>
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
> >
>
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
> > >
> >
>
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.
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
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
>
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
> >
>
© 2016 - 2025 Red Hat, Inc.