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 93bd1c87cc7da114f735bd29c67d949007e6313b..9679ddaa223b4135e22f9c0b5451b440b35a030a 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -1205,16 +1205,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 f4f5a829f0a8ea39d7a636b3a295e2dd63a3ba09..f2223afc6ee123d52b28b06cd32272791fdc8444 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -2180,38 +2180,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;
@@ -2253,6 +2229,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)
 {
@@ -2271,7 +2292,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 6d6091fe1470116de1f337d87a02d4fe757c920c..81320e4876183acb033875d1a42e2603a844c5e1 100644 (file)
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -171,6 +171,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 104272263337eadbaccf9da60a63081c7d37de60..c82bb1ed73e0f83eb1fbf5f920bb1b26d77f2212 100644 (file)
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1244,7 +1244,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 7432c839186eaff44d0359b342816b8c0fa47e79..951eaba318b176e41e4ec4f281d1df817e4ed76d 100644 (file)
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -523,6 +523,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 b0c1d7c5ec191a1d40d28a0bab8f53d6d3070301..90184c0e46e88117175f64223fd91d9b03067a10 100644 (file)
--- a/src/lxc/macro.h
+++ b/src/lxc/macro.h
@@ -3,6 +3,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>
@@ -21,6 +25,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))