[Lxc-users] total RAM limit

[Serge Hallyn]

Quoting Frederic Crozat (fcrozat at suse.com):
> 
> Le lundi 04 février 2013 à 08:38 -0600, Serge Hallyn a écrit :
> > Quoting Gary Ballantyne (gary.ballantyne at haulashore.com):
> > > On Fri, 1 Feb 2013 10:24:13 -0600
> > > Serge Hallyn <serge.hallyn at canonical.com> wrote:
> > > 
> > > > 
> > > > Did you actually test with a memory hog program? I just noticed there
> > > > appears to be a bug in that if I
> > > > 
> > > > 	d=/sys/fs/cgroup/memory/a
> > > > 	mkdir $d
> > > > 	echo 1 > $d/memory.use_hierarchy
> > > > 	echo 5000 > $d/memory.limit_in_bytes
> > > > 	mkdir $d/b
> > > > 
> > > > then $d/b/memory.limit_in_bytes does not report the reduced limit.  However
> > > > whenI run a program which does char *c = malloc(10000) after doing
> > > > 	echo $$ > $d/b/tasks
> > > > 
> > > > then I get killed by OOM.
> > > > 
> > > > -serge
> > > 
> > > I tested with a large array in ipython. Though, from your example, it seems I'm missing memory.use_hierarchy.
> > > 
> > > In principle, it seems like I need something like:
> > > 
> > > echo 1 > /sys/fs/cgroup/memory/lxc/memory.use_hierarchy
> > > 
> > > But, I can't do that before the container is started (no lxc directory) or after it is started (device busy).
> > > 
> > 
> > Yup, it looks like lxc ought to set that at startup.  I can think of
> > absolutely no cases where we would *not* want that done.  Something
> > like:
> > 
> > Subject: [PATCH 1/1] Try to enable memory cgroup use_hierarchy option.
> > 
> > The memory cgroup has a 'memory.use_hierarchy' which is initialized to
> > 0.  It needs to be set to 1 at our top level, that is
> > /sys/fs/cgroup/memory/lxc/memory.use_hiararchy, before we create any
> > containers.  After the fact is too late.
> > 
> > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> 
> Any reason this patch was not pushed ?
> 
> After fixing it a bit (it was missing some directory creation and needed some massage), it works as expected on 0.8.0 
> and it prevents kernel warning about memcgroup use_hierarchy warning. 
> 
> New version (for 0.8.0) attached.
> 
> 
> -- 
> Frederic Crozat <fcrozat at suse.com>
> SUSE
> 
> -- 
> Frederic Crozat <fcrozat at suse.com>
> SUSE

> Date: Mon, 4 Feb 2013 08:38:19 -0600
> From: Serge Hallyn <serge.hallyn at canonical.com>
> Subject: [PATCH 1/1] Try to enable memory cgroup use_hierarchy option.
> 
> The memory cgroup has a 'memory.use_hierarchy' which is initialized to
> 0.  It needs to be set to 1 at our top level, that is
> /sys/fs/cgroup/memory/lxc/memory.use_hiararchy, before we create any
> containers.  After the fact is too late.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  src/lxc/cgroup.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> Index: lxc-0.8.0/src/lxc/cgroup.c
> ===================================================================
> --- lxc-0.8.0.orig/src/lxc/cgroup.c
> +++ lxc-0.8.0/src/lxc/cgroup.c
> @@ -356,6 +356,52 @@ int try_to_move_cgname(char *cgparent, c
>  }
>  
>  /*
> + * The memory cgroup has a 'memory.use_hierarchy' which is initialized to
> + * 0.  It needs to be set to 1 at our top level, that is
> + * /sys/fs/cgroup/memory/lxc/memory.use_hierarchy, before we create any
> + * containers.  After the fact is too late.
> + */
> +static int enable_hierarchy(char *path)
> +{
> +	char filepath[MAXPATHLEN];
> +	int ret;
> +	FILE *f;
> +	int len = strlen(path);
> +
> +	if (strlen(path) < 15 || strncmp(path+15, "memory",6) != 0)
> +		return 0;
> +
> +	ret = snprintf(filepath, MAXPATHLEN, "%s/memory.use_hierarchy", path);
> +	if (ret < 0 || ret >= MAXPATHLEN) {
> +		SYSERROR("Failed creating memory hierarchy filepath name");
> +		return -1;
> +	}

Thanks - clearly my patch needed some love :)  Just a few comments below.

> +
> +	/* check if directory exists, and create it otherwise */
> +	if (access(path, F_OK)) {
> +		ret = mkdir(path, 0755);
> +		if (ret == -1 && errno == EEXIST) {

Did you mean errno != EEXIST?

> +			SYSERROR("failed to create '%s' directory", path);
> +			return -1;
> +		}
> +	}
> +
> +	f = fopen(filepath, "w");
> +	if (!f) {
> +		/* kernel may not support this file - s'ok */
> +		WARN("Unable to open cgroup memory.use_hierarchy file");
> +		return 0;
> +	}
> +
> +	if (fprintf(f, "1") < 1) {
> +		WARN("Unable to set hierarchy to true for memory cgroup");
> +	}
> +	fclose(f);
> +
> +	return 0;
> +}
> +
> +/*
>   * create a cgroup for the container in a particular subsystem.
>   */
>  static int lxc_one_cgroup_create(const char *name,
> @@ -384,6 +430,10 @@ static int lxc_one_cgroup_create(const c
>  		SYSERROR("Failed creating pathname for cgroup parent (%d)\n", ret);
>  		return -1;
>  	}
> +
> +	if (enable_hierarchy(cgparent))
> +		return -1;

Hm, do we want to fail container start on that, or just log a warning?

> +
>  	ret = snprintf(cgname, MAXPATHLEN, "%s/%s", cgparent, name);
>  	if (ret < 0 || ret >= MAXPATHLEN) {
>  		SYSERROR("Failed creating pathname for cgroup (%d)\n", ret);