[LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[Zhao Gongyi via ltp]

Add test verifies that the low nice thread executes more
time than the high nice thread since the two thread binded
on the same cpu.

Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
v3->v4: Replace getting exec time from sum_exec_runtime with pthread_getcpuclockid().

 runtest/syscalls                          |   1 +
 testcases/kernel/syscalls/nice/.gitignore |   1 +
 testcases/kernel/syscalls/nice/Makefile   |   2 +
 testcases/kernel/syscalls/nice/nice05.c   | 188 ++++++++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 testcases/kernel/syscalls/nice/nice05.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 9d58e0aa1..98fcbbe1e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -903,6 +903,7 @@ nice01 nice01
 nice02 nice02
 nice03 nice03
 nice04 nice04
+nice05 nice05

 open01 open01
 open01A symlink01 -T open01
diff --git a/testcases/kernel/syscalls/nice/.gitignore b/testcases/kernel/syscalls/nice/.gitignore
index 9d7a1bb43..58d64779e 100644
--- a/testcases/kernel/syscalls/nice/.gitignore
+++ b/testcases/kernel/syscalls/nice/.gitignore
@@ -2,3 +2,4 @@
 /nice02
 /nice03
 /nice04
+/nice05
diff --git a/testcases/kernel/syscalls/nice/Makefile b/testcases/kernel/syscalls/nice/Makefile
index 044619fb8..02e78a295 100644
--- a/testcases/kernel/syscalls/nice/Makefile
+++ b/testcases/kernel/syscalls/nice/Makefile
@@ -3,6 +3,8 @@

 top_srcdir		?= ../../../..

+nice05: CFLAGS += -pthread
+
 include $(top_srcdir)/include/mk/testcases.mk

 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/nice/nice05.c b/testcases/kernel/syscalls/nice/nice05.c
new file mode 100644
index 000000000..8ef33f932
--- /dev/null
+++ b/testcases/kernel/syscalls/nice/nice05.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright(c) 2022 Huawei Technologies Co., Ltd
+ * Author: Li Mengfei <limengfei4@huawei.com>
+ *         Zhao Gongyi <zhaogongyi@huawei.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * 1. Create a high nice thread and a low nice thread, the main
+ *    thread wake them at the same time
+ * 2. Both threads run on the same CPU
+ * 3. Verify that the low nice thread executes more time than
+ *    the high nice thread
+ */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sys/types.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "tst_safe_pthread.h"
+#include "lapi/syscalls.h"
+
+#define SEC2NS(sec)  ((sec) * 1000000000LL)
+
+static pthread_barrier_t barrier;
+static int some_cpu;
+static cpu_set_t *set;
+
+static void set_nice(int nice_inc)
+{
+	int orig_nice;
+
+	orig_nice = SAFE_GETPRIORITY(PRIO_PROCESS, 0);
+
+	TEST(nice(nice_inc));
+
+	if (TST_RET != (orig_nice + nice_inc)) {
+		tst_brk(TBROK | TTERRNO, "nice(%d) returned %li, expected %i",
+			nice_inc, TST_RET, orig_nice + nice_inc);
+	}
+
+	if (TST_ERR)
+		tst_brk(TBROK | TTERRNO, "nice(%d) failed", nice_inc);
+}
+
+static void *nice_low_thread(void *arg)
+{
+	volatile int number = 0;
+
+	set_nice((intptr_t)arg);
+	TEST(pthread_barrier_wait(&barrier));
+	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
+		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
+
+	while (1)
+		number++;
+
+	return NULL;
+}
+
+static void *nice_high_thread(void *arg)
+{
+	volatile int number = 0;
+
+	set_nice((intptr_t)arg);
+	TEST(pthread_barrier_wait(&barrier));
+	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
+		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
+
+	while (1)
+		number++;
+
+	return NULL;
+}
+
+static void setup(void)
+{
+	size_t size;
+	size_t i;
+	int nrcpus = 1024;
+
+	set = CPU_ALLOC(nrcpus);
+	if (!set)
+		tst_brk(TBROK | TERRNO, "CPU_ALLOC()");
+
+	size = CPU_ALLOC_SIZE(nrcpus);
+	CPU_ZERO_S(size, set);
+	if (sched_getaffinity(0, size, set) < 0)
+		tst_brk(TBROK | TERRNO, "sched_getaffinity()");
+
+	for (i = 0; i < size * 8; i++)
+		if (CPU_ISSET_S(i, size, set))
+			some_cpu = i;
+
+	CPU_ZERO_S(size, set);
+	CPU_SET_S(some_cpu, size, set);
+	if (sched_setaffinity(0, size, set) < 0)
+		tst_brk(TBROK | TERRNO, "sched_setaffinity()");
+}
+
+static void cleanup(void)
+{
+	if (set)
+		CPU_FREE(set);
+}
+
+static void verify_nice(void)
+{
+	intptr_t nice_inc_high = -1;
+	intptr_t nice_inc_low = -2;
+	clockid_t nice_low_clockid, nice_high_clockid;
+	struct timespec nice_high_ts, nice_low_ts;
+	long long delta;
+	pid_t pid;
+	pthread_t thread[2];
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		TEST(pthread_barrier_init(&barrier, NULL, 3));
+		if (TST_RET != 0) {
+			tst_brk(TBROK | TTERRNO,
+					"pthread_barrier_init() failed");
+		}
+
+		SAFE_PTHREAD_CREATE(&thread[0], NULL, nice_high_thread,
+				(void *)nice_inc_high);
+		SAFE_PTHREAD_CREATE(&thread[1], NULL, nice_low_thread,
+				(void *)nice_inc_low);
+
+		TEST(pthread_barrier_wait(&barrier));
+		if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD) {
+			tst_brk(TBROK | TTERRNO,
+					"pthread_barrier_wait() failed");
+		}
+
+		sleep(tst_remaining_runtime());
+
+		if (pthread_getcpuclockid(thread[1], &nice_low_clockid) != 0) {
+			perror("clock_getcpuclockid");
+			tst_brk(TBROK | TERRNO,
+					"clock_getcpuclockid() failed");
+		}
+		if (pthread_getcpuclockid(thread[0], &nice_high_clockid) != 0) {
+			perror("clock_getcpuclockid");
+			tst_brk(TBROK | TERRNO,
+					"clock_getcpuclockid() failed");
+		}
+
+		if (clock_gettime(nice_low_clockid, &nice_low_ts) == -1) {
+			tst_brk(TBROK | TERRNO,
+					"clock_getcpuclockid() failed");
+		}
+
+		if (clock_gettime(nice_high_clockid, &nice_high_ts) == -1) {
+			tst_brk(TBROK | TERRNO,
+					"clock_getcpuclockid() failed");
+		}
+
+		tst_res(TINFO, "Nice low thread CPU time: %ld.%09ld s",
+			nice_low_ts.tv_sec, nice_low_ts.tv_nsec);
+		tst_res(TINFO, "Nice high thread CPU time: %ld.%09ld s",
+			nice_high_ts.tv_sec, nice_high_ts.tv_nsec);
+
+		delta = SEC2NS(nice_low_ts.tv_sec - nice_high_ts.tv_sec) +
+			(nice_low_ts.tv_nsec - nice_high_ts.tv_nsec);
+		if (delta < 0) {
+			tst_res(TFAIL, "executes less cycles than "
+				"the high nice thread, delta: %lld ns", delta);
+		} else {
+			tst_res(TPASS, "executes more cycles than "
+				"the high nice thread, delta: %lld ns", delta);
+		}
+		return;
+	}
+	SAFE_WAIT(NULL);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_nice,
+	.needs_root = 1,
+	.forks_child = 1,
+	.max_runtime = 3,
+};
--
2.17.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[Cyril Hrubis]

Hi!
> Add test verifies that the low nice thread executes more
> time than the high nice thread since the two thread binded
> on the same cpu.

Looks very good now, there are few very minor points see below.

> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> ---
> v3->v4: Replace getting exec time from sum_exec_runtime with pthread_getcpuclockid().
> 
>  runtest/syscalls                          |   1 +
>  testcases/kernel/syscalls/nice/.gitignore |   1 +
>  testcases/kernel/syscalls/nice/Makefile   |   2 +
>  testcases/kernel/syscalls/nice/nice05.c   | 188 ++++++++++++++++++++++
>  4 files changed, 192 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/nice/nice05.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 9d58e0aa1..98fcbbe1e 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -903,6 +903,7 @@ nice01 nice01
>  nice02 nice02
>  nice03 nice03
>  nice04 nice04
> +nice05 nice05
> 
>  open01 open01
>  open01A symlink01 -T open01
> diff --git a/testcases/kernel/syscalls/nice/.gitignore b/testcases/kernel/syscalls/nice/.gitignore
> index 9d7a1bb43..58d64779e 100644
> --- a/testcases/kernel/syscalls/nice/.gitignore
> +++ b/testcases/kernel/syscalls/nice/.gitignore
> @@ -2,3 +2,4 @@
>  /nice02
>  /nice03
>  /nice04
> +/nice05
> diff --git a/testcases/kernel/syscalls/nice/Makefile b/testcases/kernel/syscalls/nice/Makefile
> index 044619fb8..02e78a295 100644
> --- a/testcases/kernel/syscalls/nice/Makefile
> +++ b/testcases/kernel/syscalls/nice/Makefile
> @@ -3,6 +3,8 @@
> 
>  top_srcdir		?= ../../../..
> 
> +nice05: CFLAGS += -pthread
> +
>  include $(top_srcdir)/include/mk/testcases.mk
> 
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/nice/nice05.c b/testcases/kernel/syscalls/nice/nice05.c
> new file mode 100644
> index 000000000..8ef33f932
> --- /dev/null
> +++ b/testcases/kernel/syscalls/nice/nice05.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright(c) 2022 Huawei Technologies Co., Ltd
> + * Author: Li Mengfei <limengfei4@huawei.com>
> + *         Zhao Gongyi <zhaogongyi@huawei.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * 1. Create a high nice thread and a low nice thread, the main
> + *    thread wake them at the same time
> + * 2. Both threads run on the same CPU
> + * 3. Verify that the low nice thread executes more time than
> + *    the high nice thread
> + */
> +
> +#define _GNU_SOURCE
> +#include <pthread.h>
> +#include <sys/types.h>
> +#include <stdio.h>
> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
> +#include "lapi/syscalls.h"
> +
> +#define SEC2NS(sec)  ((sec) * 1000000000LL)
> +
> +static pthread_barrier_t barrier;
> +static int some_cpu;
> +static cpu_set_t *set;
> +
> +static void set_nice(int nice_inc)
> +{
> +	int orig_nice;
> +
> +	orig_nice = SAFE_GETPRIORITY(PRIO_PROCESS, 0);
> +
> +	TEST(nice(nice_inc));
> +
> +	if (TST_RET != (orig_nice + nice_inc)) {
> +		tst_brk(TBROK | TTERRNO, "nice(%d) returned %li, expected %i",
> +			nice_inc, TST_RET, orig_nice + nice_inc);
> +	}
> +
> +	if (TST_ERR)
> +		tst_brk(TBROK | TTERRNO, "nice(%d) failed", nice_inc);
> +}
> +
> +static void *nice_low_thread(void *arg)
> +{
> +	volatile int number = 0;
> +
> +	set_nice((intptr_t)arg);
> +	TEST(pthread_barrier_wait(&barrier));
> +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
> +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
> +
> +	while (1)
> +		number++;
> +
> +	return NULL;
> +}
> +
> +static void *nice_high_thread(void *arg)
> +{
> +	volatile int number = 0;
> +
> +	set_nice((intptr_t)arg);
> +	TEST(pthread_barrier_wait(&barrier));
> +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
> +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");

It may be worth to add SAFE_PTHREAD_BARRIER_WAIT() to the
tst_safe_pthread_h to make the code nicer.

> +	while (1)
> +		number++;
> +
> +	return NULL;
> +}
> +
> +static void setup(void)
> +{
> +	size_t size;
> +	size_t i;
> +	int nrcpus = 1024;
> +
> +	set = CPU_ALLOC(nrcpus);
> +	if (!set)
> +		tst_brk(TBROK | TERRNO, "CPU_ALLOC()");
> +
> +	size = CPU_ALLOC_SIZE(nrcpus);
> +	CPU_ZERO_S(size, set);
> +	if (sched_getaffinity(0, size, set) < 0)
> +		tst_brk(TBROK | TERRNO, "sched_getaffinity()");
> +
> +	for (i = 0; i < size * 8; i++)
> +		if (CPU_ISSET_S(i, size, set))
> +			some_cpu = i;
> +
> +	CPU_ZERO_S(size, set);
> +	CPU_SET_S(some_cpu, size, set);
> +	if (sched_setaffinity(0, size, set) < 0)
> +		tst_brk(TBROK | TERRNO, "sched_setaffinity()");
> +}
> +
> +static void cleanup(void)
> +{
> +	if (set)
> +		CPU_FREE(set);

This is very minor however we do not seem to use set anywhere outside
the setup so we may as well free it there.

> +}
> +
> +static void verify_nice(void)
> +{
> +	intptr_t nice_inc_high = -1;
> +	intptr_t nice_inc_low = -2;
> +	clockid_t nice_low_clockid, nice_high_clockid;
> +	struct timespec nice_high_ts, nice_low_ts;
> +	long long delta;
> +	pid_t pid;
> +	pthread_t thread[2];
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {

Is there a reason why we run the actual test in the child?

> +		TEST(pthread_barrier_init(&barrier, NULL, 3));
> +		if (TST_RET != 0) {
> +			tst_brk(TBROK | TTERRNO,
> +					"pthread_barrier_init() failed");
> +		}
> +
> +		SAFE_PTHREAD_CREATE(&thread[0], NULL, nice_high_thread,
> +				(void *)nice_inc_high);
> +		SAFE_PTHREAD_CREATE(&thread[1], NULL, nice_low_thread,
> +				(void *)nice_inc_low);
> +
> +		TEST(pthread_barrier_wait(&barrier));
> +		if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD) {
> +			tst_brk(TBROK | TTERRNO,
> +					"pthread_barrier_wait() failed");
> +		}
> +
> +		sleep(tst_remaining_runtime());
> +
> +		if (pthread_getcpuclockid(thread[1], &nice_low_clockid) != 0) {
> +			perror("clock_getcpuclockid");
> +			tst_brk(TBROK | TERRNO,
> +					"clock_getcpuclockid() failed");
> +		}
> +		if (pthread_getcpuclockid(thread[0], &nice_high_clockid) != 0) {
> +			perror("clock_getcpuclockid");
> +			tst_brk(TBROK | TERRNO,
> +					"clock_getcpuclockid() failed");
> +		}
> +
> +		if (clock_gettime(nice_low_clockid, &nice_low_ts) == -1) {
> +			tst_brk(TBROK | TERRNO,
> +					"clock_getcpuclockid() failed");
> +		}
> +
> +		if (clock_gettime(nice_high_clockid, &nice_high_ts) == -1) {
> +			tst_brk(TBROK | TERRNO,
> +					"clock_getcpuclockid() failed");
> +		}

We do have SAFE_CLOCK_GETTIME() please use them.

> +		tst_res(TINFO, "Nice low thread CPU time: %ld.%09ld s",
> +			nice_low_ts.tv_sec, nice_low_ts.tv_nsec);
> +		tst_res(TINFO, "Nice high thread CPU time: %ld.%09ld s",
> +			nice_high_ts.tv_sec, nice_high_ts.tv_nsec);
> +
> +		delta = SEC2NS(nice_low_ts.tv_sec - nice_high_ts.tv_sec) +
> +			(nice_low_ts.tv_nsec - nice_high_ts.tv_nsec);

We do have a tst_timespec_diff_{us,ns,ms} functions in the tst_timer.h
so we may as well use them.

> +		if (delta < 0) {
> +			tst_res(TFAIL, "executes less cycles than "
> +				"the high nice thread, delta: %lld ns", delta);
> +		} else {
> +			tst_res(TPASS, "executes more cycles than "
> +				"the high nice thread, delta: %lld ns", delta);
> +		}
> +		return;
> +	}
> +	SAFE_WAIT(NULL);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = verify_nice,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.max_runtime = 3,
> +};
> --
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[xuyang2018.jy]

Hi Richard,Zhao

This case compile failed on centos7 as below
safe_pthread.c:93:38: error: expected ‘;’, ‘,’ or ‘)’ before ‘barrier’
           pthread_barrier_t *restrict barrier,

But I don't know why ....

ps: I guess a old gcc bug or pthread lib bug.

Best Regards
Yang Xu
> Hello,
> 
> zhaogongyi via ltp <ltp@lists.linux.it> writes:
> 
>> Hi Cyril,
>>
>> Thanks for your review! I have resubmit a patch according to your
>> suggestion. Please see:
>> https://patchwork.ozlabs.org/project/ltp/patch/20220824095144.259871-1-zhaogongyi@huawei.com/
> 
> Merged now thanks!
> 
> If the test fails randomly then increasing max_runtime will probably help.
> 
>>
>> Best Wishes,
>> Gongyi
>>
>>>
>>> Hi!
>>>> Add test verifies that the low nice thread executes more time than the
>>>> high nice thread since the two thread binded on the same cpu.
>>>
>>> Looks very good now, there are few very minor points see below.
>>>
>>>> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
>>>> ---
>>>> v3->v4: Replace getting exec time from sum_exec_runtime with
>>> pthread_getcpuclockid().
>>>>
>>>>   runtest/syscalls                          |   1 +
>>>>   testcases/kernel/syscalls/nice/.gitignore |   1 +
>>>>   testcases/kernel/syscalls/nice/Makefile   |   2 +
>>>>   testcases/kernel/syscalls/nice/nice05.c   | 188
>>> ++++++++++++++++++++++
>>>>   4 files changed, 192 insertions(+)
>>>>   create mode 100644 testcases/kernel/syscalls/nice/nice05.c
>>>>
>>>> diff --git a/runtest/syscalls b/runtest/syscalls index
>>>> 9d58e0aa1..98fcbbe1e 100644
>>>> --- a/runtest/syscalls
>>>> +++ b/runtest/syscalls
>>>> @@ -903,6 +903,7 @@ nice01 nice01
>>>>   nice02 nice02
>>>>   nice03 nice03
>>>>   nice04 nice04
>>>> +nice05 nice05
>>>>
>>>>   open01 open01
>>>>   open01A symlink01 -T open01
>>>> diff --git a/testcases/kernel/syscalls/nice/.gitignore
>>>> b/testcases/kernel/syscalls/nice/.gitignore
>>>> index 9d7a1bb43..58d64779e 100644
>>>> --- a/testcases/kernel/syscalls/nice/.gitignore
>>>> +++ b/testcases/kernel/syscalls/nice/.gitignore
>>>> @@ -2,3 +2,4 @@
>>>>   /nice02
>>>>   /nice03
>>>>   /nice04
>>>> +/nice05
>>>> diff --git a/testcases/kernel/syscalls/nice/Makefile
>>>> b/testcases/kernel/syscalls/nice/Makefile
>>>> index 044619fb8..02e78a295 100644
>>>> --- a/testcases/kernel/syscalls/nice/Makefile
>>>> +++ b/testcases/kernel/syscalls/nice/Makefile
>>>> @@ -3,6 +3,8 @@
>>>>
>>>>   top_srcdir		?= ../../../..
>>>>
>>>> +nice05: CFLAGS += -pthread
>>>> +
>>>>   include $(top_srcdir)/include/mk/testcases.mk
>>>>
>>>>   include $(top_srcdir)/include/mk/generic_leaf_target.mk
>>>> diff --git a/testcases/kernel/syscalls/nice/nice05.c
>>>> b/testcases/kernel/syscalls/nice/nice05.c
>>>> new file mode 100644
>>>> index 000000000..8ef33f932
>>>> --- /dev/null
>>>> +++ b/testcases/kernel/syscalls/nice/nice05.c
>>>> @@ -0,0 +1,188 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>> +/*
>>>> + * Copyright(c) 2022 Huawei Technologies Co., Ltd
>>>> + * Author: Li Mengfei <limengfei4@huawei.com>
>>>> + *         Zhao Gongyi <zhaogongyi@huawei.com>
>>>> + */
>>>> +
>>>> +/*\
>>>> + * [Description]
>>>> + *
>>>> + * 1. Create a high nice thread and a low nice thread, the main
>>>> + *    thread wake them at the same time
>>>> + * 2. Both threads run on the same CPU
>>>> + * 3. Verify that the low nice thread executes more time than
>>>> + *    the high nice thread
>>>> + */
>>>> +
>>>> +#define _GNU_SOURCE
>>>> +#include <pthread.h>
>>>> +#include <sys/types.h>
>>>> +#include <stdio.h>
>>>> +#include "tst_test.h"
>>>> +#include "tst_safe_pthread.h"
>>>> +#include "lapi/syscalls.h"
>>>> +
>>>> +#define SEC2NS(sec)  ((sec) * 1000000000LL)
>>>> +
>>>> +static pthread_barrier_t barrier;
>>>> +static int some_cpu;
>>>> +static cpu_set_t *set;
>>>> +
>>>> +static void set_nice(int nice_inc)
>>>> +{
>>>> +	int orig_nice;
>>>> +
>>>> +	orig_nice = SAFE_GETPRIORITY(PRIO_PROCESS, 0);
>>>> +
>>>> +	TEST(nice(nice_inc));
>>>> +
>>>> +	if (TST_RET != (orig_nice + nice_inc)) {
>>>> +		tst_brk(TBROK | TTERRNO, "nice(%d) returned %li, expected %i",
>>>> +			nice_inc, TST_RET, orig_nice + nice_inc);
>>>> +	}
>>>> +
>>>> +	if (TST_ERR)
>>>> +		tst_brk(TBROK | TTERRNO, "nice(%d) failed", nice_inc); }
>>>> +
>>>> +static void *nice_low_thread(void *arg) {
>>>> +	volatile int number = 0;
>>>> +
>>>> +	set_nice((intptr_t)arg);
>>>> +	TEST(pthread_barrier_wait(&barrier));
>>>> +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
>>>> +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
>>>> +
>>>> +	while (1)
>>>> +		number++;
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static void *nice_high_thread(void *arg) {
>>>> +	volatile int number = 0;
>>>> +
>>>> +	set_nice((intptr_t)arg);
>>>> +	TEST(pthread_barrier_wait(&barrier));
>>>> +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
>>>> +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
>>>
>>> It may be worth to add SAFE_PTHREAD_BARRIER_WAIT() to the
>>> tst_safe_pthread_h to make the code nicer.
>>>
>>>> +	while (1)
>>>> +		number++;
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static void setup(void)
>>>> +{
>>>> +	size_t size;
>>>> +	size_t i;
>>>> +	int nrcpus = 1024;
>>>> +
>>>> +	set = CPU_ALLOC(nrcpus);
>>>> +	if (!set)
>>>> +		tst_brk(TBROK | TERRNO, "CPU_ALLOC()");
>>>> +
>>>> +	size = CPU_ALLOC_SIZE(nrcpus);
>>>> +	CPU_ZERO_S(size, set);
>>>> +	if (sched_getaffinity(0, size, set) < 0)
>>>> +		tst_brk(TBROK | TERRNO, "sched_getaffinity()");
>>>> +
>>>> +	for (i = 0; i < size * 8; i++)
>>>> +		if (CPU_ISSET_S(i, size, set))
>>>> +			some_cpu = i;
>>>> +
>>>> +	CPU_ZERO_S(size, set);
>>>> +	CPU_SET_S(some_cpu, size, set);
>>>> +	if (sched_setaffinity(0, size, set) < 0)
>>>> +		tst_brk(TBROK | TERRNO, "sched_setaffinity()"); }
>>>> +
>>>> +static void cleanup(void)
>>>> +{
>>>> +	if (set)
>>>> +		CPU_FREE(set);
>>>
>>> This is very minor however we do not seem to use set anywhere outside
>>> the setup so we may as well free it there.
>>>
>>>> +}
>>>> +
>>>> +static void verify_nice(void)
>>>> +{
>>>> +	intptr_t nice_inc_high = -1;
>>>> +	intptr_t nice_inc_low = -2;
>>>> +	clockid_t nice_low_clockid, nice_high_clockid;
>>>> +	struct timespec nice_high_ts, nice_low_ts;
>>>> +	long long delta;
>>>> +	pid_t pid;
>>>> +	pthread_t thread[2];
>>>> +
>>>> +	pid = SAFE_FORK();
>>>> +	if (!pid) {
>>>
>>> Is there a reason why we run the actual test in the child?
>>>
>>>> +		TEST(pthread_barrier_init(&barrier, NULL, 3));
>>>> +		if (TST_RET != 0) {
>>>> +			tst_brk(TBROK | TTERRNO,
>>>> +					"pthread_barrier_init() failed");
>>>> +		}
>>>> +
>>>> +		SAFE_PTHREAD_CREATE(&thread[0], NULL, nice_high_thread,
>>>> +				(void *)nice_inc_high);
>>>> +		SAFE_PTHREAD_CREATE(&thread[1], NULL, nice_low_thread,
>>>> +				(void *)nice_inc_low);
>>>> +
>>>> +		TEST(pthread_barrier_wait(&barrier));
>>>> +		if (TST_RET != 0 && TST_RET !=
>>> PTHREAD_BARRIER_SERIAL_THREAD) {
>>>> +			tst_brk(TBROK | TTERRNO,
>>>> +					"pthread_barrier_wait() failed");
>>>> +		}
>>>> +
>>>> +		sleep(tst_remaining_runtime());
>>>> +
>>>> +		if (pthread_getcpuclockid(thread[1], &nice_low_clockid) != 0) {
>>>> +			perror("clock_getcpuclockid");
>>>> +			tst_brk(TBROK | TERRNO,
>>>> +					"clock_getcpuclockid() failed");
>>>> +		}
>>>> +		if (pthread_getcpuclockid(thread[0], &nice_high_clockid) != 0) {
>>>> +			perror("clock_getcpuclockid");
>>>> +			tst_brk(TBROK | TERRNO,
>>>> +					"clock_getcpuclockid() failed");
>>>> +		}
>>>> +
>>>> +		if (clock_gettime(nice_low_clockid, &nice_low_ts) == -1) {
>>>> +			tst_brk(TBROK | TERRNO,
>>>> +					"clock_getcpuclockid() failed");
>>>> +		}
>>>> +
>>>> +		if (clock_gettime(nice_high_clockid, &nice_high_ts) == -1) {
>>>> +			tst_brk(TBROK | TERRNO,
>>>> +					"clock_getcpuclockid() failed");
>>>> +		}
>>>
>>> We do have SAFE_CLOCK_GETTIME() please use them.
>>>
>>>> +		tst_res(TINFO, "Nice low thread CPU time: %ld.%09ld s",
>>>> +			nice_low_ts.tv_sec, nice_low_ts.tv_nsec);
>>>> +		tst_res(TINFO, "Nice high thread CPU time: %ld.%09ld s",
>>>> +			nice_high_ts.tv_sec, nice_high_ts.tv_nsec);
>>>> +
>>>> +		delta = SEC2NS(nice_low_ts.tv_sec - nice_high_ts.tv_sec) +
>>>> +			(nice_low_ts.tv_nsec - nice_high_ts.tv_nsec);
>>>
>>> We do have a tst_timespec_diff_{us,ns,ms} functions in the tst_timer.h so
>>> we may as well use them.
>>>
>>>> +		if (delta < 0) {
>>>> +			tst_res(TFAIL, "executes less cycles than "
>>>> +				"the high nice thread, delta: %lld ns", delta);
>>>> +		} else {
>>>> +			tst_res(TPASS, "executes more cycles than "
>>>> +				"the high nice thread, delta: %lld ns", delta);
>>>> +		}
>>>> +		return;
>>>> +	}
>>>> +	SAFE_WAIT(NULL);
>>>> +}
>>>> +
>>>> +static struct tst_test test = {
>>>> +	.setup = setup,
>>>> +	.cleanup = cleanup,
>>>> +	.test_all = verify_nice,
>>>> +	.needs_root = 1,
>>>> +	.forks_child = 1,
>>>> +	.max_runtime = 3,
>>>> +};
>>>> --
>>>> 2.17.1
>>>>
>>>
>>> --
>>> Cyril Hrubis
>>> chrubis@suse.cz
> 
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[Richard Palethorpe]

Hello,

zhaogongyi via ltp <ltp@lists.linux.it> writes:

> Hi Cyril,
>
> Thanks for your review! I have resubmit a patch according to your
> suggestion. Please see:
> https://patchwork.ozlabs.org/project/ltp/patch/20220824095144.259871-1-zhaogongyi@huawei.com/

Merged now thanks!

If the test fails randomly then increasing max_runtime will probably help.

>
> Best Wishes,
> Gongyi
>
>> 
>> Hi!
>> > Add test verifies that the low nice thread executes more time than the
>> > high nice thread since the two thread binded on the same cpu.
>> 
>> Looks very good now, there are few very minor points see below.
>> 
>> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
>> > ---
>> > v3->v4: Replace getting exec time from sum_exec_runtime with
>> pthread_getcpuclockid().
>> >
>> >  runtest/syscalls                          |   1 +
>> >  testcases/kernel/syscalls/nice/.gitignore |   1 +
>> >  testcases/kernel/syscalls/nice/Makefile   |   2 +
>> >  testcases/kernel/syscalls/nice/nice05.c   | 188
>> ++++++++++++++++++++++
>> >  4 files changed, 192 insertions(+)
>> >  create mode 100644 testcases/kernel/syscalls/nice/nice05.c
>> >
>> > diff --git a/runtest/syscalls b/runtest/syscalls index
>> > 9d58e0aa1..98fcbbe1e 100644
>> > --- a/runtest/syscalls
>> > +++ b/runtest/syscalls
>> > @@ -903,6 +903,7 @@ nice01 nice01
>> >  nice02 nice02
>> >  nice03 nice03
>> >  nice04 nice04
>> > +nice05 nice05
>> >
>> >  open01 open01
>> >  open01A symlink01 -T open01
>> > diff --git a/testcases/kernel/syscalls/nice/.gitignore
>> > b/testcases/kernel/syscalls/nice/.gitignore
>> > index 9d7a1bb43..58d64779e 100644
>> > --- a/testcases/kernel/syscalls/nice/.gitignore
>> > +++ b/testcases/kernel/syscalls/nice/.gitignore
>> > @@ -2,3 +2,4 @@
>> >  /nice02
>> >  /nice03
>> >  /nice04
>> > +/nice05
>> > diff --git a/testcases/kernel/syscalls/nice/Makefile
>> > b/testcases/kernel/syscalls/nice/Makefile
>> > index 044619fb8..02e78a295 100644
>> > --- a/testcases/kernel/syscalls/nice/Makefile
>> > +++ b/testcases/kernel/syscalls/nice/Makefile
>> > @@ -3,6 +3,8 @@
>> >
>> >  top_srcdir		?= ../../../..
>> >
>> > +nice05: CFLAGS += -pthread
>> > +
>> >  include $(top_srcdir)/include/mk/testcases.mk
>> >
>> >  include $(top_srcdir)/include/mk/generic_leaf_target.mk
>> > diff --git a/testcases/kernel/syscalls/nice/nice05.c
>> > b/testcases/kernel/syscalls/nice/nice05.c
>> > new file mode 100644
>> > index 000000000..8ef33f932
>> > --- /dev/null
>> > +++ b/testcases/kernel/syscalls/nice/nice05.c
>> > @@ -0,0 +1,188 @@
>> > +// SPDX-License-Identifier: GPL-2.0-or-later
>> > +/*
>> > + * Copyright(c) 2022 Huawei Technologies Co., Ltd
>> > + * Author: Li Mengfei <limengfei4@huawei.com>
>> > + *         Zhao Gongyi <zhaogongyi@huawei.com>
>> > + */
>> > +
>> > +/*\
>> > + * [Description]
>> > + *
>> > + * 1. Create a high nice thread and a low nice thread, the main
>> > + *    thread wake them at the same time
>> > + * 2. Both threads run on the same CPU
>> > + * 3. Verify that the low nice thread executes more time than
>> > + *    the high nice thread
>> > + */
>> > +
>> > +#define _GNU_SOURCE
>> > +#include <pthread.h>
>> > +#include <sys/types.h>
>> > +#include <stdio.h>
>> > +#include "tst_test.h"
>> > +#include "tst_safe_pthread.h"
>> > +#include "lapi/syscalls.h"
>> > +
>> > +#define SEC2NS(sec)  ((sec) * 1000000000LL)
>> > +
>> > +static pthread_barrier_t barrier;
>> > +static int some_cpu;
>> > +static cpu_set_t *set;
>> > +
>> > +static void set_nice(int nice_inc)
>> > +{
>> > +	int orig_nice;
>> > +
>> > +	orig_nice = SAFE_GETPRIORITY(PRIO_PROCESS, 0);
>> > +
>> > +	TEST(nice(nice_inc));
>> > +
>> > +	if (TST_RET != (orig_nice + nice_inc)) {
>> > +		tst_brk(TBROK | TTERRNO, "nice(%d) returned %li, expected %i",
>> > +			nice_inc, TST_RET, orig_nice + nice_inc);
>> > +	}
>> > +
>> > +	if (TST_ERR)
>> > +		tst_brk(TBROK | TTERRNO, "nice(%d) failed", nice_inc); }
>> > +
>> > +static void *nice_low_thread(void *arg) {
>> > +	volatile int number = 0;
>> > +
>> > +	set_nice((intptr_t)arg);
>> > +	TEST(pthread_barrier_wait(&barrier));
>> > +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
>> > +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
>> > +
>> > +	while (1)
>> > +		number++;
>> > +
>> > +	return NULL;
>> > +}
>> > +
>> > +static void *nice_high_thread(void *arg) {
>> > +	volatile int number = 0;
>> > +
>> > +	set_nice((intptr_t)arg);
>> > +	TEST(pthread_barrier_wait(&barrier));
>> > +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
>> > +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
>> 
>> It may be worth to add SAFE_PTHREAD_BARRIER_WAIT() to the
>> tst_safe_pthread_h to make the code nicer.
>> 
>> > +	while (1)
>> > +		number++;
>> > +
>> > +	return NULL;
>> > +}
>> > +
>> > +static void setup(void)
>> > +{
>> > +	size_t size;
>> > +	size_t i;
>> > +	int nrcpus = 1024;
>> > +
>> > +	set = CPU_ALLOC(nrcpus);
>> > +	if (!set)
>> > +		tst_brk(TBROK | TERRNO, "CPU_ALLOC()");
>> > +
>> > +	size = CPU_ALLOC_SIZE(nrcpus);
>> > +	CPU_ZERO_S(size, set);
>> > +	if (sched_getaffinity(0, size, set) < 0)
>> > +		tst_brk(TBROK | TERRNO, "sched_getaffinity()");
>> > +
>> > +	for (i = 0; i < size * 8; i++)
>> > +		if (CPU_ISSET_S(i, size, set))
>> > +			some_cpu = i;
>> > +
>> > +	CPU_ZERO_S(size, set);
>> > +	CPU_SET_S(some_cpu, size, set);
>> > +	if (sched_setaffinity(0, size, set) < 0)
>> > +		tst_brk(TBROK | TERRNO, "sched_setaffinity()"); }
>> > +
>> > +static void cleanup(void)
>> > +{
>> > +	if (set)
>> > +		CPU_FREE(set);
>> 
>> This is very minor however we do not seem to use set anywhere outside
>> the setup so we may as well free it there.
>> 
>> > +}
>> > +
>> > +static void verify_nice(void)
>> > +{
>> > +	intptr_t nice_inc_high = -1;
>> > +	intptr_t nice_inc_low = -2;
>> > +	clockid_t nice_low_clockid, nice_high_clockid;
>> > +	struct timespec nice_high_ts, nice_low_ts;
>> > +	long long delta;
>> > +	pid_t pid;
>> > +	pthread_t thread[2];
>> > +
>> > +	pid = SAFE_FORK();
>> > +	if (!pid) {
>> 
>> Is there a reason why we run the actual test in the child?
>> 
>> > +		TEST(pthread_barrier_init(&barrier, NULL, 3));
>> > +		if (TST_RET != 0) {
>> > +			tst_brk(TBROK | TTERRNO,
>> > +					"pthread_barrier_init() failed");
>> > +		}
>> > +
>> > +		SAFE_PTHREAD_CREATE(&thread[0], NULL, nice_high_thread,
>> > +				(void *)nice_inc_high);
>> > +		SAFE_PTHREAD_CREATE(&thread[1], NULL, nice_low_thread,
>> > +				(void *)nice_inc_low);
>> > +
>> > +		TEST(pthread_barrier_wait(&barrier));
>> > +		if (TST_RET != 0 && TST_RET !=
>> PTHREAD_BARRIER_SERIAL_THREAD) {
>> > +			tst_brk(TBROK | TTERRNO,
>> > +					"pthread_barrier_wait() failed");
>> > +		}
>> > +
>> > +		sleep(tst_remaining_runtime());
>> > +
>> > +		if (pthread_getcpuclockid(thread[1], &nice_low_clockid) != 0) {
>> > +			perror("clock_getcpuclockid");
>> > +			tst_brk(TBROK | TERRNO,
>> > +					"clock_getcpuclockid() failed");
>> > +		}
>> > +		if (pthread_getcpuclockid(thread[0], &nice_high_clockid) != 0) {
>> > +			perror("clock_getcpuclockid");
>> > +			tst_brk(TBROK | TERRNO,
>> > +					"clock_getcpuclockid() failed");
>> > +		}
>> > +
>> > +		if (clock_gettime(nice_low_clockid, &nice_low_ts) == -1) {
>> > +			tst_brk(TBROK | TERRNO,
>> > +					"clock_getcpuclockid() failed");
>> > +		}
>> > +
>> > +		if (clock_gettime(nice_high_clockid, &nice_high_ts) == -1) {
>> > +			tst_brk(TBROK | TERRNO,
>> > +					"clock_getcpuclockid() failed");
>> > +		}
>> 
>> We do have SAFE_CLOCK_GETTIME() please use them.
>> 
>> > +		tst_res(TINFO, "Nice low thread CPU time: %ld.%09ld s",
>> > +			nice_low_ts.tv_sec, nice_low_ts.tv_nsec);
>> > +		tst_res(TINFO, "Nice high thread CPU time: %ld.%09ld s",
>> > +			nice_high_ts.tv_sec, nice_high_ts.tv_nsec);
>> > +
>> > +		delta = SEC2NS(nice_low_ts.tv_sec - nice_high_ts.tv_sec) +
>> > +			(nice_low_ts.tv_nsec - nice_high_ts.tv_nsec);
>> 
>> We do have a tst_timespec_diff_{us,ns,ms} functions in the tst_timer.h so
>> we may as well use them.
>> 
>> > +		if (delta < 0) {
>> > +			tst_res(TFAIL, "executes less cycles than "
>> > +				"the high nice thread, delta: %lld ns", delta);
>> > +		} else {
>> > +			tst_res(TPASS, "executes more cycles than "
>> > +				"the high nice thread, delta: %lld ns", delta);
>> > +		}
>> > +		return;
>> > +	}
>> > +	SAFE_WAIT(NULL);
>> > +}
>> > +
>> > +static struct tst_test test = {
>> > +	.setup = setup,
>> > +	.cleanup = cleanup,
>> > +	.test_all = verify_nice,
>> > +	.needs_root = 1,
>> > +	.forks_child = 1,
>> > +	.max_runtime = 3,
>> > +};
>> > --
>> > 2.17.1
>> >
>> 
>> --
>> Cyril Hrubis
>> chrubis@suse.cz


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall

[zhaogongyi via ltp]

Hi Cyril,

Thanks for your review! I have resubmit a patch according to your suggestion. Please see: https://patchwork.ozlabs.org/project/ltp/patch/20220824095144.259871-1-zhaogongyi@huawei.com/

Best Wishes,
Gongyi

> 
> Hi!
> > Add test verifies that the low nice thread executes more time than the
> > high nice thread since the two thread binded on the same cpu.
> 
> Looks very good now, there are few very minor points see below.
> 
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> > ---
> > v3->v4: Replace getting exec time from sum_exec_runtime with
> pthread_getcpuclockid().
> >
> >  runtest/syscalls                          |   1 +
> >  testcases/kernel/syscalls/nice/.gitignore |   1 +
> >  testcases/kernel/syscalls/nice/Makefile   |   2 +
> >  testcases/kernel/syscalls/nice/nice05.c   | 188
> ++++++++++++++++++++++
> >  4 files changed, 192 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/nice/nice05.c
> >
> > diff --git a/runtest/syscalls b/runtest/syscalls index
> > 9d58e0aa1..98fcbbe1e 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -903,6 +903,7 @@ nice01 nice01
> >  nice02 nice02
> >  nice03 nice03
> >  nice04 nice04
> > +nice05 nice05
> >
> >  open01 open01
> >  open01A symlink01 -T open01
> > diff --git a/testcases/kernel/syscalls/nice/.gitignore
> > b/testcases/kernel/syscalls/nice/.gitignore
> > index 9d7a1bb43..58d64779e 100644
> > --- a/testcases/kernel/syscalls/nice/.gitignore
> > +++ b/testcases/kernel/syscalls/nice/.gitignore
> > @@ -2,3 +2,4 @@
> >  /nice02
> >  /nice03
> >  /nice04
> > +/nice05
> > diff --git a/testcases/kernel/syscalls/nice/Makefile
> > b/testcases/kernel/syscalls/nice/Makefile
> > index 044619fb8..02e78a295 100644
> > --- a/testcases/kernel/syscalls/nice/Makefile
> > +++ b/testcases/kernel/syscalls/nice/Makefile
> > @@ -3,6 +3,8 @@
> >
> >  top_srcdir		?= ../../../..
> >
> > +nice05: CFLAGS += -pthread
> > +
> >  include $(top_srcdir)/include/mk/testcases.mk
> >
> >  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/syscalls/nice/nice05.c
> > b/testcases/kernel/syscalls/nice/nice05.c
> > new file mode 100644
> > index 000000000..8ef33f932
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/nice/nice05.c
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright(c) 2022 Huawei Technologies Co., Ltd
> > + * Author: Li Mengfei <limengfei4@huawei.com>
> > + *         Zhao Gongyi <zhaogongyi@huawei.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * 1. Create a high nice thread and a low nice thread, the main
> > + *    thread wake them at the same time
> > + * 2. Both threads run on the same CPU
> > + * 3. Verify that the low nice thread executes more time than
> > + *    the high nice thread
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <pthread.h>
> > +#include <sys/types.h>
> > +#include <stdio.h>
> > +#include "tst_test.h"
> > +#include "tst_safe_pthread.h"
> > +#include "lapi/syscalls.h"
> > +
> > +#define SEC2NS(sec)  ((sec) * 1000000000LL)
> > +
> > +static pthread_barrier_t barrier;
> > +static int some_cpu;
> > +static cpu_set_t *set;
> > +
> > +static void set_nice(int nice_inc)
> > +{
> > +	int orig_nice;
> > +
> > +	orig_nice = SAFE_GETPRIORITY(PRIO_PROCESS, 0);
> > +
> > +	TEST(nice(nice_inc));
> > +
> > +	if (TST_RET != (orig_nice + nice_inc)) {
> > +		tst_brk(TBROK | TTERRNO, "nice(%d) returned %li, expected %i",
> > +			nice_inc, TST_RET, orig_nice + nice_inc);
> > +	}
> > +
> > +	if (TST_ERR)
> > +		tst_brk(TBROK | TTERRNO, "nice(%d) failed", nice_inc); }
> > +
> > +static void *nice_low_thread(void *arg) {
> > +	volatile int number = 0;
> > +
> > +	set_nice((intptr_t)arg);
> > +	TEST(pthread_barrier_wait(&barrier));
> > +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
> > +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
> > +
> > +	while (1)
> > +		number++;
> > +
> > +	return NULL;
> > +}
> > +
> > +static void *nice_high_thread(void *arg) {
> > +	volatile int number = 0;
> > +
> > +	set_nice((intptr_t)arg);
> > +	TEST(pthread_barrier_wait(&barrier));
> > +	if (TST_RET != 0 && TST_RET != PTHREAD_BARRIER_SERIAL_THREAD)
> > +		tst_brk(TBROK | TRERRNO, "pthread_barrier_wait() failed");
> 
> It may be worth to add SAFE_PTHREAD_BARRIER_WAIT() to the
> tst_safe_pthread_h to make the code nicer.
> 
> > +	while (1)
> > +		number++;
> > +
> > +	return NULL;
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	size_t size;
> > +	size_t i;
> > +	int nrcpus = 1024;
> > +
> > +	set = CPU_ALLOC(nrcpus);
> > +	if (!set)
> > +		tst_brk(TBROK | TERRNO, "CPU_ALLOC()");
> > +
> > +	size = CPU_ALLOC_SIZE(nrcpus);
> > +	CPU_ZERO_S(size, set);
> > +	if (sched_getaffinity(0, size, set) < 0)
> > +		tst_brk(TBROK | TERRNO, "sched_getaffinity()");
> > +
> > +	for (i = 0; i < size * 8; i++)
> > +		if (CPU_ISSET_S(i, size, set))
> > +			some_cpu = i;
> > +
> > +	CPU_ZERO_S(size, set);
> > +	CPU_SET_S(some_cpu, size, set);
> > +	if (sched_setaffinity(0, size, set) < 0)
> > +		tst_brk(TBROK | TERRNO, "sched_setaffinity()"); }
> > +
> > +static void cleanup(void)
> > +{
> > +	if (set)
> > +		CPU_FREE(set);
> 
> This is very minor however we do not seem to use set anywhere outside
> the setup so we may as well free it there.
> 
> > +}
> > +
> > +static void verify_nice(void)
> > +{
> > +	intptr_t nice_inc_high = -1;
> > +	intptr_t nice_inc_low = -2;
> > +	clockid_t nice_low_clockid, nice_high_clockid;
> > +	struct timespec nice_high_ts, nice_low_ts;
> > +	long long delta;
> > +	pid_t pid;
> > +	pthread_t thread[2];
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> 
> Is there a reason why we run the actual test in the child?
> 
> > +		TEST(pthread_barrier_init(&barrier, NULL, 3));
> > +		if (TST_RET != 0) {
> > +			tst_brk(TBROK | TTERRNO,
> > +					"pthread_barrier_init() failed");
> > +		}
> > +
> > +		SAFE_PTHREAD_CREATE(&thread[0], NULL, nice_high_thread,
> > +				(void *)nice_inc_high);
> > +		SAFE_PTHREAD_CREATE(&thread[1], NULL, nice_low_thread,
> > +				(void *)nice_inc_low);
> > +
> > +		TEST(pthread_barrier_wait(&barrier));
> > +		if (TST_RET != 0 && TST_RET !=
> PTHREAD_BARRIER_SERIAL_THREAD) {
> > +			tst_brk(TBROK | TTERRNO,
> > +					"pthread_barrier_wait() failed");
> > +		}
> > +
> > +		sleep(tst_remaining_runtime());
> > +
> > +		if (pthread_getcpuclockid(thread[1], &nice_low_clockid) != 0) {
> > +			perror("clock_getcpuclockid");
> > +			tst_brk(TBROK | TERRNO,
> > +					"clock_getcpuclockid() failed");
> > +		}
> > +		if (pthread_getcpuclockid(thread[0], &nice_high_clockid) != 0) {
> > +			perror("clock_getcpuclockid");
> > +			tst_brk(TBROK | TERRNO,
> > +					"clock_getcpuclockid() failed");
> > +		}
> > +
> > +		if (clock_gettime(nice_low_clockid, &nice_low_ts) == -1) {
> > +			tst_brk(TBROK | TERRNO,
> > +					"clock_getcpuclockid() failed");
> > +		}
> > +
> > +		if (clock_gettime(nice_high_clockid, &nice_high_ts) == -1) {
> > +			tst_brk(TBROK | TERRNO,
> > +					"clock_getcpuclockid() failed");
> > +		}
> 
> We do have SAFE_CLOCK_GETTIME() please use them.
> 
> > +		tst_res(TINFO, "Nice low thread CPU time: %ld.%09ld s",
> > +			nice_low_ts.tv_sec, nice_low_ts.tv_nsec);
> > +		tst_res(TINFO, "Nice high thread CPU time: %ld.%09ld s",
> > +			nice_high_ts.tv_sec, nice_high_ts.tv_nsec);
> > +
> > +		delta = SEC2NS(nice_low_ts.tv_sec - nice_high_ts.tv_sec) +
> > +			(nice_low_ts.tv_nsec - nice_high_ts.tv_nsec);
> 
> We do have a tst_timespec_diff_{us,ns,ms} functions in the tst_timer.h so
> we may as well use them.
> 
> > +		if (delta < 0) {
> > +			tst_res(TFAIL, "executes less cycles than "
> > +				"the high nice thread, delta: %lld ns", delta);
> > +		} else {
> > +			tst_res(TPASS, "executes more cycles than "
> > +				"the high nice thread, delta: %lld ns", delta);
> > +		}
> > +		return;
> > +	}
> > +	SAFE_WAIT(NULL);
> > +}
> > +
> > +static struct tst_test test = {
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +	.test_all = verify_nice,
> > +	.needs_root = 1,
> > +	.forks_child = 1,
> > +	.max_runtime = 3,
> > +};
> > --
> > 2.17.1
> >
> 
> --
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp