All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored
@ 2021-10-12 13:41 Juergen Gross
  2021-10-12 13:41 ` [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux Juergen Gross
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Juergen Gross @ 2021-10-12 13:41 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu

Set some limits for xenstored in order to avoid it being killed by
OOM killer, or to run out of file descriptors.

Changes in V6:
- start daemon directly via prlimit

Changes in V5:
- respect /proc/sys/fs/nr_open (Ian Jackson)

Changes in V4:
- add comments
- switch to configure open file descriptors directly

Changes in V3:
- make oom score configurable

Changes in V2:
- split into 2 patches
- set limits from start script

Juergen Gross (2):
  tools/xenstore: set oom score for xenstore daemon on Linux
  tools/xenstore: set open file descriptor limit for xenstored

 .../Linux/init.d/sysconfig.xencommons.in      | 22 +++++++++++++++
 tools/hotplug/Linux/launch-xenstore.in        | 27 ++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
  2021-10-12 13:41 [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Juergen Gross
@ 2021-10-12 13:41 ` Juergen Gross
  2021-10-18 23:25   ` Stefano Stabellini
  2021-10-12 13:41 ` [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored Juergen Gross
  2021-10-18 13:18 ` [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Ian Jackson
  2 siblings, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2021-10-12 13:41 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu

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))
+
+	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
 }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored
  2021-10-12 13:41 [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Juergen Gross
  2021-10-12 13:41 ` [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux Juergen Gross
@ 2021-10-12 13:41 ` Juergen Gross
  2021-10-12 13:56   ` Ian Jackson
  2021-10-18 13:18 ` [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Ian Jackson
  2 siblings, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2021-10-12 13:41 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu

Add a configuration item for the maximum number of open file
descriptors xenstored should be allowed to have.

The default should be "unlimited" in order not to restrict xenstored
in the number of domains it can support, but unfortunately the kernel
is normally limiting the maximum value via /proc/sys/fs/nr_open [1],
[2]. So check that file to exist and if it does, limit the maximum
value to the one specified by /proc/sys/fs/nr_open.

As an aid for the admin configuring the value add a comment specifying
the common needs of xenstored for the different domain types.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=60fd760fb9ff7034360bab7137c917c0330628c2
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c2d64fb6cae9aae480f6a46cfe79f8d7d48b59f

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- set ulimit form launch script (Julien Grall)
- split off from original patch (Julien Grall)
V4:
- switch to directly configuring the limit of file descriptors instead
  of domains (Ian Jackson)
V5:
- use /proc/sys/fs/nr_open (Ian Jackson)
V6:
- start daemon directly via prlimit
---
 .../Linux/init.d/sysconfig.xencommons.in      | 13 ++++++++++++
 tools/hotplug/Linux/launch-xenstore.in        | 21 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index b83101ab7e..433e4849af 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -32,6 +32,19 @@
 # Changing this requires a reboot to take effect.
 #XENSTORED=@XENSTORED@
 
+## Type: string
+## Default: unlimited
+#
+# Select maximum number of file descriptors xenstored is allowed to have
+# opened at one time.
+# For each HVM domain xenstored might need up to 5 open file descriptors,
+# PVH and PV domains will require up to 3 open file descriptors. Additionally
+# 20-30 file descriptors will be opened for internal uses.
+# The specified value (including "unlimited") will be capped by the contents
+# of /proc/sys/fs/nr_open if existing.
+# Only evaluated if XENSTORETYPE is "daemon".
+#XENSTORED_MAX_OPEN_FDS=unlimited
+
 ## Type: string
 ## Default: ""
 #
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index 1747c96065..8438af9977 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -54,6 +54,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
 
 [ "$XENSTORETYPE" = "daemon" ] && {
 	[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log"
+	[ -z "$XENSTORED_MAX_OPEN_FDS" ] && XENSTORED_MAX_OPEN_FDS=unlimited
 	[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
 	[ -x "$XENSTORED" ] || {
 		echo "No xenstored found"
@@ -62,10 +63,28 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF
 	[ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
 	XS_OOM_SCORE=-$(($XENSTORED_OOM_MEM_THRESHOLD * 10))
 
+	[ "$XENSTORED_MAX_OPEN_FDS" = "unlimited" ] || {
+		[ -z "${XENSTORED_MAX_OPEN_FDS//[0-9]}" ] &&
+		[ -n "$XENSTORED_MAX_OPEN_FDS" ] || {
+			echo "XENSTORED_MAX_OPEN_FDS=$XENSTORED_MAX_OPEN_FDS invalid"
+			echo "Setting to default \"unlimited\"."
+			XENSTORED_MAX_OPEN_FDS=unlimited
+		}
+	}
+	[ -r /proc/sys/fs/nr_open ] && {
+		MAX_FDS=`cat /proc/sys/fs/nr_open`
+		[ "$XENSTORED_MAX_OPEN_FDS" = "unlimited" ] && XENSTORED_MAX_OPEN_FDS=$MAX_FDS
+		[ $XENSTORED_MAX_OPEN_FDS -gt $MAX_FDS ] && {
+			echo "XENSTORED_MAX_OPEN_FDS exceeds system limit."
+			echo "Setting to \"$MAX_FDS\"."
+			XENSTORED_MAX_OPEN_FDS=$MAX_FDS
+		}
+	}
+
 	rm -f @XEN_RUN_DIR@/xenstored.pid
 
 	echo -n Starting $XENSTORED...
-	$XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
+	prlimit --nofile=$XENSTORED_MAX_OPEN_FDS $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`
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored
  2021-10-12 13:41 ` [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored Juergen Gross
@ 2021-10-12 13:56   ` Ian Jackson
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Jackson @ 2021-10-12 13:56 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, Wei Liu

Juergen Gross writes ("[PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored"):
> Add a configuration item for the maximum number of open file
> descriptors xenstored should be allowed to have.
> 
> The default should be "unlimited" in order not to restrict xenstored
> in the number of domains it can support, but unfortunately the kernel
> is normally limiting the maximum value via /proc/sys/fs/nr_open [1],
> [2]. So check that file to exist and if it does, limit the maximum
> value to the one specified by /proc/sys/fs/nr_open.
> 
> As an aid for the admin configuring the value add a comment specifying
> the common needs of xenstored for the different domain types.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=60fd760fb9ff7034360bab7137c917c0330628c2
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c2d64fb6cae9aae480f6a46cfe79f8d7d48b59f
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Ian Jackson <iwj@xenproject.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored
  2021-10-12 13:41 [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Juergen Gross
  2021-10-12 13:41 ` [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux Juergen Gross
  2021-10-12 13:41 ` [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored Juergen Gross
@ 2021-10-18 13:18 ` Ian Jackson
  2 siblings, 0 replies; 8+ messages in thread
From: Ian Jackson @ 2021-10-18 13:18 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, Wei Liu

Juergen Gross writes ("[PATCH v6 0/2] tools/xenstore: set resource limits of xenstored"):
> Set some limits for xenstored in order to avoid it being killed by
> OOM killer, or to run out of file descriptors.

It looks like these were overlooked; they should have gone in last
week.

I think this is arguably a bugfix and we have time to fix it if it
causes a problem, and it's only one working day late, so for the
record

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

and I will push these now.

Ian.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
  2021-10-12 13:41 ` [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux Juergen Gross
@ 2021-10-18 23:25   ` Stefano Stabellini
  2021-10-19  4:23     ` Juergen Gross
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2021-10-18 23:25 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, Ian Jackson, Wei Liu

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?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
  2021-10-18 23:25   ` Stefano Stabellini
@ 2021-10-19  4:23     ` Juergen Gross
  2021-10-19 20:48       ` Stefano Stabellini
  0 siblings, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2021-10-19  4:23 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Ian Jackson, Wei Liu


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

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.

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux
  2021-10-19  4:23     ` Juergen Gross
@ 2021-10-19 20:48       ` Stefano Stabellini
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Stabellini @ 2021-10-19 20:48 UTC (permalink / raw)
  To: Juergen Gross; +Cc: Stefano Stabellini, xen-devel, Ian Jackson, Wei Liu

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!


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-10-19 20:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 13:41 [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Juergen Gross
2021-10-12 13:41 ` [PATCH v6 1/2] tools/xenstore: set oom score for xenstore daemon on Linux Juergen Gross
2021-10-18 23:25   ` Stefano Stabellini
2021-10-19  4:23     ` Juergen Gross
2021-10-19 20:48       ` Stefano Stabellini
2021-10-12 13:41 ` [PATCH v6 2/2] tools/xenstore: set open file descriptor limit for xenstored Juergen Gross
2021-10-12 13:56   ` Ian Jackson
2021-10-18 13:18 ` [PATCH v6 0/2] tools/xenstore: set resource limits of xenstored Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.