linux-kernel - Re: [PATCH v5 4/6] vhost: Add worker related functions to support kthread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACLfguX9KrGPwhefy9LJZ9h5Qv1msFcO7_naBDi4DqR2V3wgKg@mail.gmail.com>
Date: Mon, 13 Jan 2025 10:31:08 +0800
From: Cindy Lu <lulu@...hat.com>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: jasowang@...hat.com, mst@...hat.com, michael.christie@...cle.com, 
	linux-kernel@...r.kernel.org, virtualization@...ts.linux-foundation.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v5 4/6] vhost: Add worker related functions to support kthread

On Wed, Jan 8, 2025 at 10:35 PM Stefano Garzarella <sgarzare@...hat.com> wrote:
>
> On Mon, Dec 30, 2024 at 08:43:51PM +0800, Cindy Lu wrote:
> >Restore the previously removed functions kthread_wakeup and
> >kthread_stop, and add two new function pointers to wake up and stop
> >the workers. The function vhost_worker_create will initialize these
> >pointers based on the value of inherit_owner.
> >
> >The functions vhost_worker_queue() and vhost_worker_destroy() will
> >use the function pointer in vhost_worker, which is initialized
> >according to the inherit_owner value.
> >
> >Signed-off-by: Cindy Lu <lulu@...hat.com>
> >---
> > drivers/vhost/vhost.c | 84 ++++++++++++++++++++++++++++++++++---------
> > drivers/vhost/vhost.h |  3 ++
> > 2 files changed, 71 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index 812dfd218bc2..ff17c42e2d1a 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
>
> [...]
>
> >@@ -736,27 +758,57 @@ static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
> >       worker->dev = dev;
> >       snprintf(name, sizeof(name), "vhost-%d", current->pid);
> >
> >-      vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
> >-                               worker, name);
> >-      if (!vtsk)
> >-              goto free_worker;
> >-
> >       mutex_init(&worker->mutex);
> >       init_llist_head(&worker->work_list);
> >       worker->kcov_handle = kcov_common_handle();
> >-      worker->vtsk = vtsk;
> >+    /*
> >+     * If inherit_owner is true we use vhost_tasks to create
> >+     * the worker so all settings/limits like cgroups, NPROC,
> >+     * scheduler, etc are inherited from the owner.
> >+     * If false,we use kthreads and only attach to the same
> >+     * cgroups as the owner for compat with older kernels.
> >+     */
>
> This comment block seems to have incorrect indentation, perhaps you need
> to replace the spaces with at least one tab.
>
sure will do
Thanks
cindy
> >+      if (dev->inherit_owner) {
> >+              vtsk = vhost_task_create(vhost_run_work_list,
> >+                                       vhost_worker_killed, worker, name);
> >+              if (!vtsk)
> >+                      goto free_worker;
> >+
> >+              worker->vtsk = vtsk;
> >+              worker->worker_wakeup = vhost_task_wakeup_fn;
> >+              worker->worker_stop = vhost_task_stop_fn;
> >+
> >+              vhost_task_start(vtsk);
> >+              ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b,
> >+                             GFP_KERNEL);
> >+              if (ret < 0)
> >+                      goto stop_worker;
> >+      } else {
> >+              task = kthread_create(vhost_run_work_kthread_list, worker,
> >+                                    "vhost-%d", current->pid);
> >+              if (IS_ERR(task)) {
> >+                      ret = PTR_ERR(task);
> >+                      goto free_worker;
> >+              }
> >+              worker->kthread_task = task;
> >+              worker->worker_wakeup = vhost_kthread_wakeup_fn;
> >+              worker->worker_stop = vhost_kthread_stop_fn;
> >
> >-      vhost_task_start(vtsk);
> >+              wake_up_process(task);
> >+              ret = xa_alloc(&dev->worker_xa, &id, worker,
> >xa_limit_32b,
> >+                             GFP_KERNEL);
> >+              if (ret < 0)
> >+                      goto stop_worker;
> >
> >-      ret = xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GFP_KERNEL);
> >-      if (ret < 0)
> >-              goto stop_worker;
> >-      worker->id = id;
> >+              ret = vhost_attach_task_to_cgroups(worker);
> >+              if (ret)
> >+                      goto stop_worker;
> >+      }
> >
> >+      worker->id = id;
> >       return worker;
> >-
> > stop_worker:
> >-      vhost_task_stop(vtsk);
> >+      worker->worker_stop(worker);
> > free_worker:
> >       kfree(worker);
> >       return NULL;
> >diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >index c650c4506c70..63b1da08a2b0 100644
> >--- a/drivers/vhost/vhost.h
> >+++ b/drivers/vhost/vhost.h
> >@@ -27,6 +27,7 @@ struct vhost_work {
> > };
> >
> > struct vhost_worker {
> >+      struct task_struct *kthread_task;
> >       struct vhost_task       *vtsk;
> >       struct vhost_dev        *dev;
> >       /* Used to serialize device wide flushing with worker swapping. */
> >@@ -36,6 +37,8 @@ struct vhost_worker {
> >       u32                     id;
> >       int                     attachment_cnt;
> >       bool                    killed;
> >+      void (*worker_wakeup)(struct vhost_worker *worker);
> >+      void (*worker_stop)(struct vhost_worker *worker);
> > };
> >
> > /* Poll a file (eventfd or socket) */
> >--
> >2.45.0
> >
>