[PATCH v4 0/5] y2038: Add mutex_timedlock64() support

[PATCH v4 0/5] y2038: Add mutex_timedlock64() support

[Florian Bezdeka]

Hi!

This series is based on the series posted by Song some time ago. The
testing part has been heavily reworked. Details are mentioned in the 
description of the last patch.

@Jan: Patch 1 and 2 are re-sends. They are not directly related to the 
y2038 stuff but the testsuite might fail if they are not applied.

Testing was done on x86 only so far, internal CI is still running but no
problems expected.

Best regards,   
Florian

Changes in v4:
  - Fixed trace integration
    (squashed into patch 3 from Song)

Changes in v3:
  - Added the new syscall to the tracing infrastructure
    (squashed into patch 3 from Song)
 
Florian Bezdeka (3):
  smokey: posix_mutex: Fix mutex/smokey_barrier leak
  cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and
    sem_timedwait()
  y2038: testsuite/smokey/y2038: Adding test cases for
    mutex_timedlock64()

Song Chen (2):
  y2038: cobalt/posix/mutex: Adding mutex_timedlock64
  y2038: lib/cobalt/mutex: dispatch mutex_timedlock

 include/cobalt/uapi/syscall.h              |   1 +
 kernel/cobalt/posix/mutex.c                |  28 ++-
 kernel/cobalt/posix/mutex.h                |   7 +
 kernel/cobalt/posix/syscall32.c            |   7 +
 kernel/cobalt/posix/syscall32.h            |   4 +
 kernel/cobalt/trace/cobalt-posix.h         |   4 +-
 lib/cobalt/mutex.c                         |   4 +
 testsuite/smokey/posix-mutex/posix-mutex.c |  62 +++++++
 testsuite/smokey/y2038/syscall-tests.c     | 187 +++++++++++++++++++++
 9 files changed, 302 insertions(+), 2 deletions(-)

-- 
2.30.2

[PATCH v4 1/5] smokey: posix_mutex: Fix mutex/smokey_barrier leak

[Florian Bezdeka]

The mutex of the smokey_barrier used inside protect_handover() was
never destroyed. This had side effects when trying to extend the test
suite with an additional function that had a mutex located on the same
address than the never cleaned up smokey_barrier lock.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 testsuite/smokey/posix-mutex/posix-mutex.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testsuite/smokey/posix-mutex/posix-mutex.c b/testsuite/smokey/posix-mutex/posix-mutex.c
index 9a55d0973..e5793c42c 100644
--- a/testsuite/smokey/posix-mutex/posix-mutex.c
+++ b/testsuite/smokey/posix-mutex/posix-mutex.c
@@ -997,6 +997,8 @@ static int protect_handover(void)
 	if (!__T(ret, pthread_mutex_destroy(&mutex)))
 		return ret;
 
+	smokey_barrier_destroy(&barrier);
+
 	return 0;
 }
 
-- 
2.30.2

[PATCH v4 2/5] cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and sem_timedwait()

[Florian Bezdeka]

According to the POSIX spec the value of the timeout parameter needs
not to be validated if the mutex/semaphore could be taken immediately.

While the implementation of the semaphore timedwait (sem_timedwait())
allowed an invalid timeout pthread_mutex_timedlock() was failing with
-EFAULT in case the mutex could be taken immediately.

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 kernel/cobalt/posix/mutex.c                |  5 ++
 testsuite/smokey/posix-mutex/posix-mutex.c | 60 ++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/kernel/cobalt/posix/mutex.c b/kernel/cobalt/posix/mutex.c
