Re: [PATCH v2 3/7] vhost: Add kthread support in function vhost_workers_free()

[Cindy Lu]

On Mon, 7 Oct 2024 at 21:33, Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>
> On Fri, Oct 04, 2024 at 09:58:17AM GMT, Cindy Lu wrote:
> >Added back the previously removed function vhost_workers_free() and
> >renamed it to vhost_workers_free_khtread(). The new vhost_workers_free()
> >will select the different mode based on the value of the parameter.
> >
> >The old function vhost_workers_free was change to support task in
> >commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads")
> >also changed in
> >commit a284f09effea ("vhost: Fix crash during early vhost_transport_send_pkt calls")
> >change to xarray in
> >commit 1cdaafa1b8b4 ("vhost: replace single worker pointer with xarray")
> >
> >Signed-off-by: Cindy Lu <lulu@xxxxxxxxxx>
> >---
> > drivers/vhost/vhost.c | 52 ++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 47 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index ad359d4b725f..fed6671c1ffb 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -648,8 +648,21 @@ static void vhost_detach_mm(struct vhost_dev *dev)
> >       dev->mm = NULL;
> > }
> >
> >-static void vhost_worker_destroy(struct vhost_dev *dev,
> >-                               struct vhost_worker *worker)
> >+static void vhost_worker_destroy_kthread(struct vhost_dev *dev,
> >+                                       struct vhost_worker *worker)
> >+{
> >+      if (!worker)
> >+              return;
> >+
> >+      WARN_ON(!llist_empty(&worker->work_list));
> >+
> >+      xa_erase(&dev->worker_xa, worker->id);
> >+      kthread_stop(worker->task);
> >+      kfree(worker);
> >+}
> >+
> >+static void vhost_worker_destroy_task(struct vhost_dev *dev,
> >+                                    struct vhost_worker *worker)
> > {
> >       if (!worker)
> >               return;
> >@@ -660,7 +673,7 @@ static void vhost_worker_destroy(struct vhost_dev *dev,
> >       kfree(worker);
> > }
> >
> >-static void vhost_workers_free(struct vhost_dev *dev)
> >+static void vhost_workers_free_task(struct vhost_dev *dev)
> > {
> >       struct vhost_worker *worker;
> >       unsigned long i;
> >@@ -675,10 +688,36 @@ static void vhost_workers_free(struct vhost_dev *dev)
> >        * created but couldn't clean up (it forgot or crashed).
> >        */
> >       xa_for_each(&dev->worker_xa, i, worker)
> >-              vhost_worker_destroy(dev, worker);
> >+              vhost_worker_destroy_task(dev, worker);
> >       xa_destroy(&dev->worker_xa);
> > }
> >
> >+static void vhost_workers_free_kthread(struct vhost_dev *dev)
> >+{
> >+      struct vhost_worker *worker;
> >+      unsigned long i;
> >+
> >+      if (!dev->use_worker)
> >+              return;
> >+
> >+      for (i = 0; i < dev->nvqs; i++)
> >+              rcu_assign_pointer(dev->vqs[i]->worker, NULL);
> >+      /*
> >+       * Free the default worker we created and cleanup workers userspace
> >+       * created but couldn't clean up (it forgot or crashed).
> >+       */
> >+      xa_for_each(&dev->worker_xa, i, worker)
> >+              vhost_worker_destroy_kthread(dev, worker);
> >+      xa_destroy(&dev->worker_xa);
> >+}
> >+
> >+static void vhost_workers_free(struct vhost_dev *dev)
> >+{
> >+      if (enforce_inherit_owner)
> >+              vhost_workers_free_task(dev);
> >+      else
> >+              vhost_workers_free_kthread(dev);
> >+}
>
> nit: new line here
>
sure, will fix this
> > static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
> > {
> >       struct vhost_worker *worker;
> >@@ -846,7 +885,10 @@ static int vhost_free_worker(struct vhost_dev *dev,
> >       __vhost_worker_flush(worker);
> >       mutex_unlock(&worker->mutex);
> >
> >-      vhost_worker_destroy(dev, worker);
> >+      if (enforce_inherit_owner)
> >+              vhost_worker_destroy_task(dev, worker);
> >+      else
> >+              vhost_worker_destroy_kthread(dev, worker);
> >       return 0;
> > }
> >
> >--
> >2.45.0
> >
> >
>