diff mbox series

[v6,1/2] tools/xenstore: set oom score for xenstore daemon on Linux

Message ID 20211012134148.6280-2-jgross@suse.com (mailing list archive)
State New, archived
Headers show
Series tools/xenstore: set resource limits of xenstored | expand

Commit Message

Juergen Gross Oct. 12, 2021, 1:41 p.m. UTC 
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) per default to -500 (this
translates to 50% of dom0 memory size) in order to allow xenstored to
use large amounts of memory without being killed.

The percentage of dom0 memory above which the oom killer is allowed to
kill xenstored can be set via XENSTORED_OOM_MEM_THRESHOLD in
xencommons.

Make sure the pid file isn't a left-over from a previous run delete it
before starting xenstored.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
---
V2:
- set oom score from launch script (Julien Grall)
- split off open file descriptor limit setting (Julien Grall)
V3:
- make oom killer threshold configurable (Julien Grall)
V4:
- extend comment (Ian Jackson)
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 9 +++++++++
 tools/hotplug/Linux/launch-xenstore.in             | 6 ++++++
 2 files changed, 15 insertions(+)

Comments

Stefano Stabellini Oct. 18, 2021, 11:25 p.m. UTC | #1 
Hi Juergen, Ian,

This patch broke gitlab-ci:

https://gitlab.com/xen-project/xen/-/jobs/1690080806

---
 * Executing: /lib/rc/sh/openrc-run.sh /lib/rc/sh/openrc-run.sh /etc/init.d/local start
 * Starting local ... *   Executing "/etc/local.d/xen.start" .../etc/xen/scripts/launch-xenstore: line 64: * 10: syntax error: operand expected (error token is "* 10")

illegal value daemon for XENSTORETYPE
---

See below about what the issue is and a potential fix.


On Tue, 12 Oct 2021, 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) per default to -500 (this
> translates to 50% of dom0 memory size) in order to allow xenstored to
> use large amounts of memory without being killed.
> 
> The percentage of dom0 memory above which the oom killer is allowed to
> kill xenstored can be set via XENSTORED_OOM_MEM_THRESHOLD in
> xencommons.
> 
> Make sure the pid file isn't a left-over from a previous run delete it
> before starting xenstored.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Ian Jackson <iwj@xenproject.org>
> ---
> V2:
> - set oom score from launch script (Julien Grall)
> - split off open file descriptor limit setting (Julien Grall)
> V3:
> - make oom killer threshold configurable (Julien Grall)
> V4:
> - extend comment (Ian Jackson)
> ---
>  tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 9 +++++++++
>  tools/hotplug/Linux/launch-xenstore.in             | 6 ++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> index 00cf7f91d4..b83101ab7e 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> @@ -48,6 +48,15 @@ XENSTORED_ARGS=
>  # Only evaluated if XENSTORETYPE is "daemon".
>  #XENSTORED_TRACE=[yes|on|1]
>  
> +## Type: integer
> +## Default: 50
> +#
> +# Percentage of dom0 memory size the xenstore daemon can use before the
> +# OOM killer is allowed to kill it.
> +# The specified value is multiplied by -10 and echoed to
> +# /proc/PID/oom_score_adj.
> +#XENSTORED_OOM_MEM_THRESHOLD=50
> +
>  ## Type: string
>  ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
>  #
> diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
> index 019f9d6f4d..1747c96065 100644
> --- a/tools/hotplug/Linux/launch-xenstore.in
> +++ b/tools/hotplug/Linux/launch-xenstore.in
> @@ -59,11 +59,17 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
>  		echo "No xenstored found"
>  		exit 1
>  	}
> +	[ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
> +	XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))

These are the problematic lines. They don't work with busybox's bash
implementation. Originally I thought it was an issue with busybox bash
implementation but it looks like they don't even work with normal bash.
Specifically the first line is an issue, it should be:

if [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ]
then
    XENSTORED_OOM_MEM_THRESHOLD=50
fi

Right?
Juergen Gross Oct. 19, 2021, 4:23 a.m. UTC | #2 
On 19.10.21 01:25, Stefano Stabellini wrote:
> Hi Juergen, Ian,
> 
> This patch broke gitlab-ci:
> 
> https://gitlab.com/xen-project/xen/-/jobs/1690080806
> 
> ---
>   * Executing: /lib/rc/sh/openrc-run.sh /lib/rc/sh/openrc-run.sh /etc/init.d/local start
>   * Starting local ... *   Executing "/etc/local.d/xen.start" .../etc/xen/scripts/launch-xenstore: line 64: * 10: syntax error: operand expected (error token is "* 10")
> 
> illegal value daemon for XENSTORETYPE
> ---
> 
> See below about what the issue is and a potential fix.
> 
> 
> On Tue, 12 Oct 2021, 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) per default to -500 (this
>> translates to 50% of dom0 memory size) in order to allow xenstored to
>> use large amounts of memory without being killed.
>>
>> The percentage of dom0 memory above which the oom killer is allowed to
>> kill xenstored can be set via XENSTORED_OOM_MEM_THRESHOLD in
>> xencommons.
>>
>> Make sure the pid file isn't a left-over from a previous run delete it
>> before starting xenstored.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Ian Jackson <iwj@xenproject.org>
>> ---
>> V2:
>> - set oom score from launch script (Julien Grall)
>> - split off open file descriptor limit setting (Julien Grall)
>> V3:
>> - make oom killer threshold configurable (Julien Grall)
>> V4:
>> - extend comment (Ian Jackson)
>> ---
>>   tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 9 +++++++++
>>   tools/hotplug/Linux/launch-xenstore.in             | 6 ++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
>> index 00cf7f91d4..b83101ab7e 100644
>> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
>> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
>> @@ -48,6 +48,15 @@ XENSTORED_ARGS=
>>   # Only evaluated if XENSTORETYPE is "daemon".
>>   #XENSTORED_TRACE=[yes|on|1]
>>   
>> +## Type: integer
>> +## Default: 50
>> +#
>> +# Percentage of dom0 memory size the xenstore daemon can use before the
>> +# OOM killer is allowed to kill it.
>> +# The specified value is multiplied by -10 and echoed to
>> +# /proc/PID/oom_score_adj.
>> +#XENSTORED_OOM_MEM_THRESHOLD=50
>> +
>>   ## Type: string
>>   ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
>>   #
>> diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
>> index 019f9d6f4d..1747c96065 100644
>> --- a/tools/hotplug/Linux/launch-xenstore.in
>> +++ b/tools/hotplug/Linux/launch-xenstore.in
>> @@ -59,11 +59,17 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
>>   		echo "No xenstored found"
>>   		exit 1
>>   	}
>> +	[ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
>> +	XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
> 
> These are the problematic lines. They don't work with busybox's bash
> implementation. Originally I thought it was an issue with busybox bash
> implementation but it looks like they don't even work with normal bash.
> Specifically the first line is an issue, it should be:
> 
> if [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ]
> then
>      XENSTORED_OOM_MEM_THRESHOLD=50
> fi
> 
> Right?
> 

Oh, shame on me. Turned out that I had XENSTORED_OOM_MEM_THRESHOLD
set explicitly in my xencommons file. :-(

Patch is coming...


Juergen

Patch is coming.
Stefano Stabellini Oct. 19, 2021, 8:48 p.m. UTC | #3 
On Tue, 19 Oct 2021, Juergen Gross wrote:
> On 19.10.21 01:25, Stefano Stabellini wrote:
> > Hi Juergen, Ian,
> > 
> > This patch broke gitlab-ci:
> > 
> > https://gitlab.com/xen-project/xen/-/jobs/1690080806
> > 
> > ---
> >   * Executing: /lib/rc/sh/openrc-run.sh /lib/rc/sh/openrc-run.sh
> > /etc/init.d/local start
> >   * Starting local ... *   Executing "/etc/local.d/xen.start"
> > .../etc/xen/scripts/launch-xenstore: line 64: * 10: syntax error: operand
> > expected (error token is "* 10")
> > 
> > illegal value daemon for XENSTORETYPE
> > ---
> > 
> > See below about what the issue is and a potential fix.
> > 
> > 
> > On Tue, 12 Oct 2021, 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) per default to -500 (this
> > > translates to 50% of dom0 memory size) in order to allow xenstored to
> > > use large amounts of memory without being killed.
> > > 
> > > The percentage of dom0 memory above which the oom killer is allowed to
> > > kill xenstored can be set via XENSTORED_OOM_MEM_THRESHOLD in
> > > xencommons.
> > > 
> > > Make sure the pid file isn't a left-over from a previous run delete it
> > > before starting xenstored.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > Reviewed-by: Ian Jackson <iwj@xenproject.org>
> > > ---
> > > V2:
> > > - set oom score from launch script (Julien Grall)
> > > - split off open file descriptor limit setting (Julien Grall)
> > > V3:
> > > - make oom killer threshold configurable (Julien Grall)
> > > V4:
> > > - extend comment (Ian Jackson)
> > > ---
> > >   tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 9 +++++++++
> > >   tools/hotplug/Linux/launch-xenstore.in             | 6 ++++++
> > >   2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > > b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > > index 00cf7f91d4..b83101ab7e 100644
> > > --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > > +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > > @@ -48,6 +48,15 @@ XENSTORED_ARGS=
> > >   # Only evaluated if XENSTORETYPE is "daemon".
> > >   #XENSTORED_TRACE=[yes|on|1]
> > >   +## Type: integer
> > > +## Default: 50
> > > +#
> > > +# Percentage of dom0 memory size the xenstore daemon can use before the
> > > +# OOM killer is allowed to kill it.
> > > +# The specified value is multiplied by -10 and echoed to
> > > +# /proc/PID/oom_score_adj.
> > > +#XENSTORED_OOM_MEM_THRESHOLD=50
> > > +
> > >   ## Type: string
> > >   ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
> > >   #
> > > diff --git a/tools/hotplug/Linux/launch-xenstore.in
> > > b/tools/hotplug/Linux/launch-xenstore.in
> > > index 019f9d6f4d..1747c96065 100644
> > > --- a/tools/hotplug/Linux/launch-xenstore.in
> > > +++ b/tools/hotplug/Linux/launch-xenstore.in
> > > @@ -59,11 +59,17 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && .
> > > @CONFIG_DIR@/@CONFIG_LEAF
> > >   		echo "No xenstored found"
> > >   		exit 1
> > >   	}
> > > +	[ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] ||
> > > XENSTORED_OOM_MEM_THRESHOLD=50
> > > +	XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
> > 
> > These are the problematic lines. They don't work with busybox's bash
> > implementation. Originally I thought it was an issue with busybox bash
> > implementation but it looks like they don't even work with normal bash.
> > Specifically the first line is an issue, it should be:
> > 
> > if [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ]
> > then
> >      XENSTORED_OOM_MEM_THRESHOLD=50
> > fi
> > 
> > Right?
> > 
> 
> Oh, shame on me. Turned out that I had XENSTORED_OOM_MEM_THRESHOLD
> set explicitly in my xencommons file. :-(
> 
> Patch is coming...

Thanks Juergen, gitlab-ci is all green again:
https://gitlab.com/xen-project/xen/-/pipelines/391015163

Thanks!
diff mbox series

Patch

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index 00cf7f91d4..b83101ab7e 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -48,6 +48,15 @@  XENSTORED_ARGS=
 # Only evaluated if XENSTORETYPE is "daemon".
 #XENSTORED_TRACE=[yes|on|1]
 
+## Type: integer
+## Default: 50
+#
+# Percentage of dom0 memory size the xenstore daemon can use before the
+# OOM killer is allowed to kill it.
+# The specified value is multiplied by -10 and echoed to
+# /proc/PID/oom_score_adj.
+#XENSTORED_OOM_MEM_THRESHOLD=50
+
 ## Type: string
 ## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
 #
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index 019f9d6f4d..1747c96065 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -59,11 +59,17 @@  test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
 		echo "No xenstored found"
 		exit 1
 	}
+	[ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
+	XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
+
+	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 $XS_OOM_SCORE >/proc/$XS_PID/oom_score_adj
 
 	exit 0
 }