From 5baf98ba3b6a9d6ef304fdedfb0fce9185229aa6 Mon Sep 17 00:00:00 2001
From: Matthieu Hautreux <matthieu.hautreux@cea.fr>
Date: Wed, 12 Sep 2012 22:42:23 +0200
Subject: [PATCH] task/cgroup/memory - ensure that ConstrainSwapSpace=no is
 correctly handled

Only set memory.memsw.limit_in_bytes to the computed amount of allowed
RAM+Swap when ConstrainSwapSpace=yes in cgroup.conf.
When ConstrainSwapSpace=yes and ConstrainRAMSpace=no, automatically
set AllowedRAMSpace to 100% in order to compute the memsw.limit based
on the allocated memory plus the allowed swap percent. Then use that
limit for both mem and mem+sw limits.
---
 doc/man/man5/cgroup.conf.5                   |  8 ++-
 src/plugins/task/cgroup/task_cgroup_memory.c | 69 +++++++++++++++-----
 2 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/doc/man/man5/cgroup.conf.5 b/doc/man/man5/cgroup.conf.5
index c5807b680d5..69c48bbc574 100644
--- a/doc/man/man5/cgroup.conf.5
+++ b/doc/man/man5/cgroup.conf.5
@@ -93,7 +93,13 @@ Also see \fBAllowedRAMSpace\fR.
 .TP
 \fBConstrainSwapSpace\fR=<yes|no>
 If configured to "yes" then constrain the job's swap space usage.
-The default value is "no".
+The default value is "no". Note that when set to "yes" and 
+ConstrainRAMSpace is set to "no", AllowedRAMSpace is automatically set
+to 100% in order to limit the RAM+Swap amount to 100% of job's requirement
+plus the percent of allowed swap space. This amount is thus set to both
+RAM and RAM+Swap limits. This means that in that particular case,
+ConstrainRAMSpace is automatically enabled with the same limit than the one
+used to constrain swap space.
 Also see \fBAllowedSwapSpace\fR.
 
 .TP
diff --git a/src/plugins/task/cgroup/task_cgroup_memory.c b/src/plugins/task/cgroup/task_cgroup_memory.c
index a2e24806629..6ca89fe48ba 100644
--- a/src/plugins/task/cgroup/task_cgroup_memory.c
+++ b/src/plugins/task/cgroup/task_cgroup_memory.c
@@ -68,6 +68,9 @@ static xcgroup_t user_memory_cg;
 static xcgroup_t job_memory_cg;
 static xcgroup_t step_memory_cg;
 
+static bool constrain_ram_space;
+static bool constrain_swap_space;
+
 static float allowed_ram_space;   /* Allowed RAM in percent       */
 static float allowed_swap_space;  /* Allowed Swap percent         */
 
@@ -120,7 +123,21 @@ extern int task_cgroup_memory_init(slurm_cgroup_conf_t *slurm_cgroup_conf)
 		}
 	}
 
-	allowed_ram_space = slurm_cgroup_conf->allowed_ram_space;
+	constrain_ram_space = slurm_cgroup_conf->constrain_ram_space;
+	constrain_swap_space = slurm_cgroup_conf->constrain_swap_space;
+
+	/* 
+	 * as the swap space threshold will be configured with a 
+	 * mem+swp parameter value, if RAM space is not monitored,
+	 * set allowed RAM space to 100% of the job requested memory.
+	 * It will help to construct the mem+swp value that will be
+	 * used for both mem and mem+swp limit during memcg creation.
+	 */
+	if ( constrain_ram_space )
+		allowed_ram_space = slurm_cgroup_conf->allowed_ram_space;
+	else
+		allowed_ram_space = 100.0;
+
 	allowed_swap_space = slurm_cgroup_conf->allowed_swap_space;
 
 	if ((totalram = (uint64_t) conf->real_memory_size) == 0)
@@ -131,17 +148,19 @@ extern int task_cgroup_memory_init(slurm_cgroup_conf_t *slurm_cgroup_conf)
 	max_swap += max_ram;
 	min_ram_space = slurm_cgroup_conf->min_ram_space * 1024 * 1024;
 
-	debug ("task/cgroup/memory: total:%luM allowed:%.4g%%, swap:%.4g%%, "
-	      "max:%.4g%%(%luM) max+swap:%.4g%%(%luM) min:%uM",
-	      (unsigned long) totalram,
-	      allowed_ram_space,
-	      allowed_swap_space,
-	      slurm_cgroup_conf->max_ram_percent,
-	      (unsigned long) (max_ram/(1024*1024)),
-	      slurm_cgroup_conf->max_swap_percent,
-	      (unsigned long) (max_swap/(1024*1024)),
-	      (unsigned) slurm_cgroup_conf->min_ram_space);
-
+	debug ("task/cgroup/memory: total:%luM allowed:%.4g%%(%s), "
+	       "swap:%.4g%%(%s), max:%.4g%%(%luM) max+swap:%.4g%%(%luM) min:%uM",
+	       (unsigned long) totalram,
+	       allowed_ram_space,
+	       constrain_ram_space?"enforced":"permissive",
+	       allowed_swap_space,
+	       constrain_swap_space?"enforced":"permissive",
+	       slurm_cgroup_conf->max_ram_percent,
+	       (unsigned long) (max_ram/(1024*1024)),
+	       slurm_cgroup_conf->max_swap_percent,
+	       (unsigned long) (max_swap/(1024*1024)),
+	       (unsigned) slurm_cgroup_conf->min_ram_space);
+	
         /*
          *  Warning: OOM Killer must be disabled for slurmstepd
          *  or it would be destroyed if the application use
@@ -264,14 +283,28 @@ static int memcg_initialize (xcgroup_ns_t *ns, xcgroup_t *cg,
 	}
 
 	xcgroup_set_param (cg, "memory.use_hierarchy","1");
+
+	/* when RAM space has not to be constrained and we are here, it
+	   means that only Swap space has to be constrained. Thus set 
+	   RAM space limit to the mem+swap limit too */
+	if ( ! constrain_ram_space )
+		mlb = mls;
 	xcgroup_set_uint64_param (cg, "memory.limit_in_bytes", mlb);
-	xcgroup_set_uint64_param (cg, "memory.memsw.limit_in_bytes", mls);
 
-	info ("task/cgroup: %s: alloc=%luMB mem.limit=%luMB memsw.limit=%luMB",
-		path,
-		(unsigned long) mem_limit,
-		(unsigned long) mlb/(1024*1024),
-		(unsigned long) mls/(1024*1024));
+	/* this limit has to be set only if ConstrainSwapSpace is set to yes */
+	if ( constrain_swap_space ) {
+		xcgroup_set_uint64_param (cg, "memory.memsw.limit_in_bytes",
+					  mls);
+		info ("task/cgroup: %s: alloc=%luMB mem.limit=%luMB "
+		      "memsw.limit=%luMB",path,
+		      (unsigned long) mem_limit,
+		      (unsigned long) mlb/(1024*1024),
+		      (unsigned long) mls/(1024*1024));
+	} else
+		info ("task/cgroup: %s: alloc=%luMB mem.limit=%luMB "
+		      "memsw.limit=unlimited",path,
+		      (unsigned long) mem_limit,
+		      (unsigned long) mlb/(1024*1024));
 
 	return 0;
 }
-- 
GitLab