[systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
If 'OOMScoreAdjust' option is ommited from unit file, this patch makes
the executed process's oom_score_adj as default OOM score in case of
explicitly 'DefaultOOMScore' is declared in configuration file.
(i.e. system.conf and user.conf)
If the unit file has 'OOMScoreAdjust' option, set the its oom_score_adj
as declared value as before.
Signed-off-by: Sangjung Woo
---
src/core/main.c |8
src/core/system.conf |1 +
src/core/user.conf |1 +
3 files changed, 10 insertions(+)
diff --git a/src/core/main.c b/src/core/main.c
index 22cec4e..e8bb015 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -90,6 +90,7 @@ static bool arg_confirm_spawn = false;
static bool arg_show_status = true;
static bool arg_switched_root = false;
static char **arg_default_controllers = NULL;
+static char *arg_default_oom_score = NULL;
static char ***arg_join_controllers = NULL;
static ExecOutput arg_default_std_output = EXEC_OUTPUT_JOURNAL;
static ExecOutput arg_default_std_error = EXEC_OUTPUT_INHERIT;
@@ -641,6 +642,7 @@ static int parse_config_file(void) {
{ "Manager", "DefaultStandardOutput", config_parse_output,
0, &arg_default_std_output },
{ "Manager", "DefaultStandardError", config_parse_output,
0, &arg_default_std_error },
{ "Manager", "JoinControllers",
config_parse_join_controllers, 0, &arg_join_controllers },
+{ "Manager", "DefaultOOMScore", config_parse_string,
0, &arg_default_oom_score },
{ "Manager", "RuntimeWatchdogSec",config_parse_sec,
0, &arg_runtime_watchdog},
{ "Manager", "ShutdownWatchdogSec", config_parse_sec,
0, &arg_shutdown_watchdog },
{ "Manager", "CapabilityBoundingSet",
config_parse_bounding_set, 0, &arg_capability_bounding_set_drop },
@@ -1414,6 +1416,12 @@ int main(int argc, char *argv[]) {
if (parse_config_file() < 0)
goto finish;
+if (arg_default_oom_score)
+if (write_string_file("/proc/self/oom_score_adj",
arg_default_oom_score) < 0){
+log_error("Fail to set default oom_score_adj: %s",
arg_default_oom_score);
+goto finish;
+}
+
if (arg_running_as == SYSTEMD_SYSTEM)
if (parse_proc_cmdline() < 0)
goto finish;
diff --git a/src/core/system.conf b/src/core/system.conf
index 508e0f5..7c0e12e 100644
--- a/src/core/system.conf
+++ b/src/core/system.conf
@@ -41,3 +41,4 @@
#DefaultLimitNICE=
#DefaultLimitRTPRIO=
#DefaultLimitRTTIME=
+#DefaultOOMScore=
diff --git a/src/core/user.conf b/src/core/user.conf
index 4252451..f101e99 100644
--- a/src/core/user.conf
+++ b/src/core/user.conf
@@ -15,3 +15,4 @@
#DefaultControllers=cpu
#DefaultStandardOutput=inherit
#DefaultStandardError=inherit
+#DefaultOOMScore=
--
1.7.10.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
Hi,
Should DefaultOOMScore= and DefaultLimit***= be really under [Manager]?
Wouldn't it be better to have an [Exec] section. Or even have [Service],
[Mount],.. sections that can give separate configuration per exec unit.
Other idea would be having both [Manager] and [Service], [Mount], .. where
configuration directives in [Service] overrides [Manager]. This way, one
can specify a top level configuration under [Manager] and overwrite it per
exec unit.
Also, there are many configuration options per exec units that are not in
the system.conf. Are we open taking patches on completing missing
configuration options?
Thanks
On Sat, Apr 27, 2013 at 4:49 PM, Sangjung Woo wrote:
> If 'OOMScoreAdjust' option is ommited from unit file, this patch makes
> the executed process's oom_score_adj as default OOM score in case of
> explicitly 'DefaultOOMScore' is declared in configuration file.
> (i.e. system.conf and user.conf)
>
> If the unit file has 'OOMScoreAdjust' option, set the its oom_score_adj
> as declared value as before.
>
> Signed-off-by: Sangjung Woo
> ---
> src/core/main.c |8
> src/core/system.conf |1 +
> src/core/user.conf |1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/src/core/main.c b/src/core/main.c
> index 22cec4e..e8bb015 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -90,6 +90,7 @@ static bool arg_confirm_spawn = false;
> static bool arg_show_status = true;
> static bool arg_switched_root = false;
> static char **arg_default_controllers = NULL;
> +static char *arg_default_oom_score = NULL;
> static char ***arg_join_controllers = NULL;
> static ExecOutput arg_default_std_output = EXEC_OUTPUT_JOURNAL;
> static ExecOutput arg_default_std_error = EXEC_OUTPUT_INHERIT;
> @@ -641,6 +642,7 @@ static int parse_config_file(void) {
> { "Manager", "DefaultStandardOutput",
> config_parse_output, 0, &arg_default_std_output },
> { "Manager", "DefaultStandardError",
> config_parse_output, 0, &arg_default_std_error },
> { "Manager", "JoinControllers",
> config_parse_join_controllers, 0, &arg_join_controllers },
> +{ "Manager", "DefaultOOMScore",
> config_parse_string, 0, &arg_default_oom_score },
> { "Manager", "RuntimeWatchdogSec",config_parse_sec,
>0, &arg_runtime_watchdog},
> { "Manager", "ShutdownWatchdogSec", config_parse_sec,
>0, &arg_shutdown_watchdog },
> { "Manager", "CapabilityBoundingSet",
> config_parse_bounding_set, 0, &arg_capability_bounding_set_drop },
> @@ -1414,6 +1416,12 @@ int main(int argc, char *argv[]) {
> if (parse_config_file() < 0)
> goto finish;
>
> +if (arg_default_oom_score)
> +if (write_string_file("/proc/self/oom_score_adj",
> arg_default_oom_score) < 0){
> +log_error("Fail to set default oom_score_adj:
> %s", arg_default_oom_score);
> +goto finish;
> +}
> +
> if (arg_running_as == SYSTEMD_SYSTEM)
> if (parse_proc_cmdline() < 0)
> goto finish;
> diff --git a/src/core/system.conf b/src/core/system.conf
> index 508e0f5..7c0e12e 100644
> --- a/src/core/system.conf
> +++ b/src/core/system.conf
> @@ -41,3 +41,4 @@
> #DefaultLimitNICE=
> #DefaultLimitRTPRIO=
> #DefaultLimitRTTIME=
> +#DefaultOOMScore=
> diff --git a/src/core/user.conf b/src/core/user.conf
> index 4252451..f101e99 100644
> --- a/src/core/user.conf
> +++ b/src/core/user.conf
> @@ -15,3 +15,4 @@
> #DefaultControllers=cpu
> #DefaultStandardOutput=inherit
> #DefaultStandardError=inherit
> +#DefaultOOMScore=
> --
> 1.7.10.4
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
On Sat, 27.04.13 07:49, Sangjung Woo (again4...@gmail.com) wrote: > If 'OOMScoreAdjust' option is ommited from unit file, this patch makes > the executed process's oom_score_adj as default OOM score in case of > explicitly 'DefaultOOMScore' is declared in configuration file. > (i.e. system.conf and user.conf) This definitely sounds like a useful addition! > If the unit file has 'OOMScoreAdjust' option, set the its oom_score_adj > as declared value as before. I'd prefer to call this option OOMScoreAdjust= too, i.e. not with the "Default" prefix, and use the same nomenclature as the per-service option. Generally, the entries in system.conf that are not prefixed with "Default" apply to PID 1 itself, while those prefixed with "Default" are defaults for units, and are not applied to PID 1. Since this option would apply to PID 1 it hence should not carry the "Default" prefix. > +static char *arg_default_oom_score = NULL; This should probably be an "int", i.e. the same way as we store this in struct ExecContext, see src/core/execute.h for details. Also, since this value can validly be negative, postive and 0, we'd need an additional bool to indicate whether the value is set at all. We should also validate all settings we read immediately and refuse invalid settings. See config_parse_exec_oom_score_adjust() in src/core/load-fragment.c for inspiration. Also, this patch would need a matching man page update. Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
On Sun, 28.04.13 20:37, Umut Tezduyar (u...@tezduyar.com) wrote: > Hi, > > Should DefaultOOMScore= and DefaultLimit***= be really under [Manager]? > Wouldn't it be better to have an [Exec] section. Or even have [Service], > [Mount],.. sections that can give separate configuration per exec > unit. Hmm, the fields without "Default" prefix are applied to PID1 itself, too, hence they definitely belong into [Manager] I'd say. For the ones prefix with Default, we could probably move them to a new section [Units] or so. But we'd probably have to do that in a compatible way, so this doesn't break existing systems. And that's not too nice. Not sure whether it is worth that effort? > Also, there are many configuration options per exec units that are not in > the system.conf. Are we open taking patches on completing missing > configuration options? Actually, the last time I checked them there were only very few where a default in system.conf really makes sense I think. Which ones do you have in mind? I mean, we can surely add more, but I let's always come up with a useful usecase first, please... Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
On Fri, 03.05.13 16:49, Lennart Poettering (lenn...@poettering.net) wrote: > > If the unit file has 'OOMScoreAdjust' option, set the its oom_score_adj > > as declared value as before. > > I'd prefer to call this option OOMScoreAdjust= too, i.e. not with the > "Default" prefix, and use the same nomenclature as the per-service > option. > > Generally, the entries in system.conf that are not prefixed with > "Default" apply to PID 1 itself, while those prefixed with "Default" are > defaults for units, and are not applied to PID 1. Since this option > would apply to PID 1 it hence should not carry the "Default" prefix. Hmm, actually, thinking about this a bit more, I wonder how much sense it actually makes to alter the OOM score adjust value for PID 1. I mean, iirc PID 1 is excluded from the OOM killer anyway, right? Maybe this should be DefaultOOMScoreAdjust= and be explicitly copied into each unit, the same way as we handle this for DefaultLimitCPU= → LimitCPU=. Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
On Fri, May 3, 2013 at 4:58 PM, Lennart Poettering wrote: > On Sun, 28.04.13 20:37, Umut Tezduyar (u...@tezduyar.com) wrote: > > > Hi, > > > > Should DefaultOOMScore= and DefaultLimit***= be really under [Manager]? > > Wouldn't it be better to have an [Exec] section. Or even have [Service], > > [Mount],.. sections that can give separate configuration per exec > > unit. > > Hmm, the fields without "Default" prefix are applied to PID1 itself, > too, hence they definitely belong into [Manager] I'd say. > > For the ones prefix with Default, we could probably move them to a new > section [Units] or so. But we'd probably have to do that in a compatible > way, so this doesn't break existing systems. And that's not too > nice. Not sure whether it is worth that effort? > > > Also, there are many configuration options per exec units that are not in > > the system.conf. Are we open taking patches on completing missing > > configuration options? > > Actually, the last time I checked them there were only very few where a > default in system.conf really makes sense I think. Which ones do you > have in mind? > Umut: I use ControlGroupPersistent= on one shot unit files to measure their cpuacct. I have a generator that adds ControlGroupPersistent dropins for oneshot service files when I want to measure the cpuacct. Maybe it would be nice to have a DefaultControlGroupPersistent=yes in .conf. > > I mean, we can surely add more, but I let's always come up with a useful > usecase first, please... > > Lennart > > -- > Lennart Poettering - Red Hat, Inc. > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/1] RFC: Set the Default OOM Score from configuration file
On Sun, 05.05.13 18:08, Umut Tezduyar (u...@tezduyar.com) wrote: > On Fri, May 3, 2013 at 4:58 PM, Lennart Poettering > wrote: > > > On Sun, 28.04.13 20:37, Umut Tezduyar (u...@tezduyar.com) wrote: > > > > > Hi, > > > > > > Should DefaultOOMScore= and DefaultLimit***= be really under [Manager]? > > > Wouldn't it be better to have an [Exec] section. Or even have [Service], > > > [Mount],.. sections that can give separate configuration per exec > > > unit. > > > > Hmm, the fields without "Default" prefix are applied to PID1 itself, > > too, hence they definitely belong into [Manager] I'd say. > > > > For the ones prefix with Default, we could probably move them to a new > > section [Units] or so. But we'd probably have to do that in a compatible > > way, so this doesn't break existing systems. And that's not too > > nice. Not sure whether it is worth that effort? > > > > > Also, there are many configuration options per exec units that are not in > > > the system.conf. Are we open taking patches on completing missing > > > configuration options? > > > > Actually, the last time I checked them there were only very few where a > > default in system.conf really makes sense I think. Which ones do you > > have in mind? > > > > Umut: I use ControlGroupPersistent= on one shot unit files to measure their > cpuacct. I have a generator that adds ControlGroupPersistent dropins for > oneshot service files when I want to measure the cpuacct. Maybe it would be > nice to have a DefaultControlGroupPersistent=yes in .conf. I'd be very careful with that... This would mean that all cgroups created by instantiated services would stay around, for example those for each ssh connection. That would be very undesirable, as for each incoming connection you'd add more left-over cgroups. Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
