[Devel] [PATCH hci-8.1 0/2] vhost: replace VHOST_SET_NWORKERS with the mainstream worker ioctls

[Devel] [PATCH hci-8.1 0/2] vhost: replace VHOST_SET_NWORKERS with the mainstream worker ioctls

[Alexander Ivanov]

VHOST_SET_NWORKERS was added to run our userspace on the rh10 kernel
unchanged. It installs workers bypassing __vhost_vq_attach_worker(), so
VHOST_FREE_WORKER can destroy a worker still in use by a virtqueue.

Patch 1 reverts VHOST_SET_NWORKERS. Patch 2 lets vhost-blk use the
mainstream VHOST_NEW_WORKER/VHOST_ATTACH_VRING_WORKER ioctls instead,
like vhost-scsi does. Userspace has to be updated together with this
series.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Alexander Ivanov (2):
  Revert "drivers/vhost: add ioctl to increase the number of workers"
  drivers/vhost/blk: add support for worker ioctls

 drivers/vhost/blk.c        |  8 ++++++
 drivers/vhost/vhost.c      | 52 +-------------------------------------
 drivers/vhost/vhost.h      |  1 -
 include/uapi/linux/vhost.h |  8 ------
 4 files changed, 9 insertions(+), 60 deletions(-)

-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/2] Revert "drivers/vhost: add ioctl to increase the number of workers"

[Alexander Ivanov]

This reverts commit 1491588e71dafc934c8d450ba36cc33bb0f990bd.

VHOST_SET_NWORKERS was a stopgap to run our userspace on the rh10
kernel unchanged. Userspace should use the mainstream worker ioctls
(VHOST_NEW_WORKER, VHOST_ATTACH_VRING_WORKER etc.) instead.

It is also unsafe next to those ioctls. vhost_set_workers() installs
its workers into vq->worker bypassing __vhost_vq_attach_worker(), so
their attachment_cnt stays 0. VHOST_FREE_WORKER then destroys such a
worker while a virtqueue still points at it, and the next
vhost_vq_work_queue() on that vq is a use-after-free. vhost-scsi
exposes this to the device owner.

The 0x1F ioctl number is freed: VHOST_SET_NWORKERS now fails with
-ENOTTY, so userspace that still issues it has to be updated
together with this kernel.

https://virtuozzo.atlassian.net/browse/VSTOR-107537
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/vhost/vhost.c      | 52 +-------------------------------------
 drivers/vhost/vhost.h      |  1 -
 include/uapi/linux/vhost.h |  8 ------
 3 files changed, 1 insertion(+), 60 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1057ba7e3fc7..56c0ecbb8352 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -605,8 +605,6 @@ void vhost_dev_init(struct vhost_dev *dev,
 	spin_lock_init(&dev->iotlb_lock);
 	xa_init_flags(&dev->worker_xa, XA_FLAGS_ALLOC);
 
-	dev->workers_set = false;
-
 	for (i = 0; i < dev->nvqs; ++i) {
 		vq = dev->vqs[i];
 		vq->log = NULL;
@@ -1089,42 +1087,6 @@ long vhost_worker_ioctl(struct vhost_dev *dev, unsigned int ioctl,
 }
 EXPORT_SYMBOL_GPL(vhost_worker_ioctl);
 
-static void vhost_propagate_workers(struct vhost_dev *dev, int nworkers)
-{
-	int i, j = 0;
-
-	for (i = nworkers; i < dev->nvqs; i++) {
-		dev->vqs[i]->worker = dev->vqs[j]->worker;
-		if (++j >= nworkers)
-			j = 0;
-	}
-}
-
-
-static int vhost_set_workers(struct vhost_dev *dev, int n)
-{
-	struct vhost_worker *worker;
-	int i, ret = 0;
-
-	if (n > dev->nvqs)
-		n = dev->nvqs;
-
-	dev->workers_set = true;
-
-	for (i = 1; i < n; i++) {
-		worker = vhost_worker_create(dev);
-		if (!worker) {
-			ret = -ENOMEM;
-			break;
-		}
-		dev->vqs[i]->worker = worker;
-	}
-
-	vhost_propagate_workers(dev, i);
-
-	return ret;
-}
-
 /* Caller should have device mutex */
 long vhost_dev_set_owner(struct vhost_dev *dev)
 {
@@ -1259,7 +1221,6 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
 	wake_up_interruptible_poll(&dev->wait, EPOLLIN | EPOLLRDNORM);
 	vhost_workers_free(dev);
 	vhost_detach_mm(dev);
-	dev->workers_set = false;
 }
 EXPORT_SYMBOL_GPL(vhost_dev_cleanup);
 
@@ -2345,7 +2306,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
 	struct eventfd_ctx *ctx;
 	u64 p;
 	long r;
-	int i, fd, n;
+	int i, fd;
 
 	/* If you are not the owner, you can become one */
 	if (ioctl == VHOST_SET_OWNER) {
@@ -2441,17 +2402,6 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
 		if (ctx)
 			eventfd_ctx_put(ctx);
 		break;
-	case VHOST_SET_NWORKERS:
-		r = get_user(n, (int __user *)argp);
-		if (r < 0)
-			break;
-		if (d->workers_set) {
-			r = -EINVAL;
-			break;
-		}
-
-		r = vhost_set_workers(d, n);
-		break;
 	default:
 		r = -ENOIOCTLCMD;
 		break;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 37ade2f1cb7d..077fa4d0cf36 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -198,7 +198,6 @@ struct vhost_dev {
 	 * The default value is set by fork_from_owner_default
 	 */
 	bool fork_owner;
-	bool workers_set;
 	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 			   struct vhost_iotlb_msg *msg);
 };
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 5553522fbf48..4371703948b9 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -102,14 +102,6 @@
 #define VHOST_GET_VRING_WORKER _IOWR(VHOST_VIRTIO, 0x16,		\
 				     struct vhost_vring_worker)
 
-/* Set number of vhost workers.
- * This can be done only once until reset.
- * All workers are freed upon reset.
- * If the value is too big it is silently truncated to the maximum number of
- * supported vhost workers
- */
-#define VHOST_SET_NWORKERS _IOW(VHOST_VIRTIO, 0x1F, int)
-
 /* The following ioctls use eventfd file descriptors to signal and poll
  * for events. */
 
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/2] drivers/vhost/blk: add support for worker ioctls