index 70fe7960a..01478978e 100644
--- a/kernel/cobalt/posix/mutex.c
+++ b/kernel/cobalt/posix/mutex.c
@@ -167,6 +167,11 @@ redo:
 	xnthread_commit_ceiling(curr);
 
 	if (xnsynch_owner_check(&mutex->synchbase, curr)) {
+		/* Check if we can take the mutex immediately */
+		ret = xnsynch_try_acquire(&mutex->synchbase);
+		if (ret != -EBUSY)
+			goto out;
+
 		if (fetch_timeout) {
 			xnlock_put_irqrestore(&nklock, s);
 			ret = fetch_timeout(&ts, u_ts);
diff --git a/testsuite/smokey/posix-mutex/posix-mutex.c b/testsuite/smokey/posix-mutex/posix-mutex.c
index e5793c42c..4aad24964 100644
--- a/testsuite/smokey/posix-mutex/posix-mutex.c
+++ b/testsuite/smokey/posix-mutex/posix-mutex.c
@@ -1002,6 +1002,65 @@ static int protect_handover(void)
 	return 0;
 }
 
+static void *mutex_timed_locker_inv_timeout(void *arg)
+{
+	struct locker_context *p = arg;
+	int ret;
+
+	if (__F(ret, pthread_mutex_timedlock(p->mutex, (void *) 0xdeadbeef)) &&
+	    __Tassert(ret == -EFAULT))
+		return (void *)1;
+
+	return NULL;
+}
+
+static int check_timedlock_abstime_validation(void)
+{
+	struct locker_context args;
+	pthread_mutex_t mutex;
+	pthread_t tid;
+	void *status;
+	int ret;
+
+	if (!__T(ret, pthread_mutex_init(&mutex, NULL)))
+		return ret;
+
+	/*
+	 * We don't own the mutex yet, so no need to validate the timeout as
+	 * the mutex can be locked immediately.
+	 *
+	 * The second parameter of phtread_mutex_timedlock() is flagged as
+	 * __nonnull so we take an invalid address instead of NULL.
+	 */
+	if (!__T(ret, pthread_mutex_timedlock(&mutex, (void *) 0xdeadbeef)))
+		return ret;
+
+	/*
+	 * Create a second thread which will have to wait and therefore will
+	 * validate the (invalid) timeout
+	 */
+	args.mutex = &mutex;
+	ret = create_thread(&tid, SCHED_FIFO, THREAD_PRIO_LOW,
+			    mutex_timed_locker_inv_timeout, &args);
+
+	if (ret)
+		return ret;
+
+	if (!__T(ret, pthread_join(tid, &status)))
+		return ret;
+
+	if (!__T(ret, pthread_mutex_unlock(&mutex)))
+		return ret;
+
+	if (!__T(ret, pthread_mutex_destroy(&mutex)))
+		return ret;
+
+	if (!__Fassert(status == NULL))
+		return -EINVAL;
+
+	return 0;
+}
+
 /* Detect obviously wrong execution times. */
 static int check_time_limit(const struct timespec *start,
 			    xnticks_t limit_ns)
@@ -1065,6 +1124,7 @@ static int run_posix_mutex(struct smokey_test *t, int argc, char *const argv[])
 	do_test(protect_dynamic, MAX_100_MS);
 	do_test(protect_trylock, MAX_100_MS);
 	do_test(protect_handover, MAX_100_MS);
+	do_test(check_timedlock_abstime_validation, MAX_100_MS);
 
 	return 0;
 }
-- 
2.30.2

[PATCH v4 3/5] y2038: cobalt/posix/mutex: Adding mutex_timedlock64

[Florian Bezdeka]

From: Song Chen <chensong_2000@189.cn>

Add a syscall specific for mutex_timedlock with 64bit time_t.

Signed-off-by: Song Chen <chensong_2000@189.cn>
[Florian: Rearranged code, coding style fixes, tracing]
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 include/cobalt/uapi/syscall.h      |  1 +
 kernel/cobalt/posix/mutex.c        | 23 ++++++++++++++++++++++-
 kernel/cobalt/posix/mutex.h        |  7 +++++++
 kernel/cobalt/posix/syscall32.c    |  7 +++++++
 kernel/cobalt/posix/syscall32.h    |  4 ++++
 kernel/cobalt/trace/cobalt-posix.h |  4 +++-
 6 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/include/cobalt/uapi/syscall.h b/include/cobalt/uapi/syscall.h
index a2795a364..6fe57c7e8 100644
--- a/include/cobalt/uapi/syscall.h
+++ b/include/cobalt/uapi/syscall.h
@@ -128,6 +128,7 @@
 #define sc_cobalt_clock_nanosleep64		105
 #define sc_cobalt_clock_getres64		106
 #define sc_cobalt_clock_adjtime64		107
+#define sc_cobalt_mutex_timedlock64		108
 
 #define __NR_COBALT_SYSCALLS			128 /* Power of 2 */
 
