[RFC][PATCH] Add support for oom_score_adj

[Marc - A. Dahlhaus]

Am 15.03.2011 01:28, schrieb Scott James Remnant:

> On Sun, Mar 13, 2011 at 1:10 PM, Marc - A. Dahlhaus <mad at wol.de> wrote:
> 
>> 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.
>>
> I don't see why this is cleaner?  It adds two members to the struct
> which have the same meaning, and with a non-standard method of
> overriding - "oom score" always overrides "oom" which isn't consistent
> with "last one wins"
> 
> e.g. if a config has
> 
>   oom score -150
> 
> and you do
> 
>   echo oom never >> /etc/init/foo.override
> 
> The score is still -150, the new value doesn't take effect.

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.

BTW is the override file stuff upstreamed into the 1.X branch already?

> 
> Having "oom" simply rescale the -17...+15 value would fix this.
> 
>>> 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?
>>
> if ((! fd) && (errno == EACCES)) I guess
> 
> Scott
> 


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: oom_score_adj_v2.patch
URL: <https://lists.ubuntu.com/archives/upstart-devel/attachments/20110317/07c7d279/attachment.ksh>