[Alexander Ivanov]

Let userspace create vhost workers and bind them to vhost-blk
virtqueues with the mainstream ioctls from c1ecd8e95007 ("vhost: allow
userspace to create workers"), the same way d74b55e65502 ("vhost_scsi:
add support for worker ioctls") does it for vhost-scsi.

VHOST_NEW_WORKER needs the device in vhost_task mode, so userspace has
to issue VHOST_SET_FORK_FROM_OWNER with VHOST_FORK_OWNER_TASK before
VHOST_SET_OWNER: our fork_from_owner_default is kthread.

https://virtuozzo.atlassian.net/browse/VSTOR-107537
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests

Suggested-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/vhost/blk.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index b170439a1806..ab3b8c4c2d5a 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -909,6 +909,14 @@ static long vhost_blk_ioctl(struct file *f, unsigned int ioctl,
 		return vhost_blk_set_features(blk, features);
 	case VHOST_RESET_OWNER:
 		return vhost_blk_reset_owner(blk);
+	case VHOST_NEW_WORKER:
+	case VHOST_FREE_WORKER:
+	case VHOST_ATTACH_VRING_WORKER:
+	case VHOST_GET_VRING_WORKER:
+		mutex_lock(&blk->dev.mutex);
+		ret = vhost_worker_ioctl(&blk->dev, ioctl, argp);
+		mutex_unlock(&blk->dev.mutex);
+		return ret;
 	default:
 		mutex_lock(&blk->dev.mutex);
 		ret = vhost_dev_ioctl(&blk->dev, ioctl, argp);
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/2] Revert "drivers/vhost: add ioctl to increase the number of workers"

[no-reply]

> This reverts commit 1491588e71dafc934c8d450ba36cc33bb0f990bd.
> 
> VHOST_SET_NWORKERS was a stopgap to run our userspace on the rh10
> kernel unchanged. Userspace should use the mainstream worker ioctls
> (VHOST_NEW_WORKER, VHOST_ATTACH_VRING_WORKER etc.) instead.
> 
> It is also unsafe next to those ioctls. vhost_set_workers() installs
> its workers into vq->worker bypassing __vhost_vq_attach_worker(), so
> their attachment_cnt stays 0. VHOST_FREE_WORKER then destroys such a
> worker while a virtqueue still points at it, and the next
> vhost_vq_work_queue() on that vq is a use-after-free. vhost-scsi
> exposes this to the device owner.
> 
> The 0x1F ioctl number is freed: VHOST_SET_NWORKERS now fails with
> -ENOTTY, so userspace that still issues it has to be updated
> together with this kernel.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027113830.2927925-1-alexander.ivanov@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/2] drivers/vhost/blk: add support for worker ioctls

[no-reply]

> Let userspace create vhost workers and bind them to vhost-blk
> virtqueues with the mainstream ioctls from c1ecd8e95007 ("vhost: allow
> userspace to create workers"), the same way d74b55e65502 ("vhost_scsi:
> add support for worker ioctls") does it for vhost-scsi.
> 
> VHOST_NEW_WORKER needs the device in vhost_task mode, so userspace has
> to issue VHOST_SET_FORK_FROM_OWNER with VHOST_FORK_OWNER_TASK before
> VHOST_SET_OWNER: our fork_from_owner_default is kthread.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
> 
> Suggested-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027113830.2927925-1-alexander.ivanov@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 0/2] vhost: replace VHOST_SET_NWORKERS with the mainstream worker ioctls

[Alexander Ivanov]

Please disregard this series. These patches I will resend as separate 
patches.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel