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

[Juergen Gross <jgross@xxxxxxxx>]

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@xxxxxxxx>
> > Reviewed-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> > ---
> > 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.

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature