[lxc-devel] [PATCH RFC] cgmanager: don't stay connected
Serge Hallyn
serge.hallyn at ubuntu.com
Sat Mar 1 18:56:27 UTC 2014
Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> There are only a few times when we need to be connected to the
> cgroup manager:
>
> * when starting a container, from cgm_init until we've set cgroup limits
> * when changing a cgroup setting (while running)
> * when cleaning up (when shutting down)
> * around the cgroup entering at attach
>
> So only connect/disconnect the cgmanager socket on-demand as
> needed. This should have a few benefits.
>
> 1. Reduce the # open fds when many containers are running
> 2. if cgmanager is stopped and restarted, the container
> doesn't have to deal with the disconnection.
>
> This is currently RFC. There are a few issues outstanding:
>
> 1. the cgm_set and cgm_get may need to be made thread-safe.
> 2. a non-daemonized start which fails while cgm is connected,
> will not disconnected.
Next week I will add a cgmanager socket refcount to allow parallel
threads using cgm_get/set, and put the cgmanager sock reference
at end of failed start and attach.
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/cgmanager.c | 19 +++++++++++++++++--
> src/lxc/cgroup.c | 6 ++++++
> src/lxc/cgroup.h | 1 +
> src/lxc/start.c | 2 ++
> 4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 7391675..7be94a7 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -92,13 +92,13 @@ static NihDBusProxy *cgroup_manager = NULL;
> static struct cgroup_ops cgmanager_ops;
> static int nr_subsystems;
> static char **subsystems;
> +static DBusConnection *connection;
>
> #define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
> static void cgm_dbus_disconnected(DBusConnection *connection);
> static bool cgm_dbus_connect(void)
> {
> DBusError dbus_error;
> - DBusConnection *connection;
> dbus_error_init(&dbus_error);
>
> lock_mutex(&thread_mutex);
> @@ -118,7 +118,6 @@ static bool cgm_dbus_connect(void)
> cgroup_manager = nih_dbus_proxy_new(NULL, connection,
> NULL /* p2p */,
> "/org/linuxcontainers/cgmanager", NULL, NULL);
> - dbus_connection_unref(connection);
> if (!cgroup_manager) {
> NihError *nerr;
> nerr = nih_error_get();
> @@ -145,13 +144,19 @@ static void cgm_dbus_disconnect(void)
> if (cgroup_manager)
> nih_free(cgroup_manager);
> cgroup_manager = NULL;
> + dbus_connection_unref(connection);
> + connection = NULL;
> unlock_mutex(&thread_mutex);
> }
>
> static void cgm_dbus_disconnected(DBusConnection *connection)
> {
> + lock_mutex(&thread_mutex);
> WARN("Cgroup manager connection was terminated");
> cgroup_manager = NULL;
> + dbus_connection_unref(connection);
> + connection = NULL;
> + unlock_mutex(&thread_mutex);
> if (cgm_dbus_connect()) {
> INFO("New cgroup manager connection was opened");
> } else {
> @@ -398,6 +403,7 @@ static void *cgm_init(const char *name)
> {
> struct cgm_data *d;
>
> + cgm_dbus_connect();
> d = malloc(sizeof(*d));
> if (!d)
> return NULL;
> @@ -430,6 +436,7 @@ static void cgm_destroy(void *hdata)
>
> if (!d)
> return;
> + cgm_dbus_connect();
> for (i = 0; i < nr_subsystems; i++)
> cgm_remove_cgroup(subsystems[i], d->cgroup_path);
>
> @@ -437,6 +444,7 @@ static void cgm_destroy(void *hdata)
> if (d->cgroup_path)
> free(d->cgroup_path);
> free(d);
> + cgm_dbus_disconnect();
> }
>
> /*
> @@ -602,6 +610,7 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
> cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> if (!cgroup)
> return -1;
> + cgm_dbus_connect();
> lock_mutex(&thread_mutex);
> if (cgmanager_get_value_sync(NULL, cgroup_manager, controller, cgroup, filename, &result) != 0) {
> /*
> @@ -614,9 +623,11 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
> nih_free(nerr);
> free(cgroup);
> unlock_mutex(&thread_mutex);
> + cgm_dbus_disconnect();
> return -1;
> }
> unlock_mutex(&thread_mutex);
> + cgm_dbus_disconnect();
> free(cgroup);
> newlen = strlen(result);
> if (!value) {
> @@ -675,7 +686,9 @@ static int cgm_set(const char *filename, const char *value, const char *name, co
> controller, lxcpath, name);
> return -1;
> }
> + cgm_dbus_connect();
> ret = cgm_do_set(controller, filename, cgroup, value);
> + cgm_dbus_disconnect();
> free(cgroup);
> return ret;
> }
> @@ -875,8 +888,10 @@ static bool cgm_attach(const char *name, const char *lxcpath, pid_t pid)
> goto out;
> }
>
> + cgm_dbus_connect();
> if (!(pass = do_cgm_enter(pid, cgroup)))
> ERROR("Failed to enter group %s", cgroup);
> + cgm_dbus_disconnect();
>
> out:
> free(cgroup);
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index d931520..23c8c96 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -175,3 +175,9 @@ void restart_cgroups(void)
> ops = NULL;
> cgroup_ops_init();
> }
> +
> +void cgroup_disconnect(void)
> +{
> + if (ops && ops->disconnect)
> + ops->disconnect();
> +}
> diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> index e3cf6e0..8cba030 100644
> --- a/src/lxc/cgroup.h
> +++ b/src/lxc/cgroup.h
> @@ -66,5 +66,6 @@ extern int cgroup_nrtasks(struct lxc_handler *handler);
> extern const char *cgroup_get_cgroup(struct lxc_handler *handler, const char *subsystem);
> extern bool cgroup_unfreeze(struct lxc_handler *handler);
> extern void restart_cgroups(void);
> +extern void cgroup_disconnect(void);
>
> #endif
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index fe3d093..97c8207 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -952,6 +952,8 @@ static int lxc_spawn(struct lxc_handler *handler)
> goto out_delete_net;
> }
>
> + cgroup_disconnect();
> +
> /* Tell the child to complete its initialization and wait for
> * it to exec or return an error. (the child will never
> * return LXC_SYNC_POST_CGROUP+1. It will either close the
> --
> 1.9.0
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list