2024-03-04 18:13:21

by Edward Liaw

[permalink] [raw]
Subject: [PATCH v1 0/3] selftests/timers/posix_timers: various cleanups

I'm sending some patches that were orignally in
https://lore.kernel.org/lkml/[email protected]/
to prevent the timer_distribution test from hanging and also fix some
format inconsistencies.

Edward Liaw (3):
selftests/timers/posix_timers: Make signal distribution test less
fragile
selftests/timers/posix_timers: Use TAP reporting format
selftests/timers/posix_timers: Use llabs for long long

tools/testing/selftests/timers/posix_timers.c | 196 ++++++++----------
1 file changed, 89 insertions(+), 107 deletions(-)

--
2.44.0.rc1.240.g4c46232300-goog



2024-03-04 18:14:08

by Edward Liaw

[permalink] [raw]
Subject: [PATCH v1 1/3] selftests/timers/posix_timers: Make signal distribution test less fragile

The signal distribution test has a tendency to hang for a long time as the
signal delivery is not really evenly distributed.

Increasing the timer interval to 10ms makes this less likely. Add a timeout
to catch the case where it hangs and terminate the test gracefully.

While at it get rid of the pointless atomic operation on a the thread local
variable in the signal handler.

Signed-off-by: Thomas Gleixner <[email protected]>
[edliaw: Rebase and fix checkpatch recommendations]
Signed-off-by: Edward Liaw <[email protected]>
---
tools/testing/selftests/timers/posix_timers.c | 43 ++++++++++++-------
1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index d49dd3ffd0d9..03779b6b3c20 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -24,7 +24,8 @@ static volatile int done;
/* Busy loop in userspace to elapse ITIMER_VIRTUAL */
static void user_loop(void)
{
- while (!done);
+ while (!done)
+ continue;
}

/*
@@ -184,18 +185,19 @@ static int check_timer_create(int which)
return 0;
}

-int remain;
-__thread int got_signal;
+static int remain;
+static __thread int got_signal;

static void *distribution_thread(void *arg)
{
- while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
+ while (__atomic_load_n(&remain, __ATOMIC_RELAXED) && !done)
+ continue;
return NULL;
}

static void distribution_handler(int nr)
{
- if (!__atomic_exchange_n(&got_signal, 1, __ATOMIC_RELAXED))
+ if (++got_signal == 1)
__atomic_fetch_sub(&remain, 1, __ATOMIC_RELAXED);
}

@@ -205,17 +207,19 @@ static void distribution_handler(int nr)
*/
static int check_timer_distribution(void)
{
- int err, i;
- timer_t id;
const int nthreads = 10;
pthread_t threads[nthreads];
struct itimerspec val = {
.it_value.tv_sec = 0,
- .it_value.tv_nsec = 1000 * 1000,
+ .it_value.tv_nsec = 20 * 1000 * 1000,
.it_interval.tv_sec = 0,
- .it_interval.tv_nsec = 1000 * 1000,
+ .it_interval.tv_nsec = 20 * 1000 * 1000,
};
+ time_t start, now;
+ int err, i;
+ timer_t id;

+ done = 0;
remain = nthreads + 1; /* worker threads + this thread */
signal(SIGALRM, distribution_handler);
err = timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
@@ -240,7 +244,18 @@ static int check_timer_distribution(void)
}

/* Wait for all threads to receive the signal. */
- while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
+ now = start = time(NULL);
+ while (__atomic_load_n(&remain, __ATOMIC_RELAXED)) {
+ now = time(NULL);
+ if (now - start > 5)
+ break;
+ }
+ done = 1;
+
+ if (timer_delete(id)) {
+ ksft_perror("Can't delete timer\n");
+ return -1;
+ }

for (i = 0; i < nthreads; i++) {
err = pthread_join(threads[i], NULL);
@@ -251,12 +266,8 @@ static int check_timer_distribution(void)
}
}

- if (timer_delete(id)) {
- ksft_perror("Can't delete timer");
- return -1;
- }
+ ksft_test_result((now - start <= 5), "%s\n", __func__);

- ksft_test_result_pass("check_timer_distribution\n");
return 0;
}

@@ -265,7 +276,7 @@ int main(int argc, char **argv)
ksft_print_header();
ksft_set_plan(6);

- ksft_print_msg("Testing posix timers. False negative may happen on CPU execution \n");
+ ksft_print_msg("Testing posix timers. False negative may happen on CPU execution\n");
ksft_print_msg("based timers if other threads run on the CPU...\n");

if (check_itimer(ITIMER_VIRTUAL) < 0)
--
2.44.0.rc1.240.g4c46232300-goog


2024-03-04 18:42:39

by Edward Liaw

[permalink] [raw]
Subject: [PATCH v1 3/3] selftests/timers/posix_timers: Use llabs for long long

Fixes clang warning: absolute value function 'abs' given an argument of
type 'long long' but has parameter of type 'int' which may cause
truncation of value.

Signed-off-by: Edward Liaw <[email protected]>
---
tools/testing/selftests/timers/posix_timers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index 0f550fc9e879..78b4b2d3dc44 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -82,7 +82,7 @@ static int check_diff(struct timeval start, struct timeval end)
diff = end.tv_usec - start.tv_usec;
diff += (end.tv_sec - start.tv_sec) * USECS_PER_SEC;

- if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2)
+ if (llabs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2)
return -1;

return 0;
--
2.44.0.rc1.240.g4c46232300-goog


2024-03-07 21:04:58

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v1 0/3] selftests/timers/posix_timers: various cleanups

On Mon, Mar 04 2024 at 18:11, Edward Liaw wrote:
> I'm sending some patches that were orignally in
> https://lore.kernel.org/lkml/[email protected]/
> to prevent the timer_distribution test from hanging and also fix some
> format inconsistencies.

Thanks for picking those up and moving them forward. Any particular
reason why you didn't pick up the full set?

Thanks,

tglx

2024-03-07 21:42:13

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v1 0/3] selftests/timers/posix_timers: various cleanups

On Thu, Mar 07 2024 at 13:34, Edward Liaw wrote:
>> Thanks for picking those up and moving them forward. Any particular
>> reason why you didn't pick up the full set?
>
> I didn't know enough about the code to resolve some of the merges in the
> full set. I had run into the issue with the test timer_distribution test
> hanging on the Android kernel and wanted to get that fixed first.

Fair enough. I've marked your series for my post merge window tree and
I'll have a look at the rest of the pile.

Thanks,

tglx