[LTP] [PATCH v4] syscalls/nice05: Add testcase for nice() syscall
Richard Palethorpe
rpalethorpe@suse.de
Mon Oct 17 11:09:38 CEST 2022
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.
More information about the ltp
mailing list