[RFC][PATCH V5] Add support for oom_score_adj

Marc - A. Dahlhaus mad at wol.de
Mon May 2 21:05:09 UTC 2011


Am 17.03.2011 21:48, schrieb Scott James Remnant:
> (Replying to the new patch separately)
> 
> On Thu, Mar 17, 2011 at 11:58 AM, Marc - A. Dahlhaus <mad at wol.de> wrote:
> 
>> The never case was no problem as i set booth values on never.
>> But i get your point. Rewrote it per your comments. Result attached.
>>
> This looks perfect, could you add some tests for "oom score" to the
> parsing code, I see you've already updated the existing ones. It also
> needs a man page update (I suggest just dropping "oom NN" out of the
> man page and replacing it with "score")
> 
> Then it looks ready to apply!
> 
> Thankyou,
> Scott

This should be it...

Running for more weeks without any problem here.

3 Test-Boxes (x86_64 and i686) did several boots, shutdowns and reboots
no problems detected.

thanks,

Marc

--- a/init/errors.h
+++ b/init/errors.h
@@ -62,7 +62,8 @@ enum {
 #define PARSE_ILLEGAL_EXIT_STR		N_("Illegal exit status, expected integer")
 #define PARSE_ILLEGAL_UMASK_STR		N_("Illegal file creation mask,
expected octal integer")
 #define PARSE_ILLEGAL_NICE_STR		N_("Illegal nice value, expected -20 to
19")
-#define PARSE_ILLEGAL_OOM_STR		N_("Illegal oom adjustment, expected -16
to 15 or never")
+#define PARSE_ILLEGAL_OOM_STR		N_("Illegal oom adjustment, expected -16
to 15 or 'never'")
+#define PARSE_ILLEGAL_OOM_SCORE_STR	N_("Illegal oom score adjustment,
expected -999 to 1000 or 'never'")
 #define PARSE_ILLEGAL_LIMIT_STR		N_("Illegal limit, expected
'unlimited' or integer")
 #define PARSE_EXPECTED_EVENT_STR	N_("Expected event")
 #define PARSE_EXPECTED_OPERATOR_STR	N_("Expected operator")
--- a/init/job_class.c
+++ b/init/job_class.c
@@ -211,7 +211,7 @@ job_class_new (const void *parent,

 	class->umask = JOB_DEFAULT_UMASK;
 	class->nice = 0;
-	class->oom_adj = 0;
+	class->oom_score_adj = 0;

 	for (i = 0; i < RLIMIT_NLIMITS; i++)
 		class->limits[i] = NULL;
--- a/init/job_class.h
+++ b/init/job_class.h
@@ -93,7 +93,7 @@ typedef enum console_type {
  * @console: how to arrange processes' stdin/out/err file descriptors,
  * @umask: file mode creation mask,
  * @nice: process priority,
- * @oom_adj: OOM killer adjustment,
+ * @oom_score_adj: OOM killer score adjustment,
  * @limits: resource limits indexed by resource,
  * @chroot: root directory of process (implies @chdir if not set),
  * @chdir: working directory of process,
@@ -141,7 +141,7 @@ typedef struct job_class {

 	mode_t          umask;
 	int             nice;
-	int             oom_adj;
+	int             oom_score_adj;
 	struct rlimit  *limits[RLIMIT_NLIMITS];
 	char           *chroot;
 	char           *chdir;
@@ -150,6 +150,8 @@ typedef struct job_class {
 	int             debug;
 } JobClass;

+#define SCORE_TO_ADJ(x) ((x * ((x < 0) ? 17 : 15)) / 1000)
+#define ADJ_TO_SCORE(x) ((x * 1000) / ((x < 0) ? 17 : 15))

 NIH_BEGIN_EXTERN

--- a/init/job_process.c
+++ b/init/job_process.c
@@ -371,7 +371,7 @@ job_process_spawn (JobClass     *class,
 {
 	sigset_t  child_set, orig_set;
 	pid_t     pid;
-	int       i, fds[2];
+	int       i, fds[2], oom_value;
 	char      filename[PATH_MAX];
 	FILE     *fd;

@@ -493,16 +493,22 @@ job_process_spawn (JobClass     *class,

 	/* Adjust the process OOM killer priority.
 	 */
-	if (class->oom_adj) {
+	if (class->oom_score_adj) {
 		snprintf (filename, sizeof (filename),
-			  "/proc/%d/oom_adj", getpid ());
-
+			  "/proc/%d/oom_score_adj", getpid ());
+		oom_value = class->oom_score_adj;
 		fd = fopen (filename, "w");
+		if ((! fd) && (errno == EACCES)) {
+			snprintf (filename, sizeof (filename),
+				  "/proc/%d/oom_adj", getpid ());
+			oom_value = SCORE_TO_ADJ(class->oom_score_adj);
+			fd = fopen (filename, "w");
+		}
 		if (! fd) {
 			nih_error_raise_system ();
 			job_process_error_abort (fds[1], JOB_PROCESS_ERROR_OOM_ADJ, 0);
 		} else {
-			fprintf (fd, "%d\n", class->oom_adj);
+			fprintf (fd, "%d\n", oom_value);

 			if (fclose (fd)) {
 				nih_error_raise_system ();
--- a/init/man/init.5
+++ b/init/man/init.5
@@ -500,15 +500,15 @@ see
 for more details.
 .\"
 .TP
-.B oom \fIADJUSTMENT\fR|\fBnever
+.B oom score \fIADJUSTMENT\fR|\fBnever
 Normally the OOM killer regards all processes equally, this stanza
 advises the kernel to treat this job differently.

 .I ADJUSTMENT
 may be an integer value from
-.I -16
+.I -999
 (very unlikely to be killed by the OOM killer) up to
-.I 14
+.I 1000
 (very likely to be killed by the OOM killer).  It may also be the special
 value
 .B never
--- a/init/parse_job.c
+++ b/init/parse_job.c
@@ -2233,6 +2233,7 @@ stanza_oom (JobClass        *class,
 	nih_local char *arg = NULL;
 	char           *endptr;
 	size_t          a_pos, a_lineno;
+	int		oom_adj;
 	int             ret = -1;

 	nih_assert (class != NULL);
@@ -2247,12 +2248,37 @@ stanza_oom (JobClass        *class,
 	if (! arg)
 		goto finish;

-	if (! strcmp (arg, "never")) {
-		class->oom_adj = -17;
+	if (! strcmp (arg, "score")) {
+		nih_local char *scorearg = NULL;
+
+		/* Update error position to the score value */
+		*pos = a_pos;
+		if (lineno)
+			*lineno = a_lineno;
+
+		scorearg = nih_config_next_arg (NULL, file, len,
+						&a_pos, &a_lineno);
+		if (! scorearg)
+			goto finish;
+
+		if (! strcmp (scorearg, "never")) {
+			class->oom_score_adj = -1000;
+		} else {
+			errno = 0;
+			class->oom_score_adj = (int)strtol (scorearg, &endptr, 10);
+			if (errno || *endptr ||
+			    (class->oom_score_adj < -1000) ||
+			    (class->oom_score_adj > 1000))
+				nih_return_error (-1, PARSE_ILLEGAL_OOM,
+						  _(PARSE_ILLEGAL_OOM_SCORE_STR));
+		}
+	} else if (! strcmp (arg, "never")) {
+		class->oom_score_adj = -1000;
 	} else {
 		errno = 0;
-		class->oom_adj = (int)strtol (arg, &endptr, 10);
-		if (errno || *endptr || (class->oom_adj < -17) || (class->oom_adj > 15))
+		oom_adj = (int)strtol (arg, &endptr, 10);
+		class->oom_score_adj = ADJ_TO_SCORE(oom_adj);
+		if (errno || *endptr || (oom_adj < -17) || (oom_adj > 15))
 			nih_return_error (-1, PARSE_ILLEGAL_OOM,
 					  _(PARSE_ILLEGAL_OOM_STR));
 	}
--- a/init/tests/test_job_class.c
+++ b/init/tests/test_job_class.c
@@ -133,7 +133,7 @@ test_new (void)

 		TEST_EQ (class->umask, 022);
 		TEST_EQ (class->nice, 0);
-		TEST_EQ (class->oom_adj, 0);
+		TEST_EQ (class->oom_score_adj, 0);

 		for (i = 0; i < RLIMIT_NLIMITS; i++)
 			TEST_EQ_P (class->limits[i], NULL);
--- a/init/tests/test_parse_job.c
+++ b/init/tests/test_parse_job.c
@@ -6198,11 +6198,39 @@ test_stanza_oom (void)

 		TEST_ALLOC_SIZE (job, sizeof (JobClass));

-		TEST_EQ (job->oom_adj, 10);
+		TEST_EQ (job->oom_score_adj, ADJ_TO_SCORE(10));

 		nih_free (job);
 	}

+	TEST_FEATURE ("with positive score argument");
+	strcpy (buf, "oom score 100\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 2);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, 100);
+
+		nih_free (job);
+	}

 	/* Check that an oom stanza with a negative timeout results
 	 * in it being stored in the job.
@@ -6231,7 +6259,36 @@ test_stanza_oom (void)

 		TEST_ALLOC_SIZE (job, sizeof (JobClass));

-		TEST_EQ (job->oom_adj, -10);
+		TEST_EQ (job->oom_score_adj, ADJ_TO_SCORE(-10));
+
+		nih_free (job);
+	}
+
+	TEST_FEATURE ("with negative score argument");
+	strcpy (buf, "oom score -100\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 2);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, -100);

 		nih_free (job);
 	}
@@ -6264,7 +6321,40 @@ test_stanza_oom (void)

 		TEST_ALLOC_SIZE (job, sizeof (JobClass));

-		TEST_EQ (job->oom_adj, -17);
+		TEST_EQ (job->oom_score_adj, ADJ_TO_SCORE(-17));
+
+		nih_free (job);
+	}
+
+
+	/* Check that an oom score stanza may have the special never
+	 *  argument which stores -1000 in the job.
+	 */
+	TEST_FEATURE ("with never score argument");
+	strcpy (buf, "oom score never\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 2);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, -1000);

 		nih_free (job);
 	}
@@ -6297,7 +6387,100 @@ test_stanza_oom (void)

 		TEST_ALLOC_SIZE (job, sizeof (JobClass));

-		TEST_EQ (job->oom_adj, 10);
+		TEST_EQ (job->oom_score_adj, ADJ_TO_SCORE(10));
+
+		nih_free (job);
+	}
+
+	TEST_FEATURE ("with multiple score stanzas");
+	strcpy (buf, "oom score -500\n");
+	strcat (buf, "oom score 500\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 3);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, 500);
+
+		nih_free (job);
+	}
+
+	/* Check that the last of multiple distinct oom stanzas is
+	 * used.
+	 */
+	TEST_FEATURE ("with an oom overriding an oom score stanza");
+	strcpy (buf, "oom score -10\n");
+	strcat (buf, "oom 10\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 3);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, ADJ_TO_SCORE(10));
+
+		nih_free (job);
+	}
+
+	TEST_FEATURE ("with an oom score overriding an oom stanza");
+	strcpy (buf, "oom -10\n");
+	strcat (buf, "oom score 10\n");
+
+	TEST_ALLOC_FAIL {
+		pos = 0;
+		lineno = 1;
+		job = parse_job (NULL, "test", buf, strlen (buf),
+				 &pos, &lineno);
+
+		if (test_alloc_failed) {
+			TEST_EQ_P (job, NULL);
+
+			err = nih_error_get ();
+			TEST_EQ (err->number, ENOMEM);
+			nih_free (err);
+
+			continue;
+		}
+
+		TEST_EQ (pos, strlen (buf));
+		TEST_EQ (lineno, 3);
+
+		TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+		TEST_EQ (job->oom_score_adj, 10);

 		nih_free (job);
 	}
@@ -6322,6 +6505,25 @@ test_stanza_oom (void)
 	nih_free (err);


+	/* Check that an oom score stanza without an argument results in a
+	 * syntax error.
+	 */
+	TEST_FEATURE ("with missing score argument");
+	strcpy (buf, "oom score\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_EXPECTED_TOKEN);
+	TEST_EQ (pos, 9);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+
+
 	/* Check that an oom stanza with an overly large argument results
 	 * in a syntax error.
 	 */
@@ -6340,6 +6542,21 @@ test_stanza_oom (void)
 	TEST_EQ (lineno, 1);
 	nih_free (err);

+	TEST_FEATURE ("with overly large score argument");
+	strcpy (buf, "oom score 1200\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, PARSE_ILLEGAL_OOM);
+	TEST_EQ (pos, 10);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+

 	/* Check that an oom stanza with an overly small argument results
 	 * in a syntax error.
@@ -6359,6 +6576,21 @@ test_stanza_oom (void)
 	TEST_EQ (lineno, 1);
 	nih_free (err);

+	TEST_FEATURE ("with overly small score argument");
+	strcpy (buf, "oom score -1200\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, PARSE_ILLEGAL_OOM);
+	TEST_EQ (pos, 10);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+

 	/* Check that an oom stanza with a non-integer argument results
 	 * in a syntax error.
@@ -6378,6 +6610,21 @@ test_stanza_oom (void)
 	TEST_EQ (lineno, 1);
 	nih_free (err);

+	TEST_FEATURE ("with non-integer score argument");
+	strcpy (buf, "oom score foo\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, PARSE_ILLEGAL_OOM);
+	TEST_EQ (pos, 10);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+

 	/* Check that an oom stanza with a partially numeric argument
 	 * results in a syntax error.
@@ -6397,6 +6644,21 @@ test_stanza_oom (void)
 	TEST_EQ (lineno, 1);
 	nih_free (err);

+	TEST_FEATURE ("with alphanumeric score argument");
+	strcpy (buf, "oom score 12foo\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, PARSE_ILLEGAL_OOM);
+	TEST_EQ (pos, 10);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
+

 	/* Check that an oom stanza with a priority but with an extra
 	 * argument afterwards results in a syntax error.
@@ -6415,6 +6677,21 @@ test_stanza_oom (void)
 	TEST_EQ (pos, 7);
 	TEST_EQ (lineno, 1);
 	nih_free (err);
+
+	TEST_FEATURE ("with extra score argument");
+	strcpy (buf, "oom score 500 foo\n");
+
+	pos = 0;
+	lineno = 1;
+	job = parse_job (NULL, "test", buf, strlen (buf), &pos, &lineno);
+
+	TEST_EQ_P (job, NULL);
+
+	err = nih_error_get ();
+	TEST_EQ (err->number, NIH_CONFIG_UNEXPECTED_TOKEN);
+	TEST_EQ (pos, 14);
+	TEST_EQ (lineno, 1);
+	nih_free (err);
 }

 void
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: oom_score_adj.patch
URL: <https://lists.ubuntu.com/archives/upstart-devel/attachments/20110502/de3b9a52/attachment.ksh>


More information about the upstart-devel mailing list