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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@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...

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

Thanks!