[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