syscallls/sched_setscheduler: add sched_setscheduler03
PATCH 1/1 syscallls/sched_setscheduler: add sched_setscheduler03
----- Original Message -----
Review
Quoted 9 lines
From: "Chunyu Hu" <[email protected]>
To: [email protected]
Sent: Thursday, 14 July, 2016 11:25:29 AM
Subject: [LTP] [PATCH] syscallls/sched_setscheduler: addsched_setscheduler03(quoted diff) +5
-0+#define _GNU_SOURCE +#include <stdio.h> +#include <errno.h> +#include <sched.h> +#include <linux/capability.h>
Hi,
from doc/style-guide.txt:
"Don't use +linux/+ headers if at all possible."
It does not compile on RHEL5.6 at the moment:sched_setscheduler03.c: In function ‘setup’:
sched_setscheduler03.c:175: error: ‘_LINUX_CAPABILITY_VERSION_1’ undeclared (first use in this function) sched_setscheduler03.c:175: error: (Each undeclared identifier is reported only once sched_setscheduler03.c:175: error: for each function it appears in.)
make: *** [sched_setscheduler03] Error 1
Are capabilities needed only to clear CAP_SYS_NICE? Couldn't we just fork and seteuid to "nobody"?
Review
Quoted 11 lines
(quoted diff) +11
-0+#include <sys/time.h> +#include <sys/resource.h> + +#include "tst_test.h" +#include "linux_syscall_numbers.h" + +#define RLIMIT_NICE_NORMAL 20 + +#ifndef SCHED_IDLE +#define SCHED_IDLE 5 +#endif
This ifdef should rather disable testcase that needs it. Otherwise you'll get failure on older kernels:
sched_setscheduler03.c:116: FAIL: unexpected error - 22 : Invalid argument - expected 0
# uname -r 2.6.18-238.el5
Quoted 41 lines
(quoted diff) +41
-0+ +/* cap_user_header_t is a pointer to __user_cap_header_struct */ +static struct __user_cap_header_struct cap_header; + +/* + * cap_user_data_t is a pointer to to __user_cap_data_struct. + * there are three versions of header, on v2 and v3, the calls + * need two data struct, here we use only v1 is enough, but + * leave it as a two item array for deprecate consideration. + */ +static struct __user_cap_data_struct cap_data[2]; + +static struct rlimit limit; +static pid_t zero_pid; +static struct sched_param param[1] = {{0}}; + +struct test_case_t { + pid_t *pid; + int policy; + struct sched_param *sched_param; + int error; +}; + +struct test_case_t cases[] = { + { + .pid = &zero_pid, + .policy = SCHED_OTHER, + .sched_param = ¶m[0] + }, + { + .pid = &zero_pid, + .policy = SCHED_BATCH, + .sched_param = ¶m[0] + }, + { + .pid = &zero_pid, + .policy = SCHED_IDLE, + .sched_param = ¶m[0] + } +}; +
<snip>
Review
Quoted 4 lines
(quoted diff) +4
-0+ +static void verify_fn(unsigned int i) +{ +
Here could be some message saying what test is going to run.
Review
Quoted 4 lines
(quoted diff) +4
-0+ TEST(sched_setscheduler(*cases[i].pid, cases[i].policy, + cases[i].sched_param)); + if (TEST_RETURN) + tst_res(TFAIL, "unexpected error - %d : %s - "
TFAIL | TTERRNO
Review
Quoted 6 lines
(quoted diff) +6
-0+ "expected %d", TEST_ERRNO, + strerror(TEST_ERRNO), cases[i].error); + else + tst_res(TPASS, "Succeed - %d : %s - " + "expected %d", TEST_ERRNO, + strerror(TEST_ERRNO), cases[i].error);
No need to print errno if call succeeded and that was expected.
Quoted 41 lines
(quoted diff) +41
-0+} + +static void setup(void) +{ + + cap_header.version = _LINUX_CAPABILITY_VERSION_1; + + l_rlimit_show(RLIMIT_NICE, &limit); + + /* + * nice rlimit ranges from 1 to 40, mapping to real nice + * value from 19 to -20. We set it to 19, as the default priority + * of process with fair policy is 120, which will be translated + * into nice 20, we make this RLIMIT_NICE smaller than that, to + * verify the can_nice usage issue. + */ + limit.rlim_cur = (RLIMIT_NICE_NORMAL - 1); + limit.rlim_max = (RLIMIT_NICE_NORMAL - 1); + + l_rlimit_setup(RLIMIT_NICE, &limit); + + tst_res(TINFO, "Setting init sched policy to SCHED_OTHER"); + if (sched_setscheduler(0, SCHED_OTHER, ¶m[0]) != 0) + tst_brk(TBROK | TERRNO, + "ERROR sched_setscheduler: (0, SCHED_OTHER, param)"); + + if (sched_getscheduler(0) != SCHED_OTHER) + tst_brk(TBROK | TERRNO, "ERROR sched_tetscheduler"); + + l_cap_show(&cap_header, &cap_data[0]); + + cap_data[0].effective &= (~(1 << CAP_SYS_NICE)); + cap_data[0].permitted &= (~(1 << CAP_SYS_NICE)); + cap_data[0].inheritable = 1; + + l_cap_setup(&cap_header, &cap_data[0]); +} + +static void cleanup(void) +{ + tst_res(TINFO, "cleanup() executed by pid %i", getpid());
Review
Quoted 10 lines
(quoted diff) +10
-0+} + +static void do_test(unsigned int i) +{ + verify_fn(i); +} + +static struct tst_test test = { + .tid = "sched_setscheduler03", + .tcnt = 3,
ARRAY_SIZE(cases)
Also needs_root seems to be missing here.
Regards,
Jan
Quoted 13 lines
(quoted diff) +6
-1+ .test = do_test, + .setup = setup, + .cleanup = cleanup, + .forks_child = 1, +}; + --1.8.3.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
PATCH 1/1 syscallls/sched_setscheduler: add sched_setscheduler03
----- Original Message -----
Quoted 23 lines
From: "Jan Stancek" <[email protected]>
To: "Chunyu Hu" <[email protected]>
Cc: [email protected]
Sent: Friday, July 15, 2016 8:04:58 PM
Subject: Re: [LTP] [PATCH] syscallls/sched_setscheduler: addsched_setscheduler03----- Original Message -----
Review
Quoted 10 lines
From: "Chunyu Hu" <[email protected]>
To: [email protected]
Sent: Thursday, 14 July, 2016 11:25:29 AM
Subject: [LTP] [PATCH] syscallls/sched_setscheduler: addsched_setscheduler03(quoted diff) +5
-0+#define _GNU_SOURCE +#include <stdio.h> +#include <errno.h> +#include <sched.h> +#include <linux/capability.h>Hi,
Quoted 2 lines
from
doc/style-guide.txt:
"Don't use +linux/+ headers if at all possible."
Ah, sure. thanks for this guide info..
Quoted 10 lines
It does not compile on RHEL5.6 at the moment:
sched_setscheduler03.c: In function ‘setup’:
sched_setscheduler03.c:175: error: ‘_LINUX_CAPABILITY_VERSION_1’ undeclared (first use in this function)
sched_setscheduler03.c:175: error: (Each undeclared identifier is reported only once
sched_setscheduler03.c:175: error: for each function it appears in.)
make: *** [sched_setscheduler03] Error 1Are capabilities needed only to clear
CAP_SYS_NICE?
Actually yes.
Quoted 1 line
Couldn't we just fork and seteuid to "nobody"?
Tried with method that fork and switch to nobody instead of calling the cap* syscall. it looks good.
2.6.18-238.el5
[root@intel-mahobay-02 sched_setscheduler]# ./sched_setscheduler03 tst_test.c:701: INFO: Timeout per run is 300s sched_setscheduler03.c:79: INFO: rlimit rlim_cur=0 sched_setscheduler03.c:81: INFO: rlimit rlim_max=0 sched_setscheduler03.c:89: INFO: Setting rlim_cur to 19 sched_setscheduler03.c:91: INFO: Setting rlim_max to 19 sched_setscheduler03.c:79: INFO: rlimit rlim_cur=19 sched_setscheduler03.c:81: INFO: rlimit rlim_max=19 sched_setscheduler03.c:146: INFO: Setting init sched policy to SCHED_OTHER sched_setscheduler03.c:154: INFO: Setting euid to nobody to drop privilege sched_setscheduler03.c:172: INFO: forked pid is 11488 sched_setscheduler03.c:113: INFO: Verifying case[1]: policy = 0, priority = 0 sched_setscheduler03.c:122: PASS: case[1] succeeded sched_setscheduler03.c:172: INFO: forked pid is 11489 sched_setscheduler03.c:113: INFO: Verifying case[2]: policy = 3, priority = 0 sched_setscheduler03.c:122: PASS: case[2] succeeded sched_setscheduler03.c:164: INFO: cleanup() executed by pid 11487
Summary:
passed 2 failed 0 skipped 0
warnings 0
Quoted 8 lines
Quoted 8 lines
(quoted diff) +8
-0+#include <sys/time.h> +#include <sys/resource.h> + +#include "tst_test.h" +#include "linux_syscall_numbers.h" + +#define RLIMIT_NICE_NORMAL 20 +
Quoted 8 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+#ifndef SCHED_IDLE +#define SCHED_IDLE 5 +#endifThis ifdef should rather disable testcase that needs it. Otherwise you'll get failure on older kernels:
sched_setscheduler03.c:116: FAIL: unexpected error - 22 : Invalid argument - expected 0
My less consideration. I modify it to
#ifdef SCHED_IDLE
{
...
}
#endif
And it can be skipped if not defined.
Quoted 45 lines
# uname -r 2.6.18-238.el5Quoted 41 lines
(quoted diff) +41
-0+ +/* cap_user_header_t is a pointer to __user_cap_header_struct */ +static struct __user_cap_header_struct cap_header; + +/* + * cap_user_data_t is a pointer to to __user_cap_data_struct. + * there are three versions of header, on v2 and v3, the calls + * need two data struct, here we use only v1 is enough, but + * leave it as a two item array for deprecate consideration. + */ +static struct __user_cap_data_struct cap_data[2]; + +static struct rlimit limit; +static pid_t zero_pid; +static struct sched_param param[1] = {{0}}; + +struct test_case_t { + pid_t *pid; + int policy; + struct sched_param *sched_param; + int error; +}; + +struct test_case_t cases[] = { + { + .pid = &zero_pid, + .policy = SCHED_OTHER, + .sched_param = ¶m[0] + }, + { + .pid = &zero_pid, + .policy = SCHED_BATCH, + .sched_param = ¶m[0] + }, + { + .pid = &zero_pid, + .policy = SCHED_IDLE, + .sched_param = ¶m[0] + } +}; +<snip>
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ +static void verify_fn(unsigned int i) +{ +Here could be some message saying what test is going to run.
OK. add it in v2.
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +4
-0+ TEST(sched_setscheduler(*cases[i].pid, cases[i].policy, + cases[i].sched_param)); + if (TEST_RETURN) + tst_res(TFAIL, "unexpected error - %d : %s - "TFAIL | TTERRNO
Quoted 8 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ "expected %d", TEST_ERRNO, + strerror(TEST_ERRNO), cases[i].error); + else + tst_res(TPASS, "Succeed - %d : %s - " + "expected %d", TEST_ERRNO, + strerror(TEST_ERRNO), cases[i].error);No need to print errno if call succeeded and that was expected.
Remove this in V2. Thanks for suggestion.
Quoted 54 lines
Quoted 41 lines
(quoted diff) +41
-0+} + +static void setup(void) +{ + + cap_header.version = _LINUX_CAPABILITY_VERSION_1; + + l_rlimit_show(RLIMIT_NICE, &limit); + + /* + * nice rlimit ranges from 1 to 40, mapping to real nice + * value from 19 to -20. We set it to 19, as the default priority + * of process with fair policy is 120, which will be translated + * into nice 20, we make this RLIMIT_NICE smaller than that, to + * verify the can_nice usage issue. + */ + limit.rlim_cur = (RLIMIT_NICE_NORMAL - 1); + limit.rlim_max = (RLIMIT_NICE_NORMAL - 1); + + l_rlimit_setup(RLIMIT_NICE, &limit); + + tst_res(TINFO, "Setting init sched policy to SCHED_OTHER"); + if (sched_setscheduler(0, SCHED_OTHER, ¶m[0]) != 0) + tst_brk(TBROK | TERRNO, + "ERROR sched_setscheduler: (0, SCHED_OTHER, param)"); + + if (sched_getscheduler(0) != SCHED_OTHER) + tst_brk(TBROK | TERRNO, "ERROR sched_tetscheduler"); + + l_cap_show(&cap_header, &cap_data[0]); + + cap_data[0].effective &= (~(1 << CAP_SYS_NICE)); + cap_data[0].permitted &= (~(1 << CAP_SYS_NICE)); + cap_data[0].inheritable = 1; + + l_cap_setup(&cap_header, &cap_data[0]); +} + +static void cleanup(void) +{ + tst_res(TINFO, "cleanup() executed by pid %i", getpid());Review
Quoted 10 lines
(quoted diff) +10
-0+} + +static void do_test(unsigned int i) +{ + verify_fn(i); +} + +static struct tst_test test = { + .tid = "sched_setscheduler03", + .tcnt = 3,
ARRAY_SIZE(cases)
Quoted 1 line
Also
needs_rootseems to be missing here.
Modify this in v2, thanks.
Quoted 17 lines
Regards,
JanQuoted 13 lines
(quoted diff) +6
-1+ .test = do_test, + .setup = setup, + .cleanup = cleanup, + .forks_child = 1, +}; + --1.8.3.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Regards, Chunyu Hu