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://source.denx.de/Xenomai/xenomai-images/-/jobs/302794
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