diff --git a/kernel/cobalt/posix/mutex.c b/kernel/cobalt/posix/mutex.c
index 01478978e..0f1c01851 100644
--- a/kernel/cobalt/posix/mutex.c
+++ b/kernel/cobalt/posix/mutex.c
@@ -21,6 +21,7 @@
 #include "mutex.h"
 #include "cond.h"
 #include "clock.h"
+#include <cobalt/kernel/time.h>
 
 static int cobalt_mutex_init_inner(struct cobalt_mutex_shadow *shadow,
 				   struct cobalt_mutex *mutex,
@@ -76,7 +77,7 @@ int __cobalt_mutex_acquire_unchecked(struct xnthread *cur,
 	int ret;
 
 	if (ts) {
-		if (ts->tv_nsec >= ONE_BILLION)
+		if (!timespec64_valid(ts))
 			return -EINVAL;
 		ret = xnsynch_acquire(&mutex->synchbase, ts2ns(ts) + 1, XN_REALTIME);
 	} else
@@ -357,6 +358,19 @@ static inline int mutex_fetch_timeout(struct timespec64 *ts,
 	return u_ts == NULL ? -EFAULT : cobalt_get_u_timespec(ts, u_ts);
 }
 
+static inline int mutex_fetch_timeout64(struct timespec64 *ts,
+					const void __user *u_ts)
+{
+	return u_ts == NULL ? -EFAULT : cobalt_get_timespec64(ts, u_ts);
+}
+
+int __cobalt_mutex_timedlock64(struct cobalt_mutex_shadow __user *u_mx,
+			       const void __user *u_ts)
+{
+	return __cobalt_mutex_timedlock_break(u_mx, u_ts,
+					      mutex_fetch_timeout64);
+}
+
 COBALT_SYSCALL(mutex_timedlock, primary,
 	       (struct cobalt_mutex_shadow __user *u_mx,
 		const struct __user_old_timespec __user *u_ts))
@@ -364,6 +378,13 @@ COBALT_SYSCALL(mutex_timedlock, primary,
 	return __cobalt_mutex_timedlock_break(u_mx, u_ts, mutex_fetch_timeout);
 }
 
+COBALT_SYSCALL(mutex_timedlock64, primary,
+	       (struct cobalt_mutex_shadow __user *u_mx,
+		const struct __kernel_timespec __user *u_ts))
+{
+	return __cobalt_mutex_timedlock64(u_mx, u_ts);
+}
+
 COBALT_SYSCALL(mutex_unlock, nonrestartable,
 	       (struct cobalt_mutex_shadow __user *u_mx))
 {
diff --git a/kernel/cobalt/posix/mutex.h b/kernel/cobalt/posix/mutex.h
index d76f2a9ea..d7d43219e 100644
--- a/kernel/cobalt/posix/mutex.h
+++ b/kernel/cobalt/posix/mutex.h
@@ -40,6 +40,9 @@ int __cobalt_mutex_timedlock_break(struct cobalt_mutex_shadow __user *u_mx,
 				   int (*fetch_timeout)(struct timespec64 *ts,
 							const void __user *u_ts));
 
+int __cobalt_mutex_timedlock64(struct cobalt_mutex_shadow __user *u_mx,
+				   const void __user *u_ts);
+
 int __cobalt_mutex_acquire_unchecked(struct xnthread *cur,
 				     struct cobalt_mutex *mutex,
 				     const struct timespec64 *ts);
@@ -64,6 +67,10 @@ COBALT_SYSCALL_DECL(mutex_timedlock,
 		    (struct cobalt_mutex_shadow __user *u_mx,
 		     const struct __user_old_timespec __user *u_ts));
 
+COBALT_SYSCALL(mutex_timedlock64, primary,
+	       (struct cobalt_mutex_shadow __user *u_mx,
+		const struct __kernel_timespec __user *u_ts));
+
 COBALT_SYSCALL_DECL(mutex_unlock,
 		    (struct cobalt_mutex_shadow __user *u_mx));
 
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index a37478f3f..bd30decce 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -267,6 +267,13 @@ COBALT_SYSCALL32emu(mutex_timedlock, primary,
 	return __cobalt_mutex_timedlock_break(u_mx, u_ts, sys32_fetch_timeout);
 }
 
+COBALT_SYSCALL32emu(mutex_timedlock64, primary,
+		    (struct cobalt_mutex_shadow __user *u_mx,
+		     const struct __kernel_timespec __user *u_ts))
+{
+	return __cobalt_mutex_timedlock64(u_mx, u_ts);
+}
+
 COBALT_SYSCALL32emu(cond_wait_prologue, nonrestartable,
 		    (struct cobalt_cond_shadow __user *u_cnd,
 		     struct cobalt_mutex_shadow __user *u_mx,
diff --git a/kernel/cobalt/posix/syscall32.h b/kernel/cobalt/posix/syscall32.h
index 64f6e4931..d464d967f 100644
--- a/kernel/cobalt/posix/syscall32.h
+++ b/kernel/cobalt/posix/syscall32.h
@@ -98,6 +98,10 @@ COBALT_SYSCALL32emu_DECL(mutex_timedlock,
 			 (struct cobalt_mutex_shadow __user *u_mx,
 			  const struct old_timespec32 __user *u_ts));
 
+COBALT_SYSCALL32emu_DECL(mutex_timedlock64,
+			 (struct cobalt_mutex_shadow __user *u_mx,
+			  const struct __kernel_timespec __user *u_ts));
+
 COBALT_SYSCALL32emu_DECL(cond_wait_prologue,
 			 (struct cobalt_cond_shadow __user *u_cnd,
 			  struct cobalt_mutex_shadow __user *u_mx,
diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
index d994007bb..b7fc5ff64 100644
--- a/kernel/cobalt/trace/cobalt-posix.h
+++ b/kernel/cobalt/trace/cobalt-posix.h
@@ -160,7 +160,9 @@
 		__cobalt_symbolic_syscall(clock_settime64),		\
 		__cobalt_symbolic_syscall(clock_nanosleep64),		\
 		__cobalt_symbolic_syscall(clock_getres64),		\
-		__cobalt_symbolic_syscall(clock_adjtime64))
+		__cobalt_symbolic_syscall(clock_adjtime64),		\
+		__cobalt_symbolic_syscall(mutex_timedlock64))
+
 
 DECLARE_EVENT_CLASS(cobalt_syscall_entry,
 	TP_PROTO(unsigned int nr),
-- 
2.30.2

[PATCH v4 4/5] y2038: lib/cobalt/mutex: dispatch mutex_timedlock

[Florian Bezdeka]

From: Song Chen <chensong_2000@189.cn>

In case the libc in use reports a 64 bit time_t dispacht
mutex_timedlock to the kernel entry point that accepts it.

Signed-off-by: Song Chen <chensong_2000@189.cn>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 lib/cobalt/mutex.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
index 29236c75a..73e45a1c4 100644
--- a/lib/cobalt/mutex.c
+++ b/lib/cobalt/mutex.c
@@ -479,7 +479,11 @@ slow_path:
 	}
 
 	do {
+#ifdef __USE_TIME_BITS64
+		ret = XENOMAI_SYSCALL2(sc_cobalt_mutex_timedlock64, _mutex, to);
+#else
 		ret = XENOMAI_SYSCALL2(sc_cobalt_mutex_timedlock, _mutex, to);
+#endif
 	} while (ret == -EINTR);
 
 	if (ret == 0)
-- 
2.30.2

[PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Florian Bezdeka]

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.bezdeka@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;
 }
-- 
2.30.2

Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Jan Kiszka]

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.bezdeka@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

Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Florian Bezdeka]

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.bezdeka@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&amp;data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Ccca6d3d98c024e30402908d9559a46f4%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634943368721149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=eV5aBYrzVjAQ9QqGyX7TTRfx9eBHG73KrSLmJUJlNqs%3D&amp;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

Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Jan Kiszka]

On 02.08.21 12:06, Florian Bezdeka wrote:
> 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.bezdeka@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
> 
> 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
> 

I'm trying to convert ctx->timedwait_timecheck into a local var, maybe
that helps the compiler.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Florian Bezdeka]

