| Message ID | 20241210164456.925060-4-lulu@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | vhost: Add support of kthread API | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Dec 11, 2024 at 12:41:42AM +0800, Cindy Lu wrote: >Add back the previously removed cgroup function to support the kthread nit: missing . at the end >The biggest change for this part is in vhost_attach_cgroups() and >vhost_attach_task_to_cgroups(). It's not clear what the big change is, is there a piece missing? > >The old function was remove in s/remove/removed BTW which is the old function? >commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads") > >Signed-off-by: Cindy Lu <lulu@redhat.com> >--- > drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c >index 1feba29abf95..812dfd218bc2 100644 >--- a/drivers/vhost/vhost.c >+++ b/drivers/vhost/vhost.c >@@ -22,6 +22,7 @@ > #include <linux/slab.h> > #include <linux/vmalloc.h> > #include <linux/kthread.h> >+#include <linux/cgroup.h> > #include <linux/module.h> > #include <linux/sort.h> > #include <linux/sched/mm.h> >@@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev) > } > EXPORT_SYMBOL_GPL(vhost_dev_check_owner); > >+struct vhost_attach_cgroups_struct { >+ struct vhost_work work; >+ struct task_struct *owner; >+ int ret; >+}; >+ >+static void vhost_attach_cgroups_work(struct vhost_work *work) >+{ >+ struct vhost_attach_cgroups_struct *s; >+ >+ s = container_of(work, struct vhost_attach_cgroups_struct, work); >+ s->ret = cgroup_attach_task_all(s->owner, current); >+} >+ >+static int vhost_attach_task_to_cgroups(struct vhost_worker *worker) This function looks renamed, should we mention in the commit description? >+{ >+ struct vhost_flush_struct flush; >+ struct vhost_attach_cgroups_struct attach; >+ >+ attach.owner = current; >+ >+ vhost_work_init(&attach.work, vhost_attach_cgroups_work); >+ vhost_worker_queue(worker, &attach.work); >+ >+ init_completion(&flush.wait_event); >+ vhost_work_init(&flush.work, vhost_flush_work); >+ vhost_worker_queue(worker, &flush.work); >+ wait_for_completion(&flush.wait_event); IIUC this block is the old "vhost_dev_flush", right? Maybe we should mention also that in the commit description. >+ >+ return attach.ret; >+} >+ > /* Caller should have device mutex */ > bool vhost_dev_has_owner(struct vhost_dev *dev) > { >-- >2.45.0 >
On Wed, Dec 11, 2024 at 1:53 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Wed, Dec 11, 2024 at 12:41:42AM +0800, Cindy Lu wrote: > >Add back the previously removed cgroup function to support the kthread > > nit: missing . at the end > will fix this > >The biggest change for this part is in vhost_attach_cgroups() and > >vhost_attach_task_to_cgroups(). > > It's not clear what the big change is, is there a piece missing? > sure, I will rewrite this part to make it more clear Thanks cindy > > > >The old function was remove in > > s/remove/removed > > BTW which is the old function? > I will add this information Thanks cindy > >commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads") > > > >Signed-off-by: Cindy Lu <lulu@redhat.com> > >--- > > drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > >index 1feba29abf95..812dfd218bc2 100644 > >--- a/drivers/vhost/vhost.c > >+++ b/drivers/vhost/vhost.c > >@@ -22,6 +22,7 @@ > > #include <linux/slab.h> > > #include <linux/vmalloc.h> > > #include <linux/kthread.h> > >+#include <linux/cgroup.h> > > #include <linux/module.h> > > #include <linux/sort.h> > > #include <linux/sched/mm.h> > >@@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev) > > } > > EXPORT_SYMBOL_GPL(vhost_dev_check_owner); > > > >+struct vhost_attach_cgroups_struct { > >+ struct vhost_work work; > >+ struct task_struct *owner; > >+ int ret; > >+}; > >+ > >+static void vhost_attach_cgroups_work(struct vhost_work *work) > >+{ > >+ struct vhost_attach_cgroups_struct *s; > >+ > >+ s = container_of(work, struct vhost_attach_cgroups_struct, work); > >+ s->ret = cgroup_attach_task_all(s->owner, current); > >+} > >+ > >+static int vhost_attach_task_to_cgroups(struct vhost_worker *worker) > > This function looks renamed, should we mention in the commit > description? > > >+{ > >+ struct vhost_flush_struct flush; > >+ struct vhost_attach_cgroups_struct attach; > >+ > >+ attach.owner = current; > >+ > >+ vhost_work_init(&attach.work, vhost_attach_cgroups_work); > >+ vhost_worker_queue(worker, &attach.work); > >+ > >+ init_completion(&flush.wait_event); > >+ vhost_work_init(&flush.work, vhost_flush_work); > >+ vhost_worker_queue(worker, &flush.work); > >+ wait_for_completion(&flush.wait_event); > > IIUC this block is the old "vhost_dev_flush", right? > > Maybe we should mention also that in the commit description. > sure, will add these informations Thanks cindy > >+ > >+ return attach.ret; > >+} > >+ > > /* Caller should have device mutex */ > > bool vhost_dev_has_owner(struct vhost_dev *dev) > > { > >-- > >2.45.0 > > >
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 1feba29abf95..812dfd218bc2 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -22,6 +22,7 @@ #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/kthread.h> +#include <linux/cgroup.h> #include <linux/module.h> #include <linux/sort.h> #include <linux/sched/mm.h> @@ -620,6 +621,38 @@ long vhost_dev_check_owner(struct vhost_dev *dev) } EXPORT_SYMBOL_GPL(vhost_dev_check_owner); +struct vhost_attach_cgroups_struct { + struct vhost_work work; + struct task_struct *owner; + int ret; +}; + +static void vhost_attach_cgroups_work(struct vhost_work *work) +{ + struct vhost_attach_cgroups_struct *s; + + s = container_of(work, struct vhost_attach_cgroups_struct, work); + s->ret = cgroup_attach_task_all(s->owner, current); +} + +static int vhost_attach_task_to_cgroups(struct vhost_worker *worker) +{ + struct vhost_flush_struct flush; + struct vhost_attach_cgroups_struct attach; + + attach.owner = current; + + vhost_work_init(&attach.work, vhost_attach_cgroups_work); + vhost_worker_queue(worker, &attach.work); + + init_completion(&flush.wait_event); + vhost_work_init(&flush.work, vhost_flush_work); + vhost_worker_queue(worker, &flush.work); + wait_for_completion(&flush.wait_event); + + return attach.ret; +} + /* Caller should have device mutex */ bool vhost_dev_has_owner(struct vhost_dev *dev) {
Add back the previously removed cgroup function to support the kthread The biggest change for this part is in vhost_attach_cgroups() and vhost_attach_task_to_cgroups(). The old function was remove in commit 6e890c5d5021 ("vhost: use vhost_tasks for worker threads") Signed-off-by: Cindy Lu <lulu@redhat.com> --- drivers/vhost/vhost.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)