Re: [PATCH v2 1/2] tools/xenstore: set oom score for xenstore daemon on Linux

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2141 bytes --]

On 08.07.21 19:40, Julien Grall wrote:
> Hi Juergen,
> 
> On 08/06/2021 06:58, Juergen Gross wrote:
>> Xenstored is absolutely mandatory for a Xen host and it can't be
>> restarted, so being killed by OOM-killer in case of memory shortage is
>> to be avoided.
>>
>> Set /proc/$pid/oom_score_adj (if available) to -500 in order to allow
>> xenstored to use large amounts of memory without being killed.
>>
>> Make sure the pid file isn't a left-over from a previous run delete it
>> before starting xenstored.
> 
> This sentence is a bit confusing to read. Do you mean "*To* make 
> sure....*,* delete it before"?

Yes, will change it.

> 
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - set oom score from launch script (Julien Grall)
>> - split off open file descriptor limit setting (Julien Grall)
>> ---
>>   tools/hotplug/Linux/launch-xenstore.in | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/tools/hotplug/Linux/launch-xenstore.in 
>> b/tools/hotplug/Linux/launch-xenstore.in
>> index 019f9d6f4d..3ad71e3d08 100644
>> --- a/tools/hotplug/Linux/launch-xenstore.in
>> +++ b/tools/hotplug/Linux/launch-xenstore.in
>> @@ -59,11 +59,14 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons 
>> && . @CONFIG_DIR@/@CONFIG_LEAF
>>           echo "No xenstored found"
>>           exit 1
>>       }
>> +    rm -f @XEN_RUN_DIR@/xenstored.pid
>>       echo -n Starting $XENSTORED...
>>       $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
>>       systemd-notify --booted 2>/dev/null || timeout_xenstore 
>> $XENSTORED || exit 1
>> +    XS_PID=`cat @XEN_RUN_DIR@/xenstored.pid`
>> +    echo -500 >/proc/$XS_PID/oom_score_adj
> 
> NIT: It would be worth considering to introduce a variable so this can 
> be set from the configuration file.

Do you have any scenario in mind where this would be beneficial?

I'm not against it, but I'm wondering why anybody would want that
to be configurable.

> 
> With or without it:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]