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
