* [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted.
@ 2021-12-22 13:52 Cyril Hrubis
2021-12-22 14:03 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Cyril Hrubis @ 2021-12-22 13:52 UTC (permalink / raw)
To: ltp
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.
Fixes: 8a0827766d ("lib: add functions to adjust oom score")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
include/tst_memutils.h | 6 ++++-
lib/tst_memutils.c | 55 ++++++++++++++++++++++++++++++++++++------
2 files changed, 52 insertions(+), 9 deletions(-)
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)
--
2.32.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-22 13:52 [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted Cyril Hrubis
@ 2021-12-22 14:03 ` Petr Vorel
2021-12-23 1:17 ` xuyang2018.jy
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-12-22 14:03 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-22 14:03 ` Petr Vorel
@ 2021-12-23 1:17 ` xuyang2018.jy
2021-12-23 2:13 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: xuyang2018.jy @ 2021-12-23 1:17 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-23 1:17 ` xuyang2018.jy
@ 2021-12-23 2:13 ` Li Wang
2021-12-23 2:48 ` [LTP] [PATCH v3] " Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2021-12-23 2:13 UTC (permalink / raw)
To: xuyang2018.jy; +Cc: ltp
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.
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-23 2:13 ` Li Wang
@ 2021-12-23 2:48 ` Li Wang
2021-12-23 3:03 ` xuyang2018.jy
2021-12-23 7:57 ` Petr Vorel
0 siblings, 2 replies; 8+ messages in thread
From: Li Wang @ 2021-12-23 2:48 UTC (permalink / raw)
To: ltp
From: Petr Vorel <pvorel@suse.cz>
Setting value < 0 in /proc/<pid>/oom_score_adj requires CAP_SYS_RESOURCE.
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.
Fixes: 8a0827766d ("lib: add functions to adjust oom score")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Li Wang <liwang@redhat.com>
---
Notes:
v2 --> v3
* drop the useless CAP_SYS_ADMIN
* add some new commments
CI job: https://github.com/wangli5665/ltp/actions/runs/1613852940
include/tst_memutils.h | 10 +++++++-
lib/tst_memutils.c | 54 +++++++++++++++++++++++++++++++++++-------
2 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/include/tst_memutils.h b/include/tst_memutils.h
index 68a6e3771..45dec55bc 100644
--- a/include/tst_memutils.h
+++ b/include/tst_memutils.h
@@ -28,13 +28,21 @@ 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 this call will be no-op because
+ * setting adj value < 0 requires it.
+ *
+ * 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.
+ *
* 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..4a4974761 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,41 @@ 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_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 +148,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)
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-23 2:48 ` [LTP] [PATCH v3] " Li Wang
@ 2021-12-23 3:03 ` xuyang2018.jy
2021-12-23 7:57 ` Petr Vorel
1 sibling, 0 replies; 8+ messages in thread
From: xuyang2018.jy @ 2021-12-23 3:03 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li
> From: Petr Vorel<pvorel@suse.cz>
>
> Setting value< 0 in/proc/<pid>/oom_score_adj requires CAP_SYS_RESOURCE.
> 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.
>
> Fixes: 8a0827766d ("lib: add functions to adjust oom score")
> Signed-off-by: Petr Vorel<pvorel@suse.cz>
> Signed-off-by: Cyril Hrubis<chrubis@suse.cz>
> Signed-off-by: Li Wang<liwang@redhat.com>
Now, LGTM,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
ps: ubuntun:groxy fails because of other thing..., it is another problem.
Best Regards
Yang Xu
> ---
>
> Notes:
> v2 --> v3
> * drop the useless CAP_SYS_ADMIN
> * add some new commments
> CI job:https://github.com/wangli5665/ltp/actions/runs/1613852940
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-23 2:48 ` [LTP] [PATCH v3] " Li Wang
2021-12-23 3:03 ` xuyang2018.jy
@ 2021-12-23 7:57 ` Petr Vorel
2021-12-23 8:29 ` Petr Vorel
1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-12-23 7:57 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi all,
> Notes:
> v2 --> v3
> * drop the useless CAP_SYS_ADMIN
> * add some new commments
Yes, CAP_SYS_ADMIN was redundant, thus I merged it.
Thanks a lot all.
> CI job: https://github.com/wangli5665/ltp/actions/runs/1613852940
Yes, ubuntu cannot reach repos:
Ign:1 http://security.ubuntu.com/ubuntu groovy-security InRelease
Ign:2 http://archive.ubuntu.com/ubuntu groovy InRelease
Err:3 http://security.ubuntu.com/ubuntu groovy-security Release
404 Not Found [IP: 91.189.91.39 80]
Ign:4 http://archive.ubuntu.com/ubuntu groovy-updates InRelease
Ign:5 http://archive.ubuntu.com/ubuntu groovy-backports InRelease
Err:6 http://archive.ubuntu.com/ubuntu groovy Release
404 Not Found [IP: 91.189.88.142 80]
Err:7 http://archive.ubuntu.com/ubuntu groovy-updates Release
404 Not Found [IP: 91.189.88.142 80]
Err:8 http://archive.ubuntu.com/ubuntu groovy-backports Release
404 Not Found [IP: 91.189.88.142 80]
Reading package lists...
E: The repository 'http://security.ubuntu.com/ubuntu groovy-security Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu groovy Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu groovy-updates Release' does not have a Release file.
E: The repository 'http://archive.ubuntu.com/ubuntu groovy-backports Release' does not have a Release file.
I'm testing upgrade to impish:
https://github.com/pevik/ltp/actions/runs/1614667613
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3] lib: Do not fail a test if oom score cannot be adjusted.
2021-12-23 7:57 ` Petr Vorel
@ 2021-12-23 8:29 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2021-12-23 8:29 UTC (permalink / raw)
To: Li Wang, ltp
Hi all,
> I'm testing upgrade to impish:
> https://github.com/pevik/ltp/actions/runs/1614667613
Hm, impish is also broken. Hope it's just temporary issue.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-23 8:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22 13:52 [LTP] [PATCH v2] lib: Do not fail a test if oom score cannot be adjusted Cyril Hrubis
2021-12-22 14:03 ` Petr Vorel
2021-12-23 1:17 ` xuyang2018.jy
2021-12-23 2:13 ` Li Wang
2021-12-23 2:48 ` [LTP] [PATCH v3] " Li Wang
2021-12-23 3:03 ` xuyang2018.jy
2021-12-23 7:57 ` Petr Vorel
2021-12-23 8:29 ` Petr Vorel
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.