From 755e9312b51f8a03f38d9287e1e2bfbae285cfbe Mon Sep 17 00:00:00 2001 From: Juha Niskanen Date: Mon, 10 Apr 2017 07:18:16 -0600 Subject: [PATCH 1/3] pthread: use cancel cleanup handlers in rwlock --- libc/pthread/pthread_rwlock_rdlock.c | 17 ++++++++++++++++- libc/pthread/pthread_rwlock_wrlock.c | 27 +++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/libc/pthread/pthread_rwlock_rdlock.c b/libc/pthread/pthread_rwlock_rdlock.c index 9fd461a758..19979f79bb 100644 --- a/libc/pthread/pthread_rwlock_rdlock.c +++ b/libc/pthread/pthread_rwlock_rdlock.c @@ -1,5 +1,5 @@ /**************************************************************************** - * libc/pthread/pthread_rwlockread.c + * libc/pthread/pthread_rwlock_rdlock.c * * Copyright (C) 2017 Mark Schulte. All rights reserved. * Author: Mark Schulte @@ -50,6 +50,15 @@ * Private Functions ****************************************************************************/ +#ifdef CONFIG_PTHREAD_CLEANUP +static void rdlock_cleanup(FAR void *arg) +{ + FAR pthread_rwlock_t *rw_lock = (FAR pthread_rwlock_t *)arg; + + pthread_mutex_unlock(&rw_lock->lock); +} +#endif + static int tryrdlock(FAR pthread_rwlock_t *rw_lock) { int err; @@ -116,6 +125,9 @@ int pthread_rwlock_timedrdlock(FAR pthread_rwlock_t *rw_lock, return err; } +#ifdef CONFIG_PTHREAD_CLEANUP + pthread_cleanup_push(&rdlock_cleanup, rw_lock); +#endif while ((err = tryrdlock(rw_lock)) == EBUSY) { if (ts != NULL) @@ -132,6 +144,9 @@ int pthread_rwlock_timedrdlock(FAR pthread_rwlock_t *rw_lock, break; } } +#ifdef CONFIG_PTHREAD_CLEANUP + pthread_cleanup_pop(0); +#endif pthread_mutex_unlock(&rw_lock->lock); return err; diff --git a/libc/pthread/pthread_rwlock_wrlock.c b/libc/pthread/pthread_rwlock_wrlock.c index ecda4cb25b..fcda35cb4f 100644 --- a/libc/pthread/pthread_rwlock_wrlock.c +++ b/libc/pthread/pthread_rwlock_wrlock.c @@ -1,5 +1,5 @@ /**************************************************************************** - * libc/pthread/pthread_rwlockwrite.c + * libc/pthread/pthread_rwlock_wrlock.c * * Copyright (C) 2017 Mark Schulte. All rights reserved. * Author: Mark Schulte @@ -46,15 +46,29 @@ #include +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +#ifdef CONFIG_PTHREAD_CLEANUP +static void wrlock_cleanup(FAR void *arg) +{ + FAR pthread_rwlock_t *rw_lock = (FAR pthread_rwlock_t *)arg; + + rw_lock->num_writers--; + pthread_mutex_unlock(&rw_lock->lock); +} +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ /**************************************************************************** - * Name: pthread_rwlock_rdlock + * Name: pthread_rwlock_wrlock * * Description: - * Locks a read/write lock for reading + * Locks a read/write lock for writing * * Parameters: * None @@ -106,6 +120,9 @@ int pthread_rwlock_timedwrlock(FAR pthread_rwlock_t *rw_lock, rw_lock->num_writers++; +#ifdef CONFIG_PTHREAD_CLEANUP + pthread_cleanup_push(&wrlock_cleanup, rw_lock); +#endif while (rw_lock->write_in_progress || rw_lock->num_readers > 0) { if (ts != NULL) @@ -122,12 +139,14 @@ int pthread_rwlock_timedwrlock(FAR pthread_rwlock_t *rw_lock, break; } } +#ifdef CONFIG_PTHREAD_CLEANUP + pthread_cleanup_pop(0); +#endif if (err == 0) { rw_lock->write_in_progress = true; } - else { /* In case of error, notify any blocked readers. */ From b51b72b2dba6dc7350cddfa40ca93203ebbc6932 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 10 Apr 2017 08:11:16 -0600 Subject: [PATCH 2/3] pthreads: Re-order some operations so that mutexes are placed in the inconsistent state BEFORE the clean-up callbacks are called. --- sched/Kconfig | 4 ++-- sched/pthread/pthread_cancel.c | 12 ++++++------ sched/pthread/pthread_exit.c | 12 ++++++------ sched/pthread/pthread_mutexconsistent.c | 4 +++- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/sched/Kconfig b/sched/Kconfig index 50613fb44d..6cefe0e121 100644 --- a/sched/Kconfig +++ b/sched/Kconfig @@ -601,8 +601,6 @@ config PTHREAD_CLEANUP_STACKSIZE 8 for a CPU with 32-bit addressing and 4 for a CPU with 16-bit addressing. -endmenu # Pthread Options - config CANCELLATION_POINTS bool "Cancellation points" default n @@ -611,6 +609,8 @@ config CANCELLATION_POINTS cancellation points will also used with the () task_delete() API even if pthreads are not enabled. +endmenu # Pthread Options + menu "Performance Monitoring" config SCHED_CPULOAD diff --git a/sched/pthread/pthread_cancel.c b/sched/pthread/pthread_cancel.c index e2cad60ebb..f68d272e03 100644 --- a/sched/pthread/pthread_cancel.c +++ b/sched/pthread/pthread_cancel.c @@ -145,6 +145,12 @@ int pthread_cancel(pthread_t thread) pthread_exit(PTHREAD_CANCELED); } +#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE + /* Recover any mutexes still held by the canceled thread */ + + pthread_mutex_inconsistent(tcb); +#endif + #ifdef CONFIG_PTHREAD_CLEANUP /* Perform any stack pthread clean-up callbacks. * @@ -162,12 +168,6 @@ int pthread_cancel(pthread_t thread) (void)pthread_completejoin((pid_t)thread, PTHREAD_CANCELED); -#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE - /* Recover any mutexes still held by the canceled thread */ - - pthread_mutex_inconsistent(tcb); -#endif - /* Then let task_terminate do the real work */ return task_terminate((pid_t)thread, false); diff --git a/sched/pthread/pthread_exit.c b/sched/pthread/pthread_exit.c index bcc8e79c85..4929db94cb 100644 --- a/sched/pthread/pthread_exit.c +++ b/sched/pthread/pthread_exit.c @@ -105,6 +105,12 @@ void pthread_exit(FAR void *exit_value) tcb->cpcount = 0; #endif +#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE + /* Recover any mutexes still held by the canceled thread */ + + pthread_mutex_inconsistent((FAR struct pthread_tcb_s *)tcb); +#endif + #ifdef CONFIG_PTHREAD_CLEANUP /* Perform any stack pthread clean-up callbacks */ @@ -123,12 +129,6 @@ void pthread_exit(FAR void *exit_value) exit(EXIT_FAILURE); } -#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE - /* Recover any mutexes still held by the canceled thread */ - - pthread_mutex_inconsistent((FAR struct pthread_tcb_s *)tcb); -#endif - /* Perform common task termination logic. This will get called again later * through logic kicked off by _exit(). However, we need to call it before * calling _exit() in order certain operations if this is the last thread diff --git a/sched/pthread/pthread_mutexconsistent.c b/sched/pthread/pthread_mutexconsistent.c index 77d9bede3e..e374083021 100644 --- a/sched/pthread/pthread_mutexconsistent.c +++ b/sched/pthread/pthread_mutexconsistent.c @@ -119,7 +119,6 @@ int pthread_mutex_consistent(FAR pthread_mutex_t *mutex) /* The thread associated with the PID no longer exists */ mutex->pid = -1; - mutex->flags &= _PTHREAD_MFLAGS_ROBUST; #ifdef CONFIG_PTHREAD_MUTEX_TYPES mutex->nlocks = 0; #endif @@ -132,6 +131,9 @@ int pthread_mutex_consistent(FAR pthread_mutex_t *mutex) } } + /* Clear the inconsistent flag in any case */ + + mutex->flags &= _PTHREAD_MFLAGS_ROBUST; sched_unlock(); ret = OK; } From 84849cfc5ec7b74bab3c7a21b2babed14c89e37d Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 10 Apr 2017 08:44:08 -0600 Subject: [PATCH 3/3] examples/ostest: pthread rwlock cleanup handlers must call pthread_consistent, not pthread_mutex_unlock() on cancellation if robust mutexes are enabled. --- libc/pthread/pthread_rwlock_rdlock.c | 13 ++++++++++++- libc/pthread/pthread_rwlock_wrlock.c | 14 +++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/libc/pthread/pthread_rwlock_rdlock.c b/libc/pthread/pthread_rwlock_rdlock.c index 19979f79bb..29dcf7daae 100644 --- a/libc/pthread/pthread_rwlock_rdlock.c +++ b/libc/pthread/pthread_rwlock_rdlock.c @@ -55,7 +55,18 @@ static void rdlock_cleanup(FAR void *arg) { FAR pthread_rwlock_t *rw_lock = (FAR pthread_rwlock_t *)arg; - pthread_mutex_unlock(&rw_lock->lock); +#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE + /* Check if this is a robust mutex in an inconsistent state */ + + if ((rw_lock->lock.flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0) + { + (void)pthread_mutex_consistent(&rw_lock->lock); + } + else +#endif + { + (void)pthread_mutex_unlock(&rw_lock->lock); + } } #endif diff --git a/libc/pthread/pthread_rwlock_wrlock.c b/libc/pthread/pthread_rwlock_wrlock.c index fcda35cb4f..93a6a23858 100644 --- a/libc/pthread/pthread_rwlock_wrlock.c +++ b/libc/pthread/pthread_rwlock_wrlock.c @@ -56,7 +56,19 @@ static void wrlock_cleanup(FAR void *arg) FAR pthread_rwlock_t *rw_lock = (FAR pthread_rwlock_t *)arg; rw_lock->num_writers--; - pthread_mutex_unlock(&rw_lock->lock); + +#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE + /* Check if this is a robust mutex in an inconsistent state */ + + if ((rw_lock->lock.flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0) + { + (void)pthread_mutex_consistent(&rw_lock->lock); + } + else +#endif + { + (void)pthread_mutex_unlock(&rw_lock->lock); + } } #endif