* [PATCH v4 1/5] smokey: posix_mutex: Fix mutex/smokey_barrier leak
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
@ 2021-07-31 7:08 ` Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 2/5] cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and sem_timedwait() Florian Bezdeka
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Florian Bezdeka @ 2021-07-31 7:08 UTC (permalink / raw)
To: xenomai
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/5] cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and sem_timedwait()
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 1/5] smokey: posix_mutex: Fix mutex/smokey_barrier leak Florian Bezdeka
@ 2021-07-31 7:08 ` Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 3/5] y2038: cobalt/posix/mutex: Adding mutex_timedlock64 Florian Bezdeka
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Florian Bezdeka @ 2021-07-31 7:08 UTC (permalink / raw)
To: xenomai
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/5] y2038: cobalt/posix/mutex: Adding mutex_timedlock64
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 1/5] smokey: posix_mutex: Fix mutex/smokey_barrier leak Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 2/5] cobalt/posix/mutex: Harmonize pthread_mutex_timedlock() and sem_timedwait() Florian Bezdeka
@ 2021-07-31 7:08 ` Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 4/5] y2038: lib/cobalt/mutex: dispatch mutex_timedlock Florian Bezdeka
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Florian Bezdeka @ 2021-07-31 7:08 UTC (permalink / raw)
To: xenomai
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/5] y2038: lib/cobalt/mutex: dispatch mutex_timedlock
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
` (2 preceding siblings ...)
2021-07-31 7:08 ` [PATCH v4 3/5] y2038: cobalt/posix/mutex: Adding mutex_timedlock64 Florian Bezdeka
@ 2021-07-31 7:08 ` Florian Bezdeka
2021-07-31 7:08 ` [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64() Florian Bezdeka
2021-08-02 8:28 ` [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Jan Kiszka
5 siblings, 0 replies; 12+ messages in thread
From: Florian Bezdeka @ 2021-07-31 7:08 UTC (permalink / raw)
To: xenomai
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
` (3 preceding siblings ...)
2021-07-31 7:08 ` [PATCH v4 4/5] y2038: lib/cobalt/mutex: dispatch mutex_timedlock Florian Bezdeka
@ 2021-07-31 7:08 ` Florian Bezdeka
2021-08-02 9:45 ` Jan Kiszka
2021-08-02 8:28 ` [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Jan Kiszka
5 siblings, 1 reply; 12+ messages in thread
From: Florian Bezdeka @ 2021-07-31 7:08 UTC (permalink / raw)
To: xenomai
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-07-31 7:08 ` [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64() Florian Bezdeka
@ 2021-08-02 9:45 ` Jan Kiszka
2021-08-02 10:06 ` Florian Bezdeka
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2021-08-02 9:45 UTC (permalink / raw)
To: Florian Bezdeka, xenomai
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-08-02 9:45 ` Jan Kiszka
@ 2021-08-02 10:06 ` Florian Bezdeka
2021-08-02 10:27 ` Jan Kiszka
0 siblings, 1 reply; 12+ messages in thread
From: Florian Bezdeka @ 2021-08-02 10:06 UTC (permalink / raw)
To: Jan Kiszka, xenomai
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&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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-08-02 10:06 ` Florian Bezdeka
@ 2021-08-02 10:27 ` Jan Kiszka
2021-08-02 10:29 ` Florian Bezdeka
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2021-08-02 10:27 UTC (permalink / raw)
To: Florian Bezdeka, xenomai
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-08-02 10:27 ` Jan Kiszka
@ 2021-08-02 10:29 ` Florian Bezdeka
2021-08-02 15:45 ` Jan Kiszka
0 siblings, 1 reply; 12+ messages in thread
From: Florian Bezdeka @ 2021-08-02 10:29 UTC (permalink / raw)
To: Jan Kiszka, xenomai
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&data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Cf3d801d35dcd4c4c73fb08d955a017b8%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634968345821616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=aCAWr9GWQT%2BfqQgYyTNMWd0UZBjjUvUcfutdcMH4k0c%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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2FXenomai%2Fxenomai-hacker-space%2F-%2Fpipelines%2F345882439&data=04%7C01%7Cflorian.bezdeka%40siemens.com%7Cf3d801d35dcd4c4c73fb08d955a017b8%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637634968345821616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Q%2F1JfxX8knBWCCcCDodyQwWvteu4DXr7GTIDxpAlF80%3D&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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64()
2021-08-02 10:29 ` Florian Bezdeka
@ 2021-08-02 15:45 ` Jan Kiszka
0 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2021-08-02 15:45 UTC (permalink / raw)
To: Florian Bezdeka, xenomai
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] y2038: Add mutex_timedlock64() support
2021-07-31 7:08 [PATCH v4 0/5] y2038: Add mutex_timedlock64() support Florian Bezdeka
` (4 preceding siblings ...)
2021-07-31 7:08 ` [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64() Florian Bezdeka
@ 2021-08-02 8:28 ` Jan Kiszka
5 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2021-08-02 8:28 UTC (permalink / raw)
To: Florian Bezdeka, xenomai
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
^ permalink raw reply [flat|nested] 12+ messages in thread