The commit is pushed to "branch-rh9-5.14.vz9.1.x-ovz" and will appear at 
https://src.openvz.org/scm/ovz/vzkernel.git
after rh9-5.14.0-4.vz9.10.12
------>
commit 94b2fa5aa580f377e3ccb9014d04988940bc6a58
Author: Valeriy Vdovin <valeriy.vdo...@virtuozzo.com>
Date:   Wed Oct 20 11:43:55 2021 +0300

    ve/cgroup: Move release_agent from system_wq to per-ve workqueues
    
    Each VE should execute release agent notifications within it's own
    workqueue. This way we achieve a more fine-grained control over
    release_agent work flushing at VE destruction.
    
    Short history of the patch:
    
    https://jira.sw.ru/browse/PSBM-83887
    Signed-off-by: Valeriy Vdovin <valeriy.vdo...@virtuozzo.com>
    
    Reviewed-by: Kirill Tkhai <ktk...@virtuozzo.com>
    (cherry picked from vz7 commit 9fbfb5b4cfb87ba7c9dd63eec5e5e27946a38d3c)
    https://jira.sw.ru/browse/PSBM-108270
    (cherry picked from vz8 commit 0eda8e4d0a9f6660a72f7584e8504a363c25a392)
    +++
    cgroup/ve: Do not run release_agent on non-running ve
    +++
    cgroup/ve: Move ve workqueue stop to ve_stop_ns()
    (cherry picked from vz8 commit 1481d130afa32eecffe28b34fd00b57e11d2666f)
    
    vz9 changes:
    - merge fixup hunks from 1481d130afa3 related to workqueue locking
    - do less code movements around cgroup->release_list and
      ve->release_list init
    - add comment to ve_rm_from_release_list about why we always have the
      right ve->release_list_lock there
    - rework cgroup1_release_agent, on vz8 release-agent patches are badly
      broken with conterintuitive reordering, e.g. cgroup_path_ve_relative
      used in this patch was defined in later patches...
    - fix missing cgroup->release_list initialization
    
    https://jira.sw.ru/browse/PSBM-134002
    Signed-off-by: Pavel Tikhomirov <ptikhomi...@virtuozzo.com>
    
    Reviewed-by: Alexander Mikhalitsyn <alexander.mikhalit...@virtuozzo.com>
    Reviewed-by: Kirill Tkhai <ktk...@virtuozzo.com>
---
 include/linux/cgroup-defs.h     |  8 +++--
 include/linux/cgroup.h          |  2 ++
 include/linux/ve.h              | 10 ++++++
 kernel/cgroup/cgroup-internal.h |  1 +
 kernel/cgroup/cgroup-v1.c       | 78 ++++++++++++++++++++++++++++-------------
 kernel/cgroup/cgroup.c          |  4 +--
 kernel/ve/ve.c                  | 73 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 148 insertions(+), 28 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 315c399c5d3b..2035909f7a0a 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -482,8 +482,12 @@ struct cgroup {
        /* used to wait for offlining of csses */
        wait_queue_head_t offline_waitq;
 
-       /* used to schedule release agent */
-       struct work_struct release_agent_work;
+       /*
+        * Linked list running through all cgroups that can
+        * potentially be reaped by the release agent. Protected by
+        * release_list_lock
+        */
+       struct list_head release_list;
 
        /* used to track pressure stalls */
        struct psi_group psi;
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index f4d9b0f44a43..d2586a8397f3 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -886,6 +886,8 @@ struct cgroup_namespace *copy_cgroup_ns(unsigned long flags,
 int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
                   struct cgroup_namespace *ns);
 
+void cgroup1_release_agent(struct work_struct *work);
+
 #ifdef CONFIG_VE
 extern int cgroup_mark_ve_roots(struct ve_struct *ve);
 void cgroup_unmark_ve_roots(struct ve_struct *ve);
diff --git a/include/linux/ve.h b/include/linux/ve.h
index eb223c9db6e5..25dee1b4d4e9 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -106,8 +106,16 @@ struct ve_struct {
        unsigned long           aio_nr;
        unsigned long           aio_max_nr;
 #endif
+       /*
+        * cgroups, that want to notify about becoming
+        * empty, are linked to this release_list.
+        */
+       struct list_head        release_list;
+       spinlock_t              release_list_lock;
+
        /* Should take rcu_read_lock and check ve->is_running before queue */
        struct workqueue_struct *wq;
+       struct work_struct      release_agent_work;
 
        struct vfsmount         *devtmpfs_mnt;
 };
@@ -133,6 +141,8 @@ extern int nr_ve;
 extern unsigned int sysctl_ve_mount_nr;
 
 #ifdef CONFIG_VE
+void ve_add_to_release_list(struct cgroup *cgrp);
+void ve_rm_from_release_list(struct cgroup *cgrp);
 extern struct ve_struct *get_ve(struct ve_struct *ve);
 extern void put_ve(struct ve_struct *ve);
 
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index bfbeabc17a9d..81bf97510198 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -151,6 +151,7 @@ extern spinlock_t css_set_lock;
 extern struct cgroup_subsys *cgroup_subsys[];
 extern struct list_head cgroup_roots;
 extern struct file_system_type cgroup_fs_type;
+struct cgroup *cgroup_get_local_root(struct cgroup *cgrp);
 
 /* iterate across the hierarchies */
 #define for_each_root(root)                                            \
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 494b4bdd3265..5b3e8e79cc32 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -774,7 +774,7 @@ void cgroup1_check_for_release(struct cgroup *cgrp)
 {
        if (notify_on_release(cgrp) && !cgroup_is_populated(cgrp) &&
            !css_has_online_children(&cgrp->self) && !cgroup_is_dead(cgrp))
-               schedule_work(&cgrp->release_agent_work);
+               ve_add_to_release_list(cgrp);
 }
 
 /*
@@ -802,42 +802,72 @@ void cgroup1_check_for_release(struct cgroup *cgrp)
  */
 void cgroup1_release_agent(struct work_struct *work)
 {
-       struct cgroup *cgrp =
-               container_of(work, struct cgroup, release_agent_work);
+       struct ve_struct *ve =
+               container_of(work, struct ve_struct, release_agent_work);
+       /*
+        * Release agent work can be queued only to running ve (see
+        * ve_add_to_release_list) and ve waits for all queued works to finish
+        * (see ve_workqueue_stop) before stopping, so we can safely access
+        * ve_ns->cgroup_ns here without rcu_read_lock - it won't disappear
+        * under us.
+        */
+       struct cgroup_namespace *ve_cgroup_ns =
+               rcu_dereference_protected(ve->ve_ns, 1)->cgroup_ns;
        char *pathbuf, *agentbuf;
        char *argv[3], *envp[3];
+       unsigned long flags;
        int ret;
 
-       /* snoop agent path and exit early if empty */
-       if (!cgrp->root->release_agent_path[0])
-               return;
-
        /* prepare argument buffers */
        pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
        agentbuf = kmalloc(PATH_MAX, GFP_KERNEL);
        if (!pathbuf || !agentbuf)
                goto out_free;
 
-       spin_lock(&release_agent_path_lock);
-       strlcpy(agentbuf, cgrp->root->release_agent_path, PATH_MAX);
-       spin_unlock(&release_agent_path_lock);
-       if (!agentbuf[0])
-               goto out_free;
+       spin_lock_irqsave(&ve->release_list_lock, flags);
+       while (!list_empty(&ve->release_list)) {
+               struct cgroup *cgrp;
 
-       ret = cgroup_path_ns(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
-       if (ret < 0 || ret >= PATH_MAX)
-               goto out_free;
-
-       argv[0] = agentbuf;
-       argv[1] = pathbuf;
-       argv[2] = NULL;
+               cgrp = list_entry(ve->release_list.next,
+                                 struct cgroup,
+                                 release_list);
+               list_del_init(&cgrp->release_list);
+               spin_unlock_irqrestore(&ve->release_list_lock, flags);
 
-       /* minimal command environment */
-       envp[0] = "HOME=/";
-       envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
-       envp[2] = NULL;
+               /* snoop agent path and exit early if empty */
+               if (!cgrp->root->release_agent_path[0])
+                       goto continue_locked;
 
-       call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
+               spin_lock(&release_agent_path_lock);
+               strlcpy(agentbuf, cgrp->root->release_agent_path, PATH_MAX);
+               spin_unlock(&release_agent_path_lock);
+               if (!agentbuf[0])
+                       goto continue_locked;
+
+               ret = cgroup_path_ns(cgrp, pathbuf, PATH_MAX, ve_cgroup_ns);
+               if (ret < 0 || ret >= PATH_MAX)
+                       goto continue_locked;
+
+               argv[0] = agentbuf;
+               argv[1] = pathbuf;
+               argv[2] = NULL;
+
+               /* minimal command environment */
+               envp[0] = "HOME=/";
+               envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
+               envp[2] = NULL;
+
+               ret = call_usermodehelper_ve(ve, argv[0], argv, envp,
+                                            UMH_WAIT_EXEC);
+
+               if (ret < 0 && ve == &ve0)
+                       pr_warn_ratelimited("cgroup1_release_agent "
+                                           "%s %s failed: %d\n",
+                                           agentbuf, pathbuf, ret);
+continue_locked:
+               spin_lock_irqsave(&ve->release_list_lock, flags);
+       }
+       spin_unlock_irqrestore(&ve->release_list_lock, flags);
 out_free:
        kfree(agentbuf);
        kfree(pathbuf);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ff9f5b9dc219..c0c92a4cf791 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2167,6 +2167,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
        INIT_LIST_HEAD(&cgrp->self.children);
        INIT_LIST_HEAD(&cgrp->cset_links);
        INIT_LIST_HEAD(&cgrp->pidlists);
+       INIT_LIST_HEAD(&cgrp->release_list);
        mutex_init(&cgrp->pidlist_mutex);
        cgrp->self.cgroup = cgrp;
        cgrp->self.flags |= CSS_ONLINE;
@@ -2180,7 +2181,6 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
                INIT_LIST_HEAD(&cgrp->e_csets[ssid]);
 
        init_waitqueue_head(&cgrp->offline_waitq);
-       INIT_WORK(&cgrp->release_agent_work, cgroup1_release_agent);
 }
 
 void init_cgroup_root(struct cgroup_fs_context *ctx)
@@ -5327,7 +5327,6 @@ static void css_free_rwork_fn(struct work_struct *work)
                /* cgroup free path */
                atomic_dec(&cgrp->root->nr_cgrps);
                cgroup1_pidlist_destroy_all(cgrp);
-               cancel_work_sync(&cgrp->release_agent_work);
 
                if (cgroup_parent(cgrp)) {
                        /*
@@ -5879,6 +5878,7 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
         * the migration path.
         */
        cgrp->self.flags &= ~CSS_ONLINE;
+       ve_rm_from_release_list(cgrp);
 
        spin_lock_irq(&css_set_lock);
        list_for_each_entry(link, &cgrp->cset_links, cset_link)
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index d8fd86568533..9c593e93a600 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -74,6 +74,11 @@ struct ve_struct ve0 = {
        .meminfo_val            = VE_MEMINFO_SYSTEM,
        .vdso_64                = (struct vdso_image*)&vdso_image_64,
        .vdso_32                = (struct vdso_image*)&vdso_image_32,
+       .release_list_lock      = __SPIN_LOCK_UNLOCKED(
+                                       ve0.release_list_lock),
+       .release_list           = LIST_HEAD_INIT(ve0.release_list),
+       .release_agent_work     = __WORK_INITIALIZER(ve0.release_agent_work,
+                                       cgroup1_release_agent),
 };
 EXPORT_SYMBOL(ve0);
 
@@ -478,6 +483,69 @@ static void ve_workqueue_stop(struct ve_struct *ve)
        ve->wq = NULL;
 }
 
+/*
+ * ve_add_to_release_list - called from cgroup1_check_for_release to put a
+ * cgroup into a release workqueue. There are two codepaths that lead to this
+ * function. One starts from cgroup_exit() which holds css_set_lock, another
+ * one from cgroup_destroy_locked which does not hold css_set_lock. So we
+ * should not use any reschedulable
+ *
+ */
+void ve_add_to_release_list(struct cgroup *cgrp)
+{
+       struct ve_struct *ve;
+       unsigned long flags;
+       int need_schedule_work = 0;
+
+       rcu_read_lock();
+       ve = cgroup_ve_owner(cgrp);
+       if (!ve)
+               ve = &ve0;
+
+       if (!ve->is_running) {
+               rcu_read_unlock();
+               return;
+       }
+
+       spin_lock_irqsave(&ve->release_list_lock, flags);
+       if (!cgroup_is_dead(cgrp) &&
+           list_empty(&cgrp->release_list)) {
+               list_add(&cgrp->release_list, &ve->release_list);
+               need_schedule_work = 1;
+       }
+       spin_unlock_irqrestore(&ve->release_list_lock, flags);
+
+       if (need_schedule_work)
+               queue_work(ve->wq, &ve->release_agent_work);
+
+       rcu_read_unlock();
+}
+
+/*
+ * As workqueue destroy happens before we unset CGRP_VE_ROOT and
+ * cgroup->ve_owner and also as destroy waits for all current works
+ * to finish, we can rely that if cgroup is in release_list of some ve
+ * then cgroup_ve_owner would return exactly the same ve, and we
+ * would take the right ve->release_list_lock for operating on
+ * our cgroup->release_list.
+ */
+void ve_rm_from_release_list(struct cgroup *cgrp)
+{
+       struct ve_struct *ve;
+       unsigned long flags;
+
+       rcu_read_lock();
+       ve = cgroup_ve_owner(cgrp);
+       if (!ve)
+               ve = &ve0;
+
+       spin_lock_irqsave(&ve->release_list_lock, flags);
+       if (!list_empty(&cgrp->release_list))
+               list_del_init(&cgrp->release_list);
+       spin_unlock_irqrestore(&ve->release_list_lock, flags);
+       rcu_read_unlock();
+}
+
 /* under ve->op_sem write-lock */
 static int ve_start_container(struct ve_struct *ve)
 {
@@ -720,6 +788,11 @@ static struct cgroup_subsys_state *ve_create(struct 
cgroup_subsys_state *parent_
                goto err_lat;
 
        ve->features = VE_FEATURES_DEF;
+
+       INIT_WORK(&ve->release_agent_work, cgroup1_release_agent);
+       spin_lock_init(&ve->release_list_lock);
+       INIT_LIST_HEAD(&ve->release_list);
+
        ve->_randomize_va_space = ve0._randomize_va_space;
 
        ve->meminfo_val = VE_MEMINFO_DEFAULT;
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to