From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [PATCH v4 5/5] y2038: testsuite/smokey/y2038: Adding test cases for mutex_timedlock64() References: <20210731070826.67908-1-florian.bezdeka@siemens.com> <20210731070826.67908-6-florian.bezdeka@siemens.com> From: Jan Kiszka Message-ID: <59ed96cc-f7e9-8559-d633-572720873341@siemens.com> Date: Mon, 2 Aug 2021 11:45:33 +0200 MIME-Version: 1.0 In-Reply-To: <20210731070826.67908-6-florian.bezdeka@siemens.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit List-Id: Discussions about the Xenomai project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Florian Bezdeka , xenomai@xenomai.org 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 > --- > 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