On 02.08.21 11:45, Jan Kiszka wrote: > On 31.07.21 09:08, Florian Bezdeka wrote: >> This patch was based on the patch sent out by Song and reworked. >> Especially >> - switched from CLOCK_MONOTONIC to CLOCK_REALTIME >> According to POSIX this service is based on CLOCK_REALTIME >> >> - Fixed some mutex leaks / missing pthread_mutex_destroy() >> >> - Removed some magic numbers used for making sure the syscall does >> not return too early >> >> - Fixed several mutex deadlocks. Once mutex_timedlock() was >> successful following calls will deadlock (as long as no special >> DEADLOCK mutex is being used) >> >> Signed-off-by: Florian Bezdeka <florian.bezd...@siemens.com> >> --- >> testsuite/smokey/y2038/syscall-tests.c | 187 +++++++++++++++++++++++++ >> 1 file changed, 187 insertions(+) >> >> diff --git a/testsuite/smokey/y2038/syscall-tests.c >> b/testsuite/smokey/y2038/syscall-tests.c >> index 08506d086..2e8add864 100644 >> --- a/testsuite/smokey/y2038/syscall-tests.c >> +++ b/testsuite/smokey/y2038/syscall-tests.c >> @@ -115,6 +115,45 @@ static inline bool ts_less(const struct xn_timespec64 >> *a, >> return false; >> } >> >> +/** >> + * Simple helper data structure for holding a thread context >> + */ >> +struct thread_context { >> + int sc_nr; >> + pthread_mutex_t *mutex; >> + struct xn_timespec64 *ts; >> + bool timedwait_timecheck; >> +}; >> + >> +/** >> + * Start the supplied function inside a separate thread, wait for completion >> + * and check the thread return value. >> + * >> + * @param thread The thread entry point >> + * @param arg The thread arguments >> + * @param exp_result The expected return value >> + * >> + * @return 0 if the thread reported @exp_result as return value, the >> thread's >> + * return value otherwise >> + */ >> +static int run_thread(void *(*thread)(void *), void *arg, int exp_result) >> +{ >> + pthread_t tid; >> + void *status; >> + long res; >> + int ret; >> + >> + if (!__T(ret, pthread_create(&tid, NULL, thread, arg))) >> + return ret; >> + >> + if (!__T(ret, pthread_join(tid, &status))) >> + return ret; >> + >> + res = *((long *) status); >> + >> + return (res == exp_result) ? 0 : ret; >> +} >> + >> static int test_sc_cobalt_sem_timedwait64(void) >> { >> int ret; >> @@ -404,6 +443,150 @@ static int test_sc_cobalt_clock_adjtime64(void) >> return 0; >> } >> >> +static void *timedlock64_thread(void *arg) >> +{ >> + struct thread_context *ctx = (struct thread_context *) arg; >> + struct xn_timespec64 t1, t2; >> + struct timespec ts_nat; >> + int ret; >> + >> + if (ctx->timedwait_timecheck) { >> + if (!__T(ret, clock_gettime(CLOCK_REALTIME, &ts_nat))) >> + return (void *)(long)ret; >> + >> + t1.tv_sec = ts_nat.tv_sec; >> + t1.tv_nsec = ts_nat.tv_nsec; >> + ts_add_ns(&t1, ctx->ts->tv_nsec); >> + ts_add_ns(&t1, ctx->ts->tv_sec * NSEC_PER_SEC); >> + } >> + >> + ret = XENOMAI_SYSCALL2(ctx->sc_nr, ctx->mutex, (void *) ctx->ts); >> + if (ret) >> + return (void *)(long)ret; >> + >> + if (ctx->timedwait_timecheck) { >> + if (!__T(ret, clock_gettime(CLOCK_REALTIME, &ts_nat))) >> + return (void *)(long)ret; >> + >> + t2.tv_sec = ts_nat.tv_sec; >> + t2.tv_nsec = ts_nat.tv_nsec; >> + >> + if (ts_less(&t2, &t1)) >> + smokey_warning("mutex_timedlock64 returned to early!\n" >> + "Expected wakeup at: %lld sec %lld >> nsec\n" >> + "Back at : %lld sec %lld >> nsec\n", >> + t1.tv_sec, t1.tv_nsec, t2.tv_sec, >> + t2.tv_nsec); >> + } >> + >> + return (void *)(long)pthread_mutex_unlock(ctx->mutex); >> +} >> + >> +static int test_sc_cobalt_mutex_timedlock64(void) >> +{ >> + int ret; >> + pthread_mutex_t mutex; >> + int sc_nr = sc_cobalt_mutex_timedlock64; >> + struct xn_timespec64 ts64; >> + struct thread_context ctx = {0}; >> + >> + ret = pthread_mutex_init(&mutex, NULL); >> + >> + /* Make sure we don't crash because of NULL pointers */ >> + ret = XENOMAI_SYSCALL2(sc_nr, NULL, NULL); >> + if (ret == -ENOSYS) { >> + smokey_note("mutex_timedlock64: skipped. (no kernel support)"); >> + return 0; // Not implemented, nothing to test, success >> + } >> + if (!smokey_assert(ret == -EINVAL)) >> + return ret ? ret : -EINVAL; >> + >> + /* >> + * mutex can be taken immediately, no need to validate >> + * NULL should be allowed >> + */ >> + ret = XENOMAI_SYSCALL2(sc_nr, &mutex, NULL); >> + if (!smokey_assert(!ret)) >> + return ret; >> + >> + if (!__T(ret, pthread_mutex_unlock(&mutex))) >> + return ret; >> + >> + /* >> + * mutex can be taken immediately, no need to validate >> + * an invalid address should be allowed >> + */ >> + ret = XENOMAI_SYSCALL2(sc_nr, &mutex, 0xdeadbeef); >> + if (!smokey_assert(!ret)) >> + return ret; >> + >> + /* >> + * mutex still locked, second thread has to fail with -EINVAL when >> + * submitting NULL as timeout >> + */ >> + ctx.sc_nr = sc_nr; >> + ctx.mutex = &mutex; >> + ctx.ts = NULL; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, -EINVAL))) >> + return ret; >> + >> + /* >> + * mutex still locked, second thread has to fail with -EFAULT when >> + * submitting an invalid address as timeout >> + */ >> + ctx.ts = (void *) 0xdeadbeef; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, -EFAULT))) >> + return ret; >> + >> + /* >> + * mutex still locked, second thread has to fail with -EFAULT when >> + * submitting an invalid timeout (while the address is valid) >> + */ >> + ts64.tv_sec = -1; >> + ctx.ts = &ts64; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, -EFAULT))) >> + return ret; >> + >> + /* >> + * mutex still locked, second thread has to fail with -ETIMEOUT when >> + * submitting a valid timeout >> + */ >> + ts64.tv_sec = 0; >> + ts64.tv_nsec = 500; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, -ETIMEDOUT))) >> + return ret; >> + >> + if (!__T(ret, pthread_mutex_unlock(&mutex))) >> + return ret; >> + >> + /* mutex available, second thread should be able to lock and unlock */ >> + ts64.tv_sec = 0; >> + ts64.tv_nsec = 500; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, 0))) >> + return ret; >> + >> + /* >> + * Locking the mutex here so the second thread has to deliver -ETIMEOUT. >> + * Timechecks will now be enabled to make sure we don't give up to early >> + */ >> + if (!__T(ret, pthread_mutex_lock(&mutex))) >> + return ret; >> + >> + ts64.tv_sec = 0; >> + ts64.tv_nsec = 500; >> + ctx.timedwait_timecheck = true; >> + if (!__T(ret, run_thread(timedlock64_thread, &ctx, -ETIMEDOUT))) >> + return ret; >> + >> + if (!__T(ret, pthread_mutex_unlock(&mutex))) >> + return ret; >> + >> + if (!__T(ret, pthread_mutex_destroy(&mutex))) >> + return ret; >> + >> + return 0; >> +} >> + >> static int run_y2038(struct smokey_test *t, int argc, char *const argv[]) >> { >> int ret; >> @@ -432,5 +615,9 @@ static int run_y2038(struct smokey_test *t, int argc, >> char *const argv[]) >> if (ret) >> return ret; >> >> + ret = test_sc_cobalt_mutex_timedlock64(); >> + if (ret) >> + return ret; >> + >> return 0; >> } >> > > We have build errors, possibly due to this change. See e.g. > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Fxenomai-images%2F-%2Fjobs%2F302794&data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Ccca6d3d98c024e30402908d9559a46f4%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634943368721149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eV5aBYrzVjAQ9QqGyX7TTRfx9eBHG73KrSLmJUJlNqs%3D&reserved=0
That's strange. Local build as well as the CI build done by the gitlab hackerspace project for the florian/y2038 is succesfull, but next is failing... florian/y2038: https://gitlab.com/Xenomai/xenomai-hacker-space/-/pipelines/345882439 > > Jan > -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux