[RFC][PATCH V5] Add support for oom_score_adj
Scott James Remnant
scott at netsplit.com
Mon May 2 23:28:58 UTC 2011
Looks good to me on a first pass
On Mon, May 2, 2011 at 2:05 PM, Marc - A. Dahlhaus <mad at wol.de> wrote:
> 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 HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/upstart-devel/attachments/20110502/c5b41fd0/attachment-0001.html>
More information about the upstart-devel
mailing list