* [PATCH 2/5] libcobalt: improve documentation regarding mutex initializers
2019-03-07 13:21 [PATCH 1/5] libcobalt: improve mutex autoinit support Norbert Lange
@ 2019-03-07 13:21 ` Norbert Lange
2019-04-01 17:20 ` Jan Kiszka
2019-03-07 13:21 ` [PATCH 3/5] libcobalt: improve condvar autoinit support Norbert Lange
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Norbert Lange @ 2019-03-07 13:21 UTC (permalink / raw)
To: xenomai
Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
lib/cobalt/mutex.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
index be9f6ed80..125981d40 100644
--- a/lib/cobalt/mutex.c
+++ b/lib/cobalt/mutex.c
@@ -49,8 +49,12 @@
* By default, Cobalt mutexes are of the normal type, use no
* priority protocol and may not be shared between several processes.
*
- * Note that only pthread_mutex_init() may be used to initialize a mutex, using
- * the static initializer @a PTHREAD_MUTEX_INITIALIZER is not supported.
+ * Note that pthread_mutex_init() should be used to initialize a mutex, using
+ * the static initializer @a PTHREAD_MUTEX_INITIALIZER will delay the
+ * initialization to the first method called on the mutex and will
+ * most likely introduce switches to secondary mode.
+ * The documentation (and specififcally api-tags) of the mutex services assumes
+ * a mutex was explicitely initialised with pthread_mutex_init().
*
*@{
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/5] libcobalt: improve documentation regarding mutex initializers
2019-03-07 13:21 ` [PATCH 2/5] libcobalt: improve documentation regarding mutex initializers Norbert Lange
@ 2019-04-01 17:20 ` Jan Kiszka
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2019-04-01 17:20 UTC (permalink / raw)
To: Norbert Lange, xenomai
On 07.03.19 14:21, Norbert Lange via Xenomai wrote:
> Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
> ---
> lib/cobalt/mutex.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
> index be9f6ed80..125981d40 100644
> --- a/lib/cobalt/mutex.c
> +++ b/lib/cobalt/mutex.c
> @@ -49,8 +49,12 @@
> * By default, Cobalt mutexes are of the normal type, use no
> * priority protocol and may not be shared between several processes.
> *
> - * Note that only pthread_mutex_init() may be used to initialize a mutex, using
> - * the static initializer @a PTHREAD_MUTEX_INITIALIZER is not supported.
> + * Note that pthread_mutex_init() should be used to initialize a mutex, using
> + * the static initializer @a PTHREAD_MUTEX_INITIALIZER will delay the
> + * initialization to the first method called on the mutex and will
> + * most likely introduce switches to secondary mode.
> + * The documentation (and specififcally api-tags) of the mutex services assumes
specifically
> + * a mutex was explicitely initialised with pthread_mutex_init().
explicitly
> *
> *@{
> */
>
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] libcobalt: improve condvar autoinit support
2019-03-07 13:21 [PATCH 1/5] libcobalt: improve mutex autoinit support Norbert Lange
2019-03-07 13:21 ` [PATCH 2/5] libcobalt: improve documentation regarding mutex initializers Norbert Lange
@ 2019-03-07 13:21 ` Norbert Lange
2019-04-01 17:27 ` Jan Kiszka
2019-03-07 13:21 ` [PATCH 4/5] libcobalt: improve documentation regarding condvar initializers Norbert Lange
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Norbert Lange @ 2019-03-07 13:21 UTC (permalink / raw)
To: xenomai
contrary to some comments, condvars are automatically
initialised on signal/wait.
Correct the destroy method to not report an
error on such condvars.
Check whether the condition variable has the
static initializer is now more strict.
Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
lib/cobalt/cond.c | 93 ++++++++++++++++++++++++++---------------------
1 file changed, 51 insertions(+), 42 deletions(-)
diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
index 00a201855..92eb230ff 100644
--- a/lib/cobalt/cond.c
+++ b/lib/cobalt/cond.c
@@ -142,6 +142,42 @@ COBALT_IMPL(int, pthread_cond_init, (pthread_cond_t *cond,
return 0;
}
+static int __attribute__((cold)) _cobalt_cond_autoinit_type(const pthread_cond_t *cond)
+{
+ static const pthread_cond_t cond_initializers[] = {
+ PTHREAD_COND_INITIALIZER
+ };
+ static const int cond_types[] =
+ {
+ 0
+ };
+
+ unsigned i = sizeof(cond_types) / sizeof(cond_types[0]);
+ while (i-- > 0)
+ {
+ if (memcmp(cond, &cond_initializers[i], sizeof(cond_initializers[0])) == 0)
+ return cond_types[i];
+ }
+ return -1;
+}
+
+static int __attribute__((cold)) _cobalt_cond_doautoinit(union cobalt_cond_union *ucond)
+{
+ int type;
+ type = _cobalt_cond_autoinit_type(&ucond->native_cond);
+ if (type < 0)
+ return EINVAL;
+
+ return __COBALT(pthread_cond_init(&ucond->native_cond, NULL));
+}
+
+static inline int _cobalt_cond_autoinit(union cobalt_cond_union *ucond)
+{
+ if (unlikely(ucond->shadow_cond.magic != COBALT_COND_MAGIC))
+ return _cobalt_cond_doautoinit(ucond);
+ return 0;
+}
+
/**
* @fn int pthread_cond_destroy(pthread_cond_t *cond)
* @brief Destroy a condition variable
@@ -170,6 +206,9 @@ COBALT_IMPL(int, pthread_cond_destroy, (pthread_cond_t *cond))
{
struct cobalt_cond_shadow *_cond = &((union cobalt_cond_union *)cond)->shadow_cond;
+ if (unlikely(_cond->magic != COBALT_COND_MAGIC))
+ return (_cobalt_cond_autoinit_type(cond) < 0) ? EINVAL : 0;
+
return -XENOMAI_SYSCALL1( sc_cobalt_cond_destroy, _cond);
}
@@ -193,12 +232,6 @@ static void __pthread_cond_cleanup(void *data)
c->mutex->lockcnt = c->count;
}
-static int __attribute__((cold)) cobalt_cond_autoinit(pthread_cond_t *cond)
-{
- return __COBALT(pthread_cond_init(cond, NULL));
-}
-
-
/**
* Wait on a condition variable.
*
@@ -262,10 +295,10 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
if (_mx->magic != COBALT_MUTEX_MAGIC)
return EINVAL;
- if (_cnd->magic != COBALT_COND_MAGIC)
- goto autoinit;
+ err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+ if (err)
+ return err;
- cont:
if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
xnhandle_t cur = cobalt_get_current();
@@ -297,12 +330,6 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
pthread_testcancel();
return -err ?: -c.err;
-
- autoinit:
- err = cobalt_cond_autoinit(cond);
- if (err)
- return err;
- goto cont;
}
/**
@@ -357,10 +384,10 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
if (_mx->magic != COBALT_MUTEX_MAGIC)
return EINVAL;
- if (_cnd->magic != COBALT_COND_MAGIC)
- goto autoinit;
+ err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+ if (err)
+ return err;
- cont:
if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
xnhandle_t cur = cobalt_get_current();
@@ -391,12 +418,6 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
pthread_testcancel();
return -err ?: -c.err;
-
- autoinit:
- err = cobalt_cond_autoinit(cond);
- if (err)
- return err;
- goto cont;
}
/**
@@ -431,10 +452,10 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
__u32 flags;
int err;
- if (_cnd->magic != COBALT_COND_MAGIC)
- goto autoinit;
+ err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+ if (err)
+ return err;
- cont:
mutex_state = get_mutex_state(_cnd);
if (mutex_state == NULL)
return 0; /* Fast path, no waiter. */
@@ -455,12 +476,6 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
cond_state->pending_signals = pending_signals + 1;
return 0;
-
- autoinit:
- err = cobalt_cond_autoinit(cond);
- if (err)
- return err;
- goto cont;
}
/**
@@ -491,10 +506,10 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
__u32 flags;
int err;
- if (_cnd->magic != COBALT_COND_MAGIC)
- goto autoinit;
+ err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
+ if (err)
+ return err;
- cont:
mutex_state = get_mutex_state(_cnd);
if (mutex_state == NULL)
return 0;
@@ -513,12 +528,6 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
cond_state->pending_signals = ~0U;
return 0;
-
- autoinit:
- err = cobalt_cond_autoinit(cond);
- if (err)
- return err;
- goto cont;
}
/**
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/5] libcobalt: improve condvar autoinit support
2019-03-07 13:21 ` [PATCH 3/5] libcobalt: improve condvar autoinit support Norbert Lange
@ 2019-04-01 17:27 ` Jan Kiszka
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2019-04-01 17:27 UTC (permalink / raw)
To: Norbert Lange, xenomai
On 07.03.19 14:21, Norbert Lange via Xenomai wrote:
> contrary to some comments, condvars are automatically
> initialised on signal/wait.
> Correct the destroy method to not report an
> error on such condvars.
>
> Check whether the condition variable has the
> static initializer is now more strict.
>
Some checkpatch warning are reported here as well. Not explicitly listing them.
> Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
> ---
> lib/cobalt/cond.c | 93 ++++++++++++++++++++++++++---------------------
> 1 file changed, 51 insertions(+), 42 deletions(-)
>
> diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
> index 00a201855..92eb230ff 100644
> --- a/lib/cobalt/cond.c
> +++ b/lib/cobalt/cond.c
> @@ -142,6 +142,42 @@ COBALT_IMPL(int, pthread_cond_init, (pthread_cond_t *cond,
> return 0;
> }
>
> +static int __attribute__((cold)) _cobalt_cond_autoinit_type(const pthread_cond_t *cond)
> +{
> + static const pthread_cond_t cond_initializers[] = {
> + PTHREAD_COND_INITIALIZER
> + };
> + static const int cond_types[] =
> + {
> + 0
> + };
> +
> + unsigned i = sizeof(cond_types) / sizeof(cond_types[0]);
> + while (i-- > 0)
> + {
> + if (memcmp(cond, &cond_initializers[i], sizeof(cond_initializers[0])) == 0)
> + return cond_types[i];
> + }
While follows the same style as for mutexes, it's a bit overkill here, given
that we only have one type.
> + return -1;
> +}
> +
> +static int __attribute__((cold)) _cobalt_cond_doautoinit(union cobalt_cond_union *ucond)
> +{
> + int type;
> + type = _cobalt_cond_autoinit_type(&ucond->native_cond);
> + if (type < 0)
> + return EINVAL;
> +
> + return __COBALT(pthread_cond_init(&ucond->native_cond, NULL));
> +}
> +
> +static inline int _cobalt_cond_autoinit(union cobalt_cond_union *ucond)
> +{
> + if (unlikely(ucond->shadow_cond.magic != COBALT_COND_MAGIC))
> + return _cobalt_cond_doautoinit(ucond);
> + return 0;
> +}
> +
> /**
> * @fn int pthread_cond_destroy(pthread_cond_t *cond)
> * @brief Destroy a condition variable
> @@ -170,6 +206,9 @@ COBALT_IMPL(int, pthread_cond_destroy, (pthread_cond_t *cond))
> {
> struct cobalt_cond_shadow *_cond = &((union cobalt_cond_union *)cond)->shadow_cond;
>
> + if (unlikely(_cond->magic != COBALT_COND_MAGIC))
> + return (_cobalt_cond_autoinit_type(cond) < 0) ? EINVAL : 0;
> +
> return -XENOMAI_SYSCALL1( sc_cobalt_cond_destroy, _cond);
> }
>
> @@ -193,12 +232,6 @@ static void __pthread_cond_cleanup(void *data)
> c->mutex->lockcnt = c->count;
> }
>
> -static int __attribute__((cold)) cobalt_cond_autoinit(pthread_cond_t *cond)
> -{
> - return __COBALT(pthread_cond_init(cond, NULL));
> -}
> -
> -
> /**
> * Wait on a condition variable.
> *
> @@ -262,10 +295,10 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
> if (_mx->magic != COBALT_MUTEX_MAGIC)
> return EINVAL;
>
> - if (_cnd->magic != COBALT_COND_MAGIC)
> - goto autoinit;
> + err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
> + if (err)
> + return err;
>
> - cont:
> if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
> xnhandle_t cur = cobalt_get_current();
>
> @@ -297,12 +330,6 @@ COBALT_IMPL(int, pthread_cond_wait, (pthread_cond_t *cond, pthread_mutex_t *mute
> pthread_testcancel();
>
> return -err ?: -c.err;
> -
> - autoinit:
> - err = cobalt_cond_autoinit(cond);
> - if (err)
> - return err;
> - goto cont;
> }
>
> /**
> @@ -357,10 +384,10 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
> if (_mx->magic != COBALT_MUTEX_MAGIC)
> return EINVAL;
>
> - if (_cnd->magic != COBALT_COND_MAGIC)
> - goto autoinit;
> + err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
> + if (err)
> + return err;
>
> - cont:
> if (_mx->attr.type == PTHREAD_MUTEX_ERRORCHECK) {
> xnhandle_t cur = cobalt_get_current();
>
> @@ -391,12 +418,6 @@ COBALT_IMPL(int, pthread_cond_timedwait, (pthread_cond_t *cond,
> pthread_testcancel();
>
> return -err ?: -c.err;
> -
> - autoinit:
> - err = cobalt_cond_autoinit(cond);
> - if (err)
> - return err;
> - goto cont;
> }
>
> /**
> @@ -431,10 +452,10 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
> __u32 flags;
> int err;
>
> - if (_cnd->magic != COBALT_COND_MAGIC)
> - goto autoinit;
> + err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
> + if (err)
> + return err;
>
> - cont:
> mutex_state = get_mutex_state(_cnd);
> if (mutex_state == NULL)
> return 0; /* Fast path, no waiter. */
> @@ -455,12 +476,6 @@ COBALT_IMPL(int, pthread_cond_signal, (pthread_cond_t *cond))
> cond_state->pending_signals = pending_signals + 1;
>
> return 0;
> -
> - autoinit:
> - err = cobalt_cond_autoinit(cond);
> - if (err)
> - return err;
> - goto cont;
> }
>
> /**
> @@ -491,10 +506,10 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
> __u32 flags;
> int err;
>
> - if (_cnd->magic != COBALT_COND_MAGIC)
> - goto autoinit;
> + err = _cobalt_cond_autoinit((union cobalt_cond_union *)cond);
> + if (err)
> + return err;
>
> - cont:
> mutex_state = get_mutex_state(_cnd);
> if (mutex_state == NULL)
> return 0;
> @@ -513,12 +528,6 @@ COBALT_IMPL(int, pthread_cond_broadcast, (pthread_cond_t *cond))
> cond_state->pending_signals = ~0U;
>
> return 0;
> -
> - autoinit:
> - err = cobalt_cond_autoinit(cond);
> - if (err)
> - return err;
> - goto cont;
> }
>
> /**
>
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] libcobalt: improve documentation regarding condvar initializers
2019-03-07 13:21 [PATCH 1/5] libcobalt: improve mutex autoinit support Norbert Lange
2019-03-07 13:21 ` [PATCH 2/5] libcobalt: improve documentation regarding mutex initializers Norbert Lange
2019-03-07 13:21 ` [PATCH 3/5] libcobalt: improve condvar autoinit support Norbert Lange
@ 2019-03-07 13:21 ` Norbert Lange
2019-04-01 17:27 ` Jan Kiszka
2019-03-07 13:21 ` [PATCH 5/5] smokey: add tests for mutex/condvar autoinit Norbert Lange
2019-04-01 17:19 ` [PATCH 1/5] libcobalt: improve mutex autoinit support Jan Kiszka
4 siblings, 1 reply; 10+ messages in thread
From: Norbert Lange @ 2019-03-07 13:21 UTC (permalink / raw)
To: xenomai
Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
lib/cobalt/cond.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
index 92eb230ff..cdbd512ae 100644
--- a/lib/cobalt/cond.c
+++ b/lib/cobalt/cond.c
@@ -47,9 +47,13 @@
* several processes (it may not be shared by default, see
* pthread_condattr_setpshared()).
*
- * Note that only pthread_cond_init() may be used to initialize a condition
- * variable, using the static initializer @a PTHREAD_COND_INITIALIZER is
- * not supported.
+ * Note that pthread_cond_init() should be used to initialize a condition
+ * variable, using the static initializer @a PTHREAD_COND_INITIALIZER will
+ * delay the initialization to the first method called on the condition
+ * variable and will most likely introduce switches to secondary mode.
+ * The documentation (and specififcally api-tags) of the
+ * condition variable services assumes the condition variable was explicitely
+ * initialised with pthread_cond_init().
*
*@{
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] libcobalt: improve documentation regarding condvar initializers
2019-03-07 13:21 ` [PATCH 4/5] libcobalt: improve documentation regarding condvar initializers Norbert Lange
@ 2019-04-01 17:27 ` Jan Kiszka
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2019-04-01 17:27 UTC (permalink / raw)
To: Norbert Lange, xenomai
On 07.03.19 14:21, Norbert Lange via Xenomai wrote:
> Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
> ---
> lib/cobalt/cond.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/cobalt/cond.c b/lib/cobalt/cond.c
> index 92eb230ff..cdbd512ae 100644
> --- a/lib/cobalt/cond.c
> +++ b/lib/cobalt/cond.c
> @@ -47,9 +47,13 @@
> * several processes (it may not be shared by default, see
> * pthread_condattr_setpshared()).
> *
> - * Note that only pthread_cond_init() may be used to initialize a condition
> - * variable, using the static initializer @a PTHREAD_COND_INITIALIZER is
> - * not supported.
> + * Note that pthread_cond_init() should be used to initialize a condition
> + * variable, using the static initializer @a PTHREAD_COND_INITIALIZER will
> + * delay the initialization to the first method called on the condition
> + * variable and will most likely introduce switches to secondary mode.
> + * The documentation (and specififcally api-tags) of the
> + * condition variable services assumes the condition variable was explicitely
> + * initialised with pthread_cond_init().
All typos of patch 2 were consistently reproduced.
Jan
> *
> *@{
> */
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] smokey: add tests for mutex/condvar autoinit
2019-03-07 13:21 [PATCH 1/5] libcobalt: improve mutex autoinit support Norbert Lange
` (2 preceding siblings ...)
2019-03-07 13:21 ` [PATCH 4/5] libcobalt: improve documentation regarding condvar initializers Norbert Lange
@ 2019-03-07 13:21 ` Norbert Lange
2019-04-01 17:32 ` Jan Kiszka
2019-04-01 17:19 ` [PATCH 1/5] libcobalt: improve mutex autoinit support Jan Kiszka
4 siblings, 1 reply; 10+ messages in thread
From: Norbert Lange @ 2019-03-07 13:21 UTC (permalink / raw)
To: xenomai
add a few testcases whete dstroy is called as first function,
and test failure if the state is a non-standard initializater.
Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
---
testsuite/smokey/posix-cond/posix-cond.c | 14 ++++
testsuite/smokey/posix-mutex/posix-mutex.c | 93 ++++++++++++++++------
2 files changed, 84 insertions(+), 23 deletions(-)
diff --git a/testsuite/smokey/posix-cond/posix-cond.c b/testsuite/smokey/posix-cond/posix-cond.c
index 153c64599..ad915c724 100644
--- a/testsuite/smokey/posix-cond/posix-cond.c
+++ b/testsuite/smokey/posix-cond/posix-cond.c
@@ -198,6 +198,19 @@ static void *cond_signaler(void *cookie)
return NULL;
}
+static void autoinit_simple_conddestroy(void)
+{
+ pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+ pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;
+ unsigned invalmagic = ~0x86860505; // ~COBALT_COND_MAGIC
+
+ memcpy((char *)&cond2 + sizeof(cond2) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
+
+ smokey_trace("%s", __func__);
+ check("cond_destroy", cond_destroy(&cond), 0);
+ check("cond_destroy invalid", cond_destroy(&cond2), -EINVAL);
+}
+
static void autoinit_simple_condwait(void)
{
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
@@ -731,6 +744,7 @@ int run_posix_cond(struct smokey_test *t, int argc, char *const argv[])
sparam.sched_priority = 2;
pthread_setschedparam(pthread_self(), SCHED_FIFO, &sparam);
+ autoinit_simple_conddestroy();
autoinit_simple_condwait();
simple_condwait();
relative_condwait();
diff --git a/testsuite/smokey/posix-mutex/posix-mutex.c b/testsuite/smokey/posix-mutex/posix-mutex.c
index 182f8c0e5..a42c52033 100644
--- a/testsuite/smokey/posix-mutex/posix-mutex.c
+++ b/testsuite/smokey/posix-mutex/posix-mutex.c
@@ -68,13 +68,13 @@ struct locker_context {
static void sleep_ms(unsigned int ms) /* < 1000 */
{
struct timespec ts;
-
+
ts.tv_sec = 0;
ts.tv_nsec = ms * 1000000;
clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);
}
-static int get_effective_prio(void)
+static int get_effective_prio(void)
{
struct cobalt_threadstat stat;
int ret;
@@ -111,13 +111,13 @@ static int do_init_mutexattr(pthread_mutexattr_t *mattr, int type, int protocol)
if (!__T(ret, pthread_mutexattr_init(mattr)))
return ret;
-
+
if (!__T(ret, pthread_mutexattr_settype(mattr, type)))
return ret;
-
+
if (!__T(ret, pthread_mutexattr_setprotocol(mattr, protocol)))
return ret;
-
+
if (!__T(ret, pthread_mutexattr_setpshared(mattr, PTHREAD_PROCESS_PRIVATE)))
return ret;
@@ -132,13 +132,13 @@ static int do_init_mutex(pthread_mutex_t *mutex, int type, int protocol)
ret = do_init_mutexattr(&mattr, type, protocol);
if (ret)
return ret;
-
+
if (!__T(ret, pthread_mutex_init(mutex, &mattr)))
return ret;
if (!__T(ret, pthread_mutexattr_destroy(&mattr)))
return ret;
-
+
return 0;
}
@@ -150,7 +150,7 @@ static int do_init_mutex_ceiling(pthread_mutex_t *mutex, int type, int prio)
ret = do_init_mutexattr(&mattr, type, PTHREAD_PRIO_PROTECT);
if (ret)
return ret;
-
+
if (!__T(ret, pthread_mutexattr_setprioceiling(&mattr, prio)))
return ret;
@@ -159,7 +159,7 @@ static int do_init_mutex_ceiling(pthread_mutex_t *mutex, int type, int prio)
if (!__T(ret, pthread_mutexattr_destroy(&mattr)))
return ret;
-
+
return 0;
}
@@ -174,7 +174,7 @@ static void *mutex_timed_locker(void *arg)
if (p->barrier)
smokey_barrier_release(p->barrier);
-
+
if (__F(ret, pthread_mutex_timedlock(p->mutex, &ts)) &&
__Tassert(ret == -ETIMEDOUT))
return (void *)1;
@@ -197,7 +197,7 @@ static int do_timed_contend(pthread_mutex_t *mutex, int prio)
mutex_timed_locker, &args);
if (ret)
return ret;
-
+
if (!__T(ret, pthread_join(tid, &status)))
return ret;
@@ -296,6 +296,28 @@ static int do_contend(pthread_mutex_t *mutex, int type)
return 0;
}
+static int do_destroy(pthread_mutex_t *mutex, pthread_mutex_t *invalmutex, int type)
+{
+ int ret;
+ if (!__T(ret, pthread_mutex_destroy(mutex)))
+ return ret;
+ if (!__F(ret, pthread_mutex_destroy(invalmutex)) &&
+ __Tassert(ret == -EINVAL))
+ return -1;
+ return 0;
+}
+
+static int static_init_normal_destroy(void)
+{
+ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+ pthread_mutex_t invalmutex = PTHREAD_MUTEX_INITIALIZER;
+
+ unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+ memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
+ return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_NORMAL);
+}
+
static int static_init_normal_contend(void)
{
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -311,7 +333,7 @@ static int __dynamic_init_contend(int type)
ret = do_init_mutex(&mutex, type, PTHREAD_PRIO_NONE);
if (ret)
return ret;
-
+
return do_contend(&mutex, type);
}
@@ -320,6 +342,17 @@ static int dynamic_init_normal_contend(void)
return __dynamic_init_contend(PTHREAD_MUTEX_NORMAL);
}
+static int static_init_recursive_destroy(void)
+{
+ pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+ pthread_mutex_t invalmutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+
+ unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+ memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
+ return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_RECURSIVE);
+}
+
static int static_init_recursive_contend(void)
{
pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
@@ -332,6 +365,17 @@ static int dynamic_init_recursive_contend(void)
return __dynamic_init_contend(PTHREAD_MUTEX_RECURSIVE);
}
+static int static_init_errorcheck_destroy(void)
+{
+ pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+ pthread_mutex_t invalmutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+ unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
+
+ memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
+ return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_ERRORCHECK);
+}
+
static int static_init_errorcheck_contend(void)
{
pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
@@ -381,7 +425,7 @@ static int weak_mode_switch(void)
return -EINVAL;
/* Enter SCHED_WEAK scheduling. */
-
+
if (!__T(ret, pthread_setschedparam(pthread_self(),
SCHED_OTHER, ¶m)))
return ret;
@@ -405,7 +449,7 @@ static int weak_mode_switch(void)
return ret;
/* Dropped it, we should have relaxed in the same move. */
-
+
mode = cobalt_thread_mode();
if (!__Tassert((mode & (XNWEAK|XNRELAX)) == (XNWEAK|XNRELAX)))
return -EINVAL;
@@ -460,7 +504,7 @@ static int do_pi_contend(int prio)
*/
if (!__Tassert(get_effective_prio() == prio))
return -EINVAL;
-
+
if (!__T(ret, pthread_join(tid, &status)))
return ret;
@@ -489,7 +533,7 @@ static void *mutex_locker_steal(void *arg)
int ret;
smokey_barrier_release(p->barrier);
-
+
if (!__T(ret, pthread_mutex_lock(p->mutex)))
return (void *)(long)ret;
@@ -623,7 +667,7 @@ static int protect_raise(void)
/* We should have been given a MEDIUM -> HIGH boost. */
if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex)))
return ret;
@@ -654,7 +698,7 @@ static int protect_lower(void)
/* No boost should be applied. */
if (!__Tassert(get_effective_prio() == THREAD_PRIO_MEDIUM))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex)))
return ret;
@@ -700,7 +744,7 @@ static int protect_weak(void)
/* We should have been sent to SCHED_FIFO, THREAD_PRIO_HIGH. */
if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex)))
return ret;
@@ -754,7 +798,7 @@ static int protect_nesting_protect(void)
if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex_high)))
return ret;
@@ -786,7 +830,7 @@ static int protect_nesting_pi(void)
/* PP ceiling: MEDIUM -> HIGH */
if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
return -EINVAL;
-
+
/* PI boost expected: HIGH -> VERY_HIGH, then back to HIGH */
ret = do_pi_contend(THREAD_PRIO_VERY_HIGH);
if (ret)
@@ -794,7 +838,7 @@ static int protect_nesting_pi(void)
if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex_pp)))
return ret;
@@ -833,7 +877,7 @@ static int protect_dynamic(void)
/* We should have been given a HIGH -> VERY_HIGH boost. */
if (!__Tassert(get_effective_prio() == THREAD_PRIO_VERY_HIGH))
return -EINVAL;
-
+
if (!__T(ret, pthread_mutex_unlock(&mutex)))
return ret;
@@ -991,10 +1035,13 @@ static int run_posix_mutex(struct smokey_test *t, int argc, char *const argv[])
SCHED_FIFO, ¶m)))
return ret;
+ do_test(static_init_normal_destroy, MAX_100_MS);
do_test(static_init_normal_contend, MAX_100_MS);
do_test(dynamic_init_normal_contend, MAX_100_MS);
+ do_test(static_init_recursive_destroy, MAX_100_MS);
do_test(static_init_recursive_contend, MAX_100_MS);
do_test(dynamic_init_recursive_contend, MAX_100_MS);
+ do_test(static_init_errorcheck_destroy, MAX_100_MS);
do_test(static_init_errorcheck_contend, MAX_100_MS);
do_test(dynamic_init_errorcheck_contend, MAX_100_MS);
do_test(timed_contend, MAX_100_MS);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] smokey: add tests for mutex/condvar autoinit
2019-03-07 13:21 ` [PATCH 5/5] smokey: add tests for mutex/condvar autoinit Norbert Lange
@ 2019-04-01 17:32 ` Jan Kiszka
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2019-04-01 17:32 UTC (permalink / raw)
To: Norbert Lange, xenomai
On 07.03.19 14:21, Norbert Lange via Xenomai wrote:
> add a few testcases whete dstroy is called as first function,
...where destroy...
> and test failure if the state is a non-standard initializater.
>
checkpatch run on this one as well, please.
> Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
> ---
> testsuite/smokey/posix-cond/posix-cond.c | 14 ++++
> testsuite/smokey/posix-mutex/posix-mutex.c | 93 ++++++++++++++++------
> 2 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/testsuite/smokey/posix-cond/posix-cond.c b/testsuite/smokey/posix-cond/posix-cond.c
> index 153c64599..ad915c724 100644
> --- a/testsuite/smokey/posix-cond/posix-cond.c
> +++ b/testsuite/smokey/posix-cond/posix-cond.c
> @@ -198,6 +198,19 @@ static void *cond_signaler(void *cookie)
> return NULL;
> }
>
> +static void autoinit_simple_conddestroy(void)
> +{
> + pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> + pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;
> + unsigned invalmagic = ~0x86860505; // ~COBALT_COND_MAGIC
> +
> + memcpy((char *)&cond2 + sizeof(cond2) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
> +
> + smokey_trace("%s", __func__);
> + check("cond_destroy", cond_destroy(&cond), 0);
> + check("cond_destroy invalid", cond_destroy(&cond2), -EINVAL);
> +}
> +
> static void autoinit_simple_condwait(void)
> {
> pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> @@ -731,6 +744,7 @@ int run_posix_cond(struct smokey_test *t, int argc, char *const argv[])
> sparam.sched_priority = 2;
> pthread_setschedparam(pthread_self(), SCHED_FIFO, &sparam);
>
> + autoinit_simple_conddestroy();
> autoinit_simple_condwait();
> simple_condwait();
> relative_condwait();
> diff --git a/testsuite/smokey/posix-mutex/posix-mutex.c b/testsuite/smokey/posix-mutex/posix-mutex.c
> index 182f8c0e5..a42c52033 100644
> --- a/testsuite/smokey/posix-mutex/posix-mutex.c
> +++ b/testsuite/smokey/posix-mutex/posix-mutex.c
> @@ -68,13 +68,13 @@ struct locker_context {
> static void sleep_ms(unsigned int ms) /* < 1000 */
> {
> struct timespec ts;
> -
> +
> ts.tv_sec = 0;
> ts.tv_nsec = ms * 1000000;
> clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);
> }
>
> -static int get_effective_prio(void)
> +static int get_effective_prio(void)
> {
> struct cobalt_threadstat stat;
> int ret;
> @@ -111,13 +111,13 @@ static int do_init_mutexattr(pthread_mutexattr_t *mattr, int type, int protocol)
>
> if (!__T(ret, pthread_mutexattr_init(mattr)))
> return ret;
> -
> +
> if (!__T(ret, pthread_mutexattr_settype(mattr, type)))
> return ret;
> -
> +
> if (!__T(ret, pthread_mutexattr_setprotocol(mattr, protocol)))
> return ret;
> -
> +
> if (!__T(ret, pthread_mutexattr_setpshared(mattr, PTHREAD_PROCESS_PRIVATE)))
> return ret;
>
> @@ -132,13 +132,13 @@ static int do_init_mutex(pthread_mutex_t *mutex, int type, int protocol)
> ret = do_init_mutexattr(&mattr, type, protocol);
> if (ret)
> return ret;
> -
> +
> if (!__T(ret, pthread_mutex_init(mutex, &mattr)))
> return ret;
>
> if (!__T(ret, pthread_mutexattr_destroy(&mattr)))
> return ret;
> -
> +
> return 0;
> }
>
> @@ -150,7 +150,7 @@ static int do_init_mutex_ceiling(pthread_mutex_t *mutex, int type, int prio)
> ret = do_init_mutexattr(&mattr, type, PTHREAD_PRIO_PROTECT);
> if (ret)
> return ret;
> -
> +
> if (!__T(ret, pthread_mutexattr_setprioceiling(&mattr, prio)))
> return ret;
>
> @@ -159,7 +159,7 @@ static int do_init_mutex_ceiling(pthread_mutex_t *mutex, int type, int prio)
>
> if (!__T(ret, pthread_mutexattr_destroy(&mattr)))
> return ret;
> -
> +
> return 0;
> }
>
> @@ -174,7 +174,7 @@ static void *mutex_timed_locker(void *arg)
>
> if (p->barrier)
> smokey_barrier_release(p->barrier);
> -
> +
> if (__F(ret, pthread_mutex_timedlock(p->mutex, &ts)) &&
> __Tassert(ret == -ETIMEDOUT))
> return (void *)1;
> @@ -197,7 +197,7 @@ static int do_timed_contend(pthread_mutex_t *mutex, int prio)
> mutex_timed_locker, &args);
> if (ret)
> return ret;
> -
> +
> if (!__T(ret, pthread_join(tid, &status)))
> return ret;
>
> @@ -296,6 +296,28 @@ static int do_contend(pthread_mutex_t *mutex, int type)
> return 0;
> }
>
> +static int do_destroy(pthread_mutex_t *mutex, pthread_mutex_t *invalmutex, int type)
> +{
> + int ret;
> + if (!__T(ret, pthread_mutex_destroy(mutex)))
> + return ret;
> + if (!__F(ret, pthread_mutex_destroy(invalmutex)) &&
> + __Tassert(ret == -EINVAL))
> + return -1;
> + return 0;
> +}
> +
> +static int static_init_normal_destroy(void)
> +{
> + pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> + pthread_mutex_t invalmutex = PTHREAD_MUTEX_INITIALIZER;
> +
> + unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
> +
> + memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
> + return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_NORMAL);
> +}
> +
> static int static_init_normal_contend(void)
> {
> pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> @@ -311,7 +333,7 @@ static int __dynamic_init_contend(int type)
> ret = do_init_mutex(&mutex, type, PTHREAD_PRIO_NONE);
> if (ret)
> return ret;
> -
> +
> return do_contend(&mutex, type);
> }
>
> @@ -320,6 +342,17 @@ static int dynamic_init_normal_contend(void)
> return __dynamic_init_contend(PTHREAD_MUTEX_NORMAL);
> }
>
> +static int static_init_recursive_destroy(void)
> +{
> + pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> + pthread_mutex_t invalmutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> +
> + unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
> +
> + memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
> + return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_RECURSIVE);
> +}
> +
> static int static_init_recursive_contend(void)
> {
> pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> @@ -332,6 +365,17 @@ static int dynamic_init_recursive_contend(void)
> return __dynamic_init_contend(PTHREAD_MUTEX_RECURSIVE);
> }
>
> +static int static_init_errorcheck_destroy(void)
> +{
> + pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> + pthread_mutex_t invalmutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> +
> + unsigned invalmagic = ~0x86860303; // ~COBALT_MUTEX_MAGIC
> +
> + memcpy((char *)&invalmutex + sizeof(invalmutex) - sizeof(invalmagic), &invalmagic, sizeof(invalmagic));
> + return do_destroy(&mutex, &invalmutex, PTHREAD_MUTEX_ERRORCHECK);
> +}
> +
> static int static_init_errorcheck_contend(void)
> {
> pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> @@ -381,7 +425,7 @@ static int weak_mode_switch(void)
> return -EINVAL;
>
> /* Enter SCHED_WEAK scheduling. */
> -
> +
> if (!__T(ret, pthread_setschedparam(pthread_self(),
> SCHED_OTHER, ¶m)))
> return ret;
> @@ -405,7 +449,7 @@ static int weak_mode_switch(void)
> return ret;
>
> /* Dropped it, we should have relaxed in the same move. */
> -
> +
> mode = cobalt_thread_mode();
> if (!__Tassert((mode & (XNWEAK|XNRELAX)) == (XNWEAK|XNRELAX)))
> return -EINVAL;
> @@ -460,7 +504,7 @@ static int do_pi_contend(int prio)
> */
> if (!__Tassert(get_effective_prio() == prio))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_join(tid, &status)))
> return ret;
>
> @@ -489,7 +533,7 @@ static void *mutex_locker_steal(void *arg)
> int ret;
>
> smokey_barrier_release(p->barrier);
> -
> +
> if (!__T(ret, pthread_mutex_lock(p->mutex)))
> return (void *)(long)ret;
>
> @@ -623,7 +667,7 @@ static int protect_raise(void)
> /* We should have been given a MEDIUM -> HIGH boost. */
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex)))
> return ret;
>
> @@ -654,7 +698,7 @@ static int protect_lower(void)
> /* No boost should be applied. */
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_MEDIUM))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex)))
> return ret;
>
> @@ -700,7 +744,7 @@ static int protect_weak(void)
> /* We should have been sent to SCHED_FIFO, THREAD_PRIO_HIGH. */
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex)))
> return ret;
>
> @@ -754,7 +798,7 @@ static int protect_nesting_protect(void)
>
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex_high)))
> return ret;
>
> @@ -786,7 +830,7 @@ static int protect_nesting_pi(void)
> /* PP ceiling: MEDIUM -> HIGH */
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
> return -EINVAL;
> -
> +
> /* PI boost expected: HIGH -> VERY_HIGH, then back to HIGH */
> ret = do_pi_contend(THREAD_PRIO_VERY_HIGH);
> if (ret)
> @@ -794,7 +838,7 @@ static int protect_nesting_pi(void)
>
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_HIGH))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex_pp)))
> return ret;
>
> @@ -833,7 +877,7 @@ static int protect_dynamic(void)
> /* We should have been given a HIGH -> VERY_HIGH boost. */
> if (!__Tassert(get_effective_prio() == THREAD_PRIO_VERY_HIGH))
> return -EINVAL;
> -
> +
> if (!__T(ret, pthread_mutex_unlock(&mutex)))
> return ret;
>
> @@ -991,10 +1035,13 @@ static int run_posix_mutex(struct smokey_test *t, int argc, char *const argv[])
> SCHED_FIFO, ¶m)))
> return ret;
>
> + do_test(static_init_normal_destroy, MAX_100_MS);
> do_test(static_init_normal_contend, MAX_100_MS);
> do_test(dynamic_init_normal_contend, MAX_100_MS);
> + do_test(static_init_recursive_destroy, MAX_100_MS);
> do_test(static_init_recursive_contend, MAX_100_MS);
> do_test(dynamic_init_recursive_contend, MAX_100_MS);
> + do_test(static_init_errorcheck_destroy, MAX_100_MS);
> do_test(static_init_errorcheck_contend, MAX_100_MS);
> do_test(dynamic_init_errorcheck_contend, MAX_100_MS);
> do_test(timed_contend, MAX_100_MS);
>
I'm generally fine with sneaking some style fixes into a function change. This
one is on the edge, actually more beyond it, as there are quite a few, compared
to functional changes. When respinning this series, please split that up.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] libcobalt: improve mutex autoinit support
2019-03-07 13:21 [PATCH 1/5] libcobalt: improve mutex autoinit support Norbert Lange
` (3 preceding siblings ...)
2019-03-07 13:21 ` [PATCH 5/5] smokey: add tests for mutex/condvar autoinit Norbert Lange
@ 2019-04-01 17:19 ` Jan Kiszka
4 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2019-04-01 17:19 UTC (permalink / raw)
To: Norbert Lange, xenomai
Hi Norbert,
thanks for the patches, and sorry for the late reply.
On 07.03.19 14:21, Norbert Lange via Xenomai wrote:
> contrary to some comments, mutexes are automatically
> initialised on lock/unlock.
> Correct the destroy method to not report an
> error on such mutexes.
>
> {get,set}prioceiling requires mutexes that were explicitely
> initialised, so no change needed there
>
> Signed-off-by: Norbert Lange <norbert.lange@andritz.com>
> ---
> lib/cobalt/mutex.c | 190 +++++++++++++++++++++++----------------------
> 1 file changed, 98 insertions(+), 92 deletions(-)
>
> diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
> index ed32bba32..be9f6ed80 100644
> --- a/lib/cobalt/mutex.c
> +++ b/lib/cobalt/mutex.c
> @@ -164,70 +164,51 @@ COBALT_IMPL(int, pthread_mutex_init, (pthread_mutex_t *mutex,
> }
>
> /**
> - * Destroy a mutex.
> - *
> - * This service destroys the mutex @a mx, if it is unlocked and not referenced
> - * by any condition variable. The mutex becomes invalid for all mutex services
> - * (they all return the EINVAL error) except pthread_mutex_init().
> - *
> - * @param mutex the mutex to be destroyed.
> - *
> - * @return 0 on success,
> - * @return an error number if:
> - * - EINVAL, the mutex @a mx is invalid;
> - * - EPERM, the mutex is not process-shared and does not belong to the current
> - * process;
> - * - EBUSY, the mutex is locked, or used by a condition variable.
> + * Test if a mutex structure contains an valid autoinitializer.
...a valid...
> *
> - * @see
> - * <a href="http://www.opengroup.org/onlinepubs/000095399/functions/pthread_mutex_destroy.html">
> - * Specification.</a>
> - *
> - * @apitags{thread-unrestricted}
> + * @return the mutex type on success,
> + * @return -1 if not in supported autoinitializer state
> */
> -COBALT_IMPL(int, pthread_mutex_destroy, (pthread_mutex_t *mutex))
> +static int __attribute__((cold)) _cobalt_mutex_autoinit_type(const pthread_mutex_t *mutex)
Overlong line.
And why the leading underscore in "_cobalt_..."?
> {
> - struct cobalt_mutex_shadow *_mutex =
> - &((union cobalt_mutex_union *)mutex)->shadow_mutex;
> - int err;
> -
> - if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - return EINVAL;
> -
> - err = XENOMAI_SYSCALL1(sc_cobalt_mutex_destroy, _mutex);
> -
> - return -err;
> -}
> -
> -static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
> -{
> - static pthread_mutex_t uninit_normal_mutex =
> - PTHREAD_MUTEX_INITIALIZER;
> + static const pthread_mutex_t mutex_initializers[] = {
> +#if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
> + PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP,
> +#endif
> #if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
> - static pthread_mutex_t uninit_recursive_mutex =
> - PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> + PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP,
> #endif
> + PTHREAD_MUTEX_INITIALIZER
> + };
> + static const int mutex_types[] =
> + {
Coding style: "... = {"
> #if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
> - static pthread_mutex_t uninit_errorcheck_mutex =
> - PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> + PTHREAD_MUTEX_ERRORCHECK_NP,
> #endif
> - struct cobalt_mutex_shadow *_mutex =
> - &((union cobalt_mutex_union *)mutex)->shadow_mutex;
> +#if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
> + PTHREAD_MUTEX_RECURSIVE_NP,
> +#endif
> + PTHREAD_MUTEX_DEFAULT
> + };
> +
> + unsigned i = sizeof(mutex_types) / sizeof(mutex_types[0]);
Blank line after this variable definition - and probably not before it.
> + while (i-- > 0)
How about "for (i = 0; i < ARRAY_SIZE(mutex_types); i++)"?
> + {
Style: only put the brace on a new line when starting a function.
> + if (memcmp(mutex, &mutex_initializers[i], sizeof(mutex_initializers[0])) == 0)
Overlong line.
> + return mutex_types[i];
> + }
> + return -1;
> +}
> +
> +static int __attribute__((cold)) _cobalt_mutex_doautoinit(union cobalt_mutex_union *umutex)
Overlong line, and that leading "_".
> +{
> + struct cobalt_mutex_shadow *_mutex = &umutex->shadow_mutex;
> int err __attribute__((unused));
> pthread_mutexattr_t mattr;
> int ret = 0, type;
>
> - if (memcmp(mutex, &uninit_normal_mutex, sizeof(*mutex)) == 0)
> - type = PTHREAD_MUTEX_DEFAULT;
> -#if HAVE_DECL_PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
> - else if (memcmp(mutex, &uninit_recursive_mutex, sizeof(*mutex)) == 0)
> - type = PTHREAD_MUTEX_RECURSIVE_NP;
> -#endif
> -#if HAVE_DECL_PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
> - else if (memcmp(mutex, &uninit_errorcheck_mutex, sizeof(*mutex)) == 0)
> - type = PTHREAD_MUTEX_ERRORCHECK_NP;
> -#endif
> - else
> + type = _cobalt_mutex_autoinit_type(&umutex->native_mutex);
> + if (type < 0)
> return EINVAL;
>
> pthread_mutexattr_init(&mattr);
> @@ -238,7 +219,7 @@ static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
> goto out;
> }
> if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - ret = __COBALT(pthread_mutex_init(mutex, &mattr));
> + ret = __COBALT(pthread_mutex_init(&umutex->native_mutex, &mattr));
Overlong line - the kernel's checkpatch.pl can be helpful.
> err = __COBALT(pthread_mutex_unlock(cobalt_autoinit_mutex));
> if (err) {
> if (ret == 0)
> @@ -251,6 +232,49 @@ static int __attribute__((cold)) cobalt_mutex_autoinit(pthread_mutex_t *mutex)
> return ret;
> }
>
> +static inline int _cobalt_mutex_autoinit(union cobalt_mutex_union *umutex)
Leading "_" and overlong line
> +{
> + if (unlikely(umutex->shadow_mutex.magic != COBALT_MUTEX_MAGIC))
> + return _cobalt_mutex_doautoinit(umutex);
> + return 0;
> +}
> +
> +/**
> + * Destroy a mutex.
> + *
> + * This service destroys the mutex @a mx, if it is unlocked and not referenced
> + * by any condition variable. The mutex becomes invalid for all mutex services
> + * (they all return the EINVAL error) except pthread_mutex_init().
> + *
> + * @param mutex the mutex to be destroyed.
> + *
> + * @return 0 on success,
> + * @return an error number if:
> + * - EINVAL, the mutex @a mx is invalid;
> + * - EPERM, the mutex is not process-shared and does not belong to the current
> + * process;
> + * - EBUSY, the mutex is locked, or used by a condition variable.
> + *
> + * @see
> + * <a href="http://www.opengroup.org/onlinepubs/000095399/functions/pthread_mutex_destroy.html">
> + * Specification.</a>
> + *
> + * @apitags{thread-unrestricted}
> + */
> +COBALT_IMPL(int, pthread_mutex_destroy, (pthread_mutex_t *mutex))
> +{
> + struct cobalt_mutex_shadow *_mutex =
> + &((union cobalt_mutex_union *)mutex)->shadow_mutex;
> + int err;
> +
> + if (unlikely(_mutex->magic != COBALT_MUTEX_MAGIC))
> + return (_cobalt_mutex_autoinit_type(mutex) < 0) ? EINVAL : 0;
> +
> + err = XENOMAI_SYSCALL1(sc_cobalt_mutex_destroy, _mutex);
> +
> + return -err;
> +}
> +
> /**
> * Lock a mutex.
> *
> @@ -296,15 +320,15 @@ COBALT_IMPL(int, pthread_mutex_lock, (pthread_mutex_t *mutex))
> if (cur == XN_NO_HANDLE)
> return EPERM;
>
> - if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - goto autoinit;
> + ret = _cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
> + if (ret)
> + return ret;
>
> /*
> * We track resource ownership for auto-relax of non real-time
> * shadows and some debug features, so we must always obtain
> * them via a syscall.
> */
> -start:
> status = cobalt_get_current_mode();
> if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
> if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
> @@ -349,7 +373,7 @@ slow_path:
> _mutex->lockcnt = 1;
>
> return -ret;
> -protect:
> +protect:
> u_window = cobalt_get_current_window();
> /*
> * Can't nest lazy ceiling requests, have to take the slow
> @@ -360,11 +384,6 @@ protect:
> u_window->pp_pending = _mutex->handle;
> lazy_protect = 1;
> goto fast_path;
> -autoinit:
> - ret = cobalt_mutex_autoinit(mutex);
> - if (ret)
> - return ret;
> - goto start;
> }
>
> /**
> @@ -411,11 +430,11 @@ COBALT_IMPL(int, pthread_mutex_timedlock, (pthread_mutex_t *mutex,
> if (cur == XN_NO_HANDLE)
> return EPERM;
>
> - if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - goto autoinit;
> + ret = _cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
> + if (ret)
> + return ret;
>
> /* See __cobalt_pthread_mutex_lock() */
> -start:
> status = cobalt_get_current_mode();
> if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
> if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
> @@ -436,7 +455,7 @@ slow_path:
> if (ret == -EBUSY) {
> if (lazy_protect)
> u_window->pp_pending = XN_NO_HANDLE;
> -
> +
> switch(_mutex->attr.type) {
> case PTHREAD_MUTEX_NORMAL:
> break;
> @@ -460,7 +479,7 @@ slow_path:
> if (ret == 0)
> _mutex->lockcnt = 1;
> return -ret;
> -protect:
> +protect:
> u_window = cobalt_get_current_window();
> /*
> * Can't nest lazy ceiling requests, have to take the slow
> @@ -471,11 +490,6 @@ protect:
> u_window->pp_pending = _mutex->handle;
> lazy_protect = 1;
> goto fast_path;
> -autoinit:
> - ret = cobalt_mutex_autoinit(mutex);
> - if (ret)
> - return ret;
> - goto start;
> }
>
> /**
> @@ -515,9 +529,10 @@ COBALT_IMPL(int, pthread_mutex_trylock, (pthread_mutex_t *mutex))
> if (cur == XN_NO_HANDLE)
> return EPERM;
>
> - if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - goto autoinit;
> -start:
> + ret = _cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
> + if (ret)
> + return ret;
> +
> status = cobalt_get_current_mode();
> if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
> if (_mutex->attr.protocol == PTHREAD_PRIO_PROTECT)
> @@ -558,12 +573,7 @@ slow_path:
> _mutex->lockcnt = 1;
>
> return -ret;
> -autoinit:
> - ret = cobalt_mutex_autoinit(mutex);
> - if (ret)
> - return ret;
> - goto start;
> -protect:
> +protect:
> u_window = cobalt_get_current_window();
> /*
> * Can't nest lazy ceiling requests, have to take the slow
> @@ -611,9 +621,10 @@ COBALT_IMPL(int, pthread_mutex_unlock, (pthread_mutex_t *mutex))
> xnhandle_t cur;
> int err;
>
> - if (_mutex->magic != COBALT_MUTEX_MAGIC)
> - goto autoinit;
> -start:
> + err = _cobalt_mutex_autoinit((union cobalt_mutex_union *)mutex);
> + if (err)
> + return err;
> +
> cur = cobalt_get_current();
> if (cur == XN_NO_HANDLE)
> return EPERM;
> @@ -645,11 +656,6 @@ do_syscall:
>
> return -err;
>
> -autoinit:
> - err = cobalt_mutex_autoinit(mutex);
> - if (err)
> - return err;
> - goto start;
> unprotect:
> u_window = cobalt_get_current_window();
> u_window->pp_pending = XN_NO_HANDLE;
> @@ -707,7 +713,7 @@ COBALT_IMPL(int, pthread_mutex_setprioceiling,
> if (_mutex->magic != COBALT_MUTEX_MAGIC ||
> _mutex->attr.protocol != PTHREAD_PRIO_PROTECT)
> return EINVAL;
> -
> +
> if (prioceiling < __cobalt_std_fifo_minpri ||
> prioceiling > __cobalt_std_fifo_maxpri)
> return EINVAL;
> @@ -757,7 +763,7 @@ COBALT_IMPL(int, pthread_mutex_getprioceiling,
> if (_mutex->magic != COBALT_MUTEX_MAGIC ||
> _mutex->attr.protocol != PTHREAD_PRIO_PROTECT)
> return EINVAL;
> -
> +
> state = mutex_get_state(_mutex);
> *prioceiling = state->ceiling;
>
>
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread