cgroups: improve container cgroup attaching

The current attach.c codepath which handles moving the attaching process into
the container's cgroups allocates a whole new struct cgroup_ops and goes
through the trouble of reparsing the whole cgroup layout.
That's costly and wasteful. My plan has always been to move this into the
command api by getting fds for attaching back but but it's not worth going
through that hazzle for non-unified hosts. On pure unified hosts however -
being the future - we can just attach through a single fd so there's no need to
allocate and setup struct cgroup_ops.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 80c41fe263a164a9ed628fe87163c5b833f99e75..8f5f8424dcde4287f23fd0b30f28cbf6d57babf9 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -1230,16 +1230,21 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 
                /* Attach to cgroup, if requested. */
                if (options->attach_flags & LXC_ATTACH_MOVE_TO_CGROUP) {
-                       struct cgroup_ops *cgroup_ops;
+                       /*
+                        * If this is the unified hierarchy cgroup_attach() is
+                        * enough.
+                        */
+                       ret = cgroup_attach(name, lxcpath, pid);
+                       if (ret) {
+                               __do_cgroup_exit struct cgroup_ops *cgroup_ops = NULL;
 
-                       cgroup_ops = cgroup_init(conf);
-                       if (!cgroup_ops)
-                               goto on_error;
+                               cgroup_ops = cgroup_init(conf);
+                               if (!cgroup_ops)
+                                       goto on_error;
 
-                       if (!cgroup_ops->attach(cgroup_ops, name, lxcpath, pid))
-                               goto on_error;
-
-                       cgroup_exit(cgroup_ops);
+                               if (!cgroup_ops->attach(cgroup_ops, name, lxcpath, pid))
+                                       goto on_error;
+                       }
                        TRACE("Moved intermediate process %d into container's cgroups", pid);
                }
 

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index d2e1ecf6f537ecf1a5fc70880d952a2d9bd39c82..35763c8cefba878a604e0d75ea5aadfaf8fcc837 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -2202,38 +2202,14 @@ static inline char *build_full_cgpath_from_monitorpath(struct hierarchy *h,
        return must_make_path(h->mountpoint, inpath, filename, NULL);
 }
 
-/* Technically, we're always at a delegation boundary here (This is especially
- * true when cgroup namespaces are available.). The reasoning is that in order
- * for us to have been able to start a container in the first place the root
- * cgroup must have been a leaf node. Now, either the container's init system
- * has populated the cgroup and kept it as a leaf node or it has created
- * subtrees. In the former case we will simply attach to the leaf node we
- * created when we started the container in the latter case we create our own
- * cgroup for the attaching process.
- */
-static int __cg_unified_attach(const struct hierarchy *h, const char *name,
-                              const char *lxcpath, const char *pidstr,
-                              size_t pidstr_len, const char *controller)
+static int cgroup_attach_leaf(int unified_fd, int64_t pid)
 {
-       __do_close_prot_errno int unified_fd = -EBADF;
        int idx = 0;
        int ret;
+       char pidstr[INTTYPE_TO_STRLEN(int64_t) + 1];
+       size_t pidstr_len;
 
-       unified_fd = lxc_cmd_get_cgroup2_fd(name, lxcpath);
-       if (unified_fd < 0) {
-               __do_free char *base_path = NULL, *container_cgroup = NULL;
-
-               container_cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
-               /* not running */
-               if (!container_cgroup)
-                       return 0;
-
-               base_path = must_make_path(h->mountpoint, container_cgroup, NULL);
-               unified_fd = open(base_path, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
-       }
-       if (unified_fd < 0)
-               return -1;
-
+       pidstr_len = sprintf(pidstr, INT64_FMT, pid);
        ret = lxc_writeat(unified_fd, "cgroup.procs", pidstr, pidstr_len);
        if (ret == 0)
                return 0;
@@ -2275,6 +2251,51 @@ static int __cg_unified_attach(const struct hierarchy *h, const char *name,
        return -1;
 }
 
+int cgroup_attach(const char *name, const char *lxcpath, int64_t pid)
+{
+       __do_close_prot_errno int unified_fd = -EBADF;
+
+       unified_fd = lxc_cmd_get_cgroup2_fd(name, lxcpath);
+       if (unified_fd < 0)
+               return -1;
+
+       return cgroup_attach_leaf(unified_fd, pid);
+}
+
+/* Technically, we're always at a delegation boundary here (This is especially
+ * true when cgroup namespaces are available.). The reasoning is that in order
+ * for us to have been able to start a container in the first place the root
+ * cgroup must have been a leaf node. Now, either the container's init system
+ * has populated the cgroup and kept it as a leaf node or it has created
+ * subtrees. In the former case we will simply attach to the leaf node we
+ * created when we started the container in the latter case we create our own
+ * cgroup for the attaching process.
+ */
+static int __cg_unified_attach(const struct hierarchy *h, const char *name,
+                              const char *lxcpath, pid_t pid,
+                              const char *controller)
+{
+       __do_close_prot_errno int unified_fd = -EBADF;
+       int ret;
+
+       ret = cgroup_attach(name, lxcpath, pid);
+       if (ret < 0) {
+               __do_free char *path = NULL, *cgroup = NULL;
+
+               cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
+               /* not running */
+               if (!cgroup)
+                       return 0;
+
+               path = must_make_path(h->mountpoint, cgroup, NULL);
+               unified_fd = open(path, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
+       }
+       if (unified_fd < 0)
+               return -1;
+
+       return cgroup_attach_leaf(unified_fd, pid);
+}
+
 __cgfsng_ops static bool cgfsng_attach(struct cgroup_ops *ops, const char *name,
                                         const char *lxcpath, pid_t pid)
 {
@@ -2293,7 +2314,7 @@ __cgfsng_ops static bool cgfsng_attach(struct cgroup_ops *ops, const char *name,
                struct hierarchy *h = ops->hierarchies[i];
 
                if (h->version == CGROUP2_SUPER_MAGIC) {
-                       ret = __cg_unified_attach(h, name, lxcpath, pidstr, len,
+                       ret = __cg_unified_attach(h, name, lxcpath, pid,
                                                  h->controllers[0]);
                        if (ret < 0)
                                return false;

diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index 91519fbe9c59752bd81b7b62f5281f32edc3c23d..f88c9ca41592dd620484bf657c7a31e4dc906bb4 100644 (file)
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -192,6 +192,8 @@ static inline void __auto_cgroup_exit__(struct cgroup_ops **ops)
                cgroup_exit(*ops);
 }
 
+extern int cgroup_attach(const char *name, const char *lxcpath, int64_t pid);
+
 #define __do_cgroup_exit __attribute__((__cleanup__(__auto_cgroup_exit__)))
 
 #endif

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index ccf8cf8905174228b30e0c91b5dbba0fb8d2bc88..6260d5dbd6c3b1f032072d4c7714ed056dccd3c7 100644 (file)
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1300,7 +1300,7 @@ int lxc_cmd_get_cgroup2_fd(const char *name, const char *lxcpath)
                return -1;
 
        if (cmd.rsp.ret < 0)
-               return error_log_errno(errno, "Failed to receive cgroup2 fd");
+               return log_debug_errno(-1, errno, "Failed to receive cgroup2 fd");
 
        return PTR_TO_INT(cmd.rsp.data);
 }

diff --git a/src/lxc/log.h b/src/lxc/log.h
index 58c98d1e021eeb8164ffe58f4428bbd992dc8d83..d8d5fdd23e75b3ab04fd1d6a2708babc533f06a9 100644 (file)
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -544,6 +544,12 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,        \
                __ret__;                      \
        })
 
+#define log_debug_errno(__ret__, __errno__, format, ...) \
+       ({                                               \
+               SYSDEBUG(format, ##__VA_ARGS__);         \
+               __ret__;                                 \
+       })
+
 extern int lxc_log_fd;
 
 extern int lxc_log_syslog(int facility);

diff --git a/src/lxc/macro.h b/src/lxc/macro.h
index d9d91cd527eab98a2aca62391db90a45c048010b..ed58bb1c6879d25b100a032e29005f8b7f4d0c9a 100644 (file)
--- a/src/lxc/macro.h
+++ b/src/lxc/macro.h
@@ -21,6 +21,10 @@
 #ifndef __LXC_MACRO_H
 #define __LXC_MACRO_H
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE 1
+#endif
+#define __STDC_FORMAT_MACROS
 #include <asm/types.h>
 #include <limits.h>
 #include <linux/if_link.h>
@@ -39,6 +43,8 @@
 #define PATH_MAX 4096
 #endif
 
+#define INT64_FMT "%" PRId64
+
 /* Define __S_ISTYPE if missing from the C library. */
 #ifndef __S_ISTYPE
 #define __S_ISTYPE(mode, mask) (((mode)&S_IFMT) == (mask))