On 02.08.21 12:27, Jan Kiszka wrote:
> On 02.08.21 12:06, Florian Bezdeka wrote:
>> 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.bezdeka@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&amp;data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Cf3d801d35dcd4c4c73fb08d955a017b8%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634968345821616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aCAWr9GWQT%2BfqQgYyTNMWd0UZBjjUvUcfutdcMH4k0c%3D&amp;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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2FXenomai%2Fxenomai-hacker-space%2F-%2Fpipelines%2F345882439&amp;data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Cf3d801d35dcd4c4c73fb08d955a017b8%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634968345821616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Q%2F1JfxX8knBWCCcCDodyQwWvteu4DXr7GTIDxpAlF80%3D&amp;reserved=0
>>
> 
> I'm trying to convert ctx->timedwait_timecheck into a local var, maybe
> that helps the compiler.

Seems to be an compiler thing.

index 36475347e..d7784e3da 100644
--- a/testsuite/smokey/y2038/syscall-tests.c
+++ b/testsuite/smokey/y2038/syscall-tests.c
@@ -447,7 +447,7 @@ static int test_sc_cobalt_clock_adjtime64(void)
 static void *timedlock64_thread(void *arg)
 {
        struct thread_context *ctx = (struct thread_context *) arg;
-       struct xn_timespec64 t1, t2;
+       struct xn_timespec64 t1 = {0}, t2 = {0};
        struct timespec ts_nat;
        int ret;

Should do it.

> 
> Jan
> 


-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()

[Jan Kiszka]

On 02.08.21 12:29, Florian Bezdeka wrote:
> On 02.08.21 12:27, Jan Kiszka wrote:
>> On 02.08.21 12:06, Florian Bezdeka wrote:
>>> 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.bezdeka@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
>>>
>>> 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
>>>
>>
>> I'm trying to convert ctx->timedwait_timecheck into a local var, maybe
>> that helps the compiler.
> 
> Seems to be an compiler thing.
> 
> index 36475347e..d7784e3da 100644
> --- a/testsuite/smokey/y2038/syscall-tests.c
> +++ b/testsuite/smokey/y2038/syscall-tests.c
> @@ -447,7 +447,7 @@ static int test_sc_cobalt_clock_adjtime64(void)
>  static void *timedlock64_thread(void *arg)
>  {
>         struct thread_context *ctx = (struct thread_context *) arg;
> -       struct xn_timespec64 t1, t2;
> +       struct xn_timespec64 t1 = {0}, t2 = {0};
>         struct timespec ts_nat;
>         int ret;
> 
> Should do it.
> 

I've done t1 = {} now. Build is green.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH v4 0/5] y2038: Add mutex_timedlock64() support

[Jan Kiszka]

On 31.07.21 09:08, Florian Bezdeka wrote:
> Hi!
> 
> This series is based on the series posted by Song some time ago. The
> testing part has been heavily reworked. Details are mentioned in the 
> description of the last patch.
> 
> @Jan: Patch 1 and 2 are re-sends. They are not directly related to the 
> y2038 stuff but the testsuite might fail if they are not applied.
> 
> Testing was done on x86 only so far, internal CI is still running but no
> problems expected.
> 
> Best regards,   
> Florian
> 
> Changes in v4:
>   - Fixed trace integration
>     (squashed into patch 3 from Song)
> 
> Changes in v3:
>   - Added the new syscall to the tracing infrastructure
>     (squashed into patch 3 from Song)
>  
> Florian Bezdeka (3):
>   smokey: posix_mutex: Fix mutex/smokey_barrier leak
>   cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and
>     sem_timedwait()
>   y2038: testsuite/smokey/y2038: Adding test cases for
>     mutex_timedlock64()
> 
> Song Chen (2):
>   y2038: cobalt/posix/mutex: Adding mutex_timedlock64
>   y2038: lib/cobalt/mutex: dispatch mutex_timedlock
> 
>  include/cobalt/uapi/syscall.h              |   1 +
>  kernel/cobalt/posix/mutex.c                |  28 ++-
>  kernel/cobalt/posix/mutex.h                |   7 +
>  kernel/cobalt/posix/syscall32.c            |   7 +
>  kernel/cobalt/posix/syscall32.h            |   4 +
>  kernel/cobalt/trace/cobalt-posix.h         |   4 +-
>  lib/cobalt/mutex.c                         |   4 +
>  testsuite/smokey/posix-mutex/posix-mutex.c |  62 +++++++
>  testsuite/smokey/y2038/syscall-tests.c     | 187 +++++++++++++++++++++
>  9 files changed, 302 insertions(+), 2 deletions(-)
> 

Thanks, remaing 3 also applied.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux