| Message ID | 20211222135234.30025-1-chrubis@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] lib: Do not fail a test if oom score cannot be adjusted. | expand |
Hi Cyril, all, > From: Petr Vorel <pvorel@suse.cz> > Setting value < 0 in /proc/*/oom_score_adj requires CAP_SYS_RESOURCE or > CAP_SYS_ADMIN. However setting the library process score is a best > effort operation, so let's skip it silently when the user is not > privileged to do so. +1 LGTM, thanks for this version, I'm for merging it. Also tested locally and on CI, working as expected. https://github.com/pevik/ltp/runs/4607602484?check_suite_focus=true Kind regards, Petr
Hi Petr, All > Hi Cyril, all, > >> From: Petr Vorel<pvorel@suse.cz> > >> Setting value< 0 in /proc/*/oom_score_adj requires CAP_SYS_RESOURCE or >> CAP_SYS_ADMIN. Here will mislead user. Since the default oom_score_adj value is 0, so we can not set a value < 0. The value of /proc/<pid>/oom_score_adj may be reduced no lower than the last value set by a CAP_SYS_RESOURCE process. To reduce the value any lower requires CAP_SYS_RESOURCE. Also looks man 7 capabilities, CAP_SYS_ADMIN doesn't have this cap and I have do a test to verify this #include <stdio.h> #include <unistd.h> #include <sys/types.h> #include <errno.h> #include "tst_test.h" #include "tst_capability.h" static void run(void) { FILE *f; f = fopen("/proc/self/oom_score_adj", "w"); if (!f) tst_res(TFAIL, "non-exist"); if (fprintf(f, "%d", -200) <= 0) tst_res(TFAIL, "write failed"); if (fclose(f)) tst_res(TFAIL, "close %d",errno); tst_res(TPASS, "write pass"); } static struct tst_test test = { .test_all = run, .caps = (struct tst_cap []) { TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN), TST_CAP(TST_CAP_DROP, CAP_SYS_RESOURCE), {} }, }; It still fails if we only have CAP_SYS_ADMIN. Best Regards Yang Xu However setting the library process score is a best >> effort operation, so let's skip it silently when the user is not >> privileged to do so. > +1 > > LGTM, thanks for this version, I'm for merging it. > > Also tested locally and on CI, working as expected. > https://github.com/pevik/ltp/runs/4607602484?check_suite_focus=true > > Kind regards, > Petr
Hi All, On Thu, Dec 23, 2021 at 9:17 AM xuyang2018.jy@fujitsu.com <xuyang2018.jy@fujitsu.com> wrote: > >> Setting value< 0 in /proc/*/oom_score_adj requires CAP_SYS_RESOURCE or > >> CAP_SYS_ADMIN. > Here will mislead user. > Since the default oom_score_adj value is 0, so we can not set a value < 0. > > The value of /proc/<pid>/oom_score_adj may be reduced no lower than the > last value set by a CAP_SYS_RESOURCE process. To reduce the value any > lower requires CAP_SYS_RESOURCE. Good to know this, thank you Xu! From the man page, CAP_SYS_RESOURCE gives: " CAP_SYS_RESOURCE ... * set /proc/[pid]/oom_score_adj to a value lower than the value last set by a process with CAP_SYS_RESOURCE. " I will send a V3 base on Petr and Cyril's work to speed up merging this fix.
diff --git a/include/tst_memutils.h b/include/tst_memutils.h index 68a6e3771..855c6f289 100644 --- a/include/tst_memutils.h +++ b/include/tst_memutils.h @@ -28,13 +28,17 @@ long long tst_available_mem(void); /* * Enable OOM protection to prevent process($PID) being killed by OOM Killer. * echo -1000 >/proc/$PID/oom_score_adj + * * If the pid is 0 which means it will set on current(self) process. * + * Unless the process has CAP_SYS_RESOURCE or CAP_SYS_ADMIN this call will be + * no-op because setting adj value < 0 requires it. + * * Note: * This exported tst_enable_oom_protection function can be used at anywhere * you want to protect, but please remember that if you do enable protection * on a process($PID) that all the children will inherit its score and be - * ignored by OOM Killer as well. So that's why tst_disable_oom_protection + * ignored by OOM Killer as well. So that's why tst_disable_oom_protection() * to be used in combination. */ void tst_enable_oom_protection(pid_t pid); diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c index 4dea30330..4a47bbb33 100644 --- a/lib/tst_memutils.c +++ b/lib/tst_memutils.c @@ -11,6 +11,8 @@ #define TST_NO_DEFAULT_MAIN #include "tst_test.h" +#include "tst_capability.h" +#include "lapi/syscalls.h" #define BLOCKSIZE (16 * 1024 * 1024) @@ -93,6 +95,42 @@ long long tst_available_mem(void) return mem_available; } +static int has_caps(void) +{ + struct tst_cap_user_header hdr = { + .version = 0x20080522, + .pid = tst_syscall(__NR_gettid), + }; + + struct tst_cap_user_data caps[2]; + + if (tst_capget(&hdr, caps)) + tst_brk(TBROK | TERRNO, "tst_capget()"); + + if ((caps[0].effective & (1U << CAP_SYS_ADMIN)) || + (caps[0].effective & (1U << CAP_SYS_RESOURCE))) + return 1; + + return 0; +} + +static int write_score(const char *path, int score) +{ + FILE *f; + + f = fopen(path, "w"); + if (!f) + return 1; + + if (fprintf(f, "%d", score) <= 0) + return 1; + + if (fclose(f)) + return 1; + + return 0; +} + static void set_oom_score_adj(pid_t pid, int value) { int val; @@ -111,17 +149,18 @@ static void set_oom_score_adj(pid_t pid, int value) tst_brk(TBROK, "%s does not exist, please check if PID is valid", score_path); } - FILE_PRINTF(score_path, "%d", value); + if (write_score(score_path, value)) { + if (!has_caps()) + return; + + tst_res(TWARN, "Can't adjust score, even with capabilities!?"); + return; + } + FILE_SCANF(score_path, "%d", &val); - if (val != value) { - if (value < 0) { - tst_res(TWARN, "'%s' cannot be set to %i, are you root?", - score_path, value); - return; - } + if (val != value) tst_brk(TBROK, "oom_score_adj = %d, but expect %d.", val, value); - } } void tst_enable_oom_protection(pid_t pid)