[RFC][PATCH] Add support for oom_score_adj

[Marc - A. Dahlhaus]

Hello Scott,

Am 13.03.2011 19:56, schrieb Scott James Remnant:

> Thanks for the patch, a few comments inline below...
> 
> On Sun, Mar 13, 2011 at 11:29 AM, Marc - A. Dahlhaus <mad at wol.de> wrote:
>> --- a/init/job_class.h
>> +++ b/init/job_class.h
>> @@ -94,6 +94,7 @@ typedef enum console_type {
>>  * @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,
>> @@ -142,6 +143,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;
> 
> Why keep both oom_adj and oom_score_adj in the struct? If you only
> kept the oom_score_adj in the struct, you could multiple the oom_adj
> values to it, and divide later.

with both values in the struct it was possible to add exact values for each interface by specifying

oom value
oom score value

in a conf file.

Also this way the calculation of values is in the configuration parsing only which looked cleaner IMO.

>> --- a/init/job_process.c
>> +++ b/init/job_process.c
>> @@ -487,14 +487,20 @@ job_process_spawn (JobClass     *class,
>>                fd = fopen (filename, "w");
>>                if (! fd) {
>> +                       snprintf (filename, sizeof (filename),
>> +                                 "/proc/%d/oom_adj", getpid ());
>> +                       oom_value = class->oom_adj;
>> +                       fd = fopen (filename, "w");
>> +               }
>> +               if (! fd) {
>>
> There's no errno checking here - the file could fail to open for any
> number of reasons, it should only fall back to the old interface in
> the one reason that the new interface doesn't exist.

if (errno == EACCES) {
instead of the first
if (! fd) {
should be ok, right?

> Scott
> 


thanks,

Marc