Add a new UAPI to support setting the vhost device to
use kthread mode. The user space application needs to use
VHOST_SET_USE_KTHREAD to set the mode. This setting must
be set before VHOST_SET_OWNER is set.
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
drivers/vhost/vhost.c | 11 ++++++++++-
include/uapi/linux/vhost.h | 2 ++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 0a7b2999100f..d6b71bddc272 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2340,14 +2340,23 @@ 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;
+ bool kthread;
/* 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_USE_KTHREAD) {
+ if (copy_from_user(&kthread, argp, sizeof(kthread))) {
+ r = -EFAULT;
+ goto done;
+ }
+ use_kthread = kthread;
+ goto done;
+ }
/* You must be the owner to do anything else */
r = vhost_dev_check_owner(d);
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index b95dd84eef2d..386fe735da63 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -235,4 +235,6 @@
*/
#define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \
struct vhost_vring_state)
+
+#define VHOST_SET_USE_KTHREAD _IOW(VHOST_VIRTIO, 0x83, bool)
#endif
--
2.45.0On Mon, Aug 19, 2024 at 5:29 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Add a new UAPI to support setting the vhost device to
> use kthread mode. The user space application needs to use
> VHOST_SET_USE_KTHREAD to set the mode. This setting must
> be set before VHOST_SET_OWNER is set.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> drivers/vhost/vhost.c | 11 ++++++++++-
> include/uapi/linux/vhost.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 0a7b2999100f..d6b71bddc272 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2340,14 +2340,23 @@ 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;
> + bool kthread;
>
> /* 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_USE_KTHREAD) {
Btw, should we enforce this to be called when there's no owner?
> + if (copy_from_user(&kthread, argp, sizeof(kthread))) {
> + r = -EFAULT;
> + goto done;
> + }
> + use_kthread = kthread;
> + goto done;
> + }
>
> /* You must be the owner to do anything else */
> r = vhost_dev_check_owner(d);
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index b95dd84eef2d..386fe735da63 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -235,4 +235,6 @@
> */
> #define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \
> struct vhost_vring_state)
> +
> +#define VHOST_SET_USE_KTHREAD _IOW(VHOST_VIRTIO, 0x83, bool)
> #endif
> --
> 2.45.0
>
Thanks
On Mon, Aug 19, 2024 at 5:29 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Add a new UAPI to support setting the vhost device to
> use kthread mode. The user space application needs to use
> VHOST_SET_USE_KTHREAD to set the mode. This setting must
> be set before VHOST_SET_OWNER is set.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> drivers/vhost/vhost.c | 11 ++++++++++-
> include/uapi/linux/vhost.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 0a7b2999100f..d6b71bddc272 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2340,14 +2340,23 @@ 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;
> + bool kthread;
>
> /* 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_USE_KTHREAD) {
> + if (copy_from_user(&kthread, argp, sizeof(kthread))) {
> + r = -EFAULT;
> + goto done;
> + }
> + use_kthread = kthread;
> + goto done;
> + }
>
> /* You must be the owner to do anything else */
> r = vhost_dev_check_owner(d);
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index b95dd84eef2d..386fe735da63 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -235,4 +235,6 @@
> */
> #define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \
> struct vhost_vring_state)
> +
Unnecessary changes.
> +#define VHOST_SET_USE_KTHREAD _IOW(VHOST_VIRTIO, 0x83, bool)
So I think we need to do something the opposite. New flag for new
behaviour instead of new flag for the old one ...
By using this we can unbreak the old applications.
Btw, I think this needs to come before/along with the introduction of
the module parameter that enforce old beahviour.
Thanks
> #endif
> --
> 2.45.0
>
On Tue, 27 Aug 2024 at 10:32, Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Aug 19, 2024 at 5:29 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > Add a new UAPI to support setting the vhost device to
> > use kthread mode. The user space application needs to use
> > VHOST_SET_USE_KTHREAD to set the mode. This setting must
> > be set before VHOST_SET_OWNER is set.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > drivers/vhost/vhost.c | 11 ++++++++++-
> > include/uapi/linux/vhost.h | 2 ++
> > 2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 0a7b2999100f..d6b71bddc272 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2340,14 +2340,23 @@ 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;
> > + bool kthread;
> >
> > /* 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_USE_KTHREAD) {
> > + if (copy_from_user(&kthread, argp, sizeof(kthread))) {
> > + r = -EFAULT;
> > + goto done;
> > + }
> > + use_kthread = kthread;
> > + goto done;
> > + }
> >
> > /* You must be the owner to do anything else */
> > r = vhost_dev_check_owner(d);
> > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > index b95dd84eef2d..386fe735da63 100644
> > --- a/include/uapi/linux/vhost.h
> > +++ b/include/uapi/linux/vhost.h
> > @@ -235,4 +235,6 @@
> > */
> > #define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \
> > struct vhost_vring_state)
> > +
>
> Unnecessary changes.
>
> > +#define VHOST_SET_USE_KTHREAD _IOW(VHOST_VIRTIO, 0x83, bool)
>
> So I think we need to do something the opposite. New flag for new
> behaviour instead of new flag for the old one ...
>
> By using this we can unbreak the old applications.
>
> Btw, I think this needs to come before/along with the introduction of
> the module parameter that enforce old behavior.
>
I think the old behavior your mentioned here is use Kthread?
So If I understand correctly, the NEW uapi is something like
VHOST_SET_ENFORCE_TASK
The module parameter can also be changed to.
bool enforce_kthread = true;
module_param(enforce_true, bool, 0444);
thanks
Cindy
> Thanks
>
>
> > #endif
>
> > --
> > 2.45.0
> >
>
On Mon, Aug 19, 2024 at 05:27:33PM +0800, Cindy Lu wrote: > Add a new UAPI to support setting the vhost device to > use kthread mode. The user space application needs to use > VHOST_SET_USE_KTHREAD to set the mode. This setting must > be set before VHOST_SET_OWNER is set. Usage of an API is a complete kernel internal detail that has absolutely no business being exposed to applications. What is the application visible behavior that the API use is the proxy for?
On Wed, Aug 21, 2024 at 1:07 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Aug 19, 2024 at 05:27:33PM +0800, Cindy Lu wrote: > > Add a new UAPI to support setting the vhost device to > > use kthread mode. The user space application needs to use > > VHOST_SET_USE_KTHREAD to set the mode. I think we need a better name. Probably something like VHOST_INHERIT_OWNER or others (not a native speaker) > > This setting must > > be set before VHOST_SET_OWNER is set. > > Usage of an API is a complete kernel internal detail that has absolutely > no business being exposed to applications. > > What is the application visible behavior that the API use is the proxy > for? Vhost used to be created by kthreadd but some recent patches change it to behave like being froked by the owner. So the various attributes that interhit from the parent has been changed (scheduling and namespace etc). Thanks > >
On Mon, Aug 26, 2024 at 02:21:52PM +0800, Jason Wang wrote: > > What is the application visible behavior that the API use is the proxy > > for? > > Vhost used to be created by kthreadd but some recent patches change it > to behave like being froked by the owner. So the various attributes > that interhit from the parent has been changed (scheduling and > namespace etc). Well, if that breaks userspace it needs to be changed to opt into the new behavior rather than requiring a flag to not break the existing applications. Assuming the change is intentional to start with.
On Mon, Aug 26, 2024 at 2:31 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Aug 26, 2024 at 02:21:52PM +0800, Jason Wang wrote: > > > What is the application visible behavior that the API use is the proxy > > > for? > > > > Vhost used to be created by kthreadd but some recent patches change it > > to behave like being froked by the owner. So the various attributes > > that interhit from the parent has been changed (scheduling and > > namespace etc). > > Well, if that breaks userspace it needs to be changed to opt into the > new behavior rather than requiring a flag to not break the existing > applications. Yes, if I was not wrong, this is something this series tries to reach. No flag means old behaviour, new flag means new behaviour. > Assuming the change is intentional to start with. > > Thanks
On Tue, Aug 27, 2024 at 10:09 AM Jason Wang <jasowang@redhat.com> wrote: > > On Mon, Aug 26, 2024 at 2:31 PM Christoph Hellwig <hch@infradead.org> wrote: > > > > On Mon, Aug 26, 2024 at 02:21:52PM +0800, Jason Wang wrote: > > > > What is the application visible behavior that the API use is the proxy > > > > for? > > > > > > Vhost used to be created by kthreadd but some recent patches change it > > > to behave like being froked by the owner. So the various attributes > > > that interhit from the parent has been changed (scheduling and > > > namespace etc). > > > > Well, if that breaks userspace it needs to be changed to opt into the > > new behavior rather than requiring a flag to not break the existing > > applications. > > Yes, if I was not wrong, this is something this series tries to reach. > > No flag means old behaviour, new flag means new behaviour. Ok, I see. The series is trying to do the reverse. We need to fix that. Thanks > > > Assuming the change is intentional to start with. > > > > > > Thanks
On Wed, 21 Aug 2024 at 13:07, Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Aug 19, 2024 at 05:27:33PM +0800, Cindy Lu wrote: > > Add a new UAPI to support setting the vhost device to > > use kthread mode. The user space application needs to use > > VHOST_SET_USE_KTHREAD to set the mode. This setting must > > be set before VHOST_SET_OWNER is set. > > Usage of an API is a complete kernel internal detail that has absolutely > no business being exposed to applications. > > What is the application visible behavior that the API use is the proxy > for? > The userspace application may need to know the details of the kernel. For example, some userspaces may use a script to track threads. If they use task mode, the script may not work properly. In that case, they can choose Kthread mode. Thanks Cindy >
© 2016 - 2026 Red Hat, Inc.