diff --git a/TODO b/TODO index 62d2e562ce..f770f9a2f6 100644 --- a/TODO +++ b/TODO @@ -216,7 +216,7 @@ o Task/Scheduler (sched/) Status: Open Priority: Medium-ish - Title: ISSUES WITH PRIORITY INHERITANCE WHEN SEMAPHORE USED AS IPC + Title: ISSUES WITH PRIORITY INHERITANCE WHEN SEMAPHORE/MUTX IS USED AS IPC Description: Semaphores have multiple uses. The typical usage is where the semaphore is used as lock on one or more resources. In this typical case, priority inheritance works perfectly: The @@ -264,6 +264,20 @@ o Task/Scheduler (sched/) The fix is to call sem_setprotocol(SEM_PRIO_NONE) immediately after the sem_init() call so that there will be no priority inheritance operations on this semaphore used for signalling. + + NOTE also that in NuttX, pthread mutexes are build on top of + binary semaphores. As a result, the above recommendation also + applies when pthread mutexes are used for inter-thread + signaling. That is, a mutex that is used for signaling should + be initialize like this (simplified, no error checking here): + + pthread_mutexattr_t attr; + pthread_mutex_t mutex; + + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_PRIO_NONE); + pthread_mutex_init(&mutex, &attr); + Status: Open Priority: High. If you have priority inheritance enabled and you use semaphores for signalling events, then you *must* call diff --git a/arch/sim/src/up_uartwait.c b/arch/sim/src/up_uartwait.c index 50f6b5f281..681e87dcdd 100644 --- a/arch/sim/src/up_uartwait.c +++ b/arch/sim/src/up_uartwait.c @@ -39,7 +39,7 @@ #include -#include +#include #include "up_internal.h" diff --git a/include/pthread.h b/include/pthread.h index 1c368d737b..ac495e44c2 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -96,12 +96,10 @@ * An implementation is allowed to map this mutex to one of the other mutex types. */ -#ifdef CONFIG_MUTEX_TYPES -# define PTHREAD_MUTEX_NORMAL 0 -# define PTHREAD_MUTEX_ERRORCHECK 1 -# define PTHREAD_MUTEX_RECURSIVE 2 -# define PTHREAD_MUTEX_DEFAULT PTHREAD_MUTEX_NORMAL -#endif +#define PTHREAD_MUTEX_NORMAL 0 +#define PTHREAD_MUTEX_ERRORCHECK 1 +#define PTHREAD_MUTEX_RECURSIVE 2 +#define PTHREAD_MUTEX_DEFAULT PTHREAD_MUTEX_NORMAL /* Valid ranges for the pthread stacksize attribute */ @@ -389,10 +387,12 @@ int pthread_mutexattr_getpshared(FAR const pthread_mutexattr_t *attr, FAR int *pshared); int pthread_mutexattr_setpshared(FAR pthread_mutexattr_t *attr, int pshared); -#ifdef CONFIG_MUTEX_TYPES int pthread_mutexattr_gettype(const pthread_mutexattr_t *attr, int *type); int pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type); -#endif +int pthread_mutexattr_getprotocol(FAR const pthread_mutexattr_t *attr, + FAR int *protocol); +int pthread_mutexattr_setprotocol(FAR pthread_mutexattr_t *attr, + int protocol); /* The following routines create, delete, lock and unlock mutexes. */ @@ -403,15 +403,6 @@ int pthread_mutex_lock(FAR pthread_mutex_t *mutex); int pthread_mutex_trylock(FAR pthread_mutex_t *mutex); int pthread_mutex_unlock(FAR pthread_mutex_t *mutex); -#ifdef CONFIG_PRIORITY_INHERITANCE -/* Manage priority inheritance */ - -int pthread_mutexattr_getprotocol(FAR const pthread_mutexattr_t *attr, - FAR int *protocol); -int pthread_mutexattr_setprotocol(FAR pthread_mutexattr_t *attr, - int protocol); -#endif - /* Operations on condition variables */ int pthread_condattr_init(FAR pthread_condattr_t *attr); diff --git a/libc/pthread/Make.defs b/libc/pthread/Make.defs index 9d343f6052..bbb3280959 100644 --- a/libc/pthread/Make.defs +++ b/libc/pthread/Make.defs @@ -35,27 +35,24 @@ # Add the pthread C files to the build -CSRCS += pthread_attr_init.c pthread_attr_destroy.c \ - pthread_attr_setschedpolicy.c pthread_attr_getschedpolicy.c \ - pthread_attr_setinheritsched.c pthread_attr_getinheritsched.c \ - pthread_attr_setstacksize.c pthread_attr_getstacksize.c \ - pthread_attr_setschedparam.c pthread_attr_getschedparam.c \ - pthread_barrierattr_init.c pthread_barrierattr_destroy.c \ - pthread_barrierattr_getpshared.c pthread_barrierattr_setpshared.c \ - pthread_condattr_init.c pthread_condattr_destroy.c \ - pthread_mutexattr_init.c pthread_mutexattr_destroy.c \ - pthread_mutexattr_getpshared.c pthread_mutexattr_setpshared.c +CSRCS += pthread_attr_init.c pthread_attr_destroy.c +CSRCS += pthread_attr_setschedpolicy.c pthread_attr_getschedpolicy.c +CSRCS += pthread_attr_setinheritsched.c pthread_attr_getinheritsched.c +CSRCS += pthread_attr_setstacksize.c pthread_attr_getstacksize.c +CSRCS += pthread_attr_setschedparam.c pthread_attr_getschedparam.c +CSRCS += pthread_barrierattr_init.c pthread_barrierattr_destroy.c +CSRCS += pthread_barrierattr_getpshared.c pthread_barrierattr_setpshared.c +CSRCS += pthread_condattr_init.c pthread_condattr_destroy.c +CSRCS += pthread_mutexattr_init.c pthread_mutexattr_destroy.c +CSRCS += pthread_mutexattr_getpshared.c pthread_mutexattr_setpshared.c +CSRCS += pthread_mutexattr_setprotocol.c pthread_mutexattr_getprotocol.c +CSRCS += pthread_mutexattr_settype.c pthread_mutexattr_gettype.c ifeq ($(CONFIG_SMP),y) CSRCS += pthread_attr_getaffinity.c pthread_attr_setaffinity.c endif ifeq ($(CONFIG_MUTEX_TYPES),y) -CSRCS += pthread_mutexattr_settype.c pthread_mutexattr_gettype.c -endif - -ifeq ($(CONFIG_PRIORITY_INHERITANCE),y) -CSRCS += pthread_mutexattr_setprotocol.c pthread_mutexattr_getprotocol.c endif ifeq ($(CONFIG_BUILD_PROTECTED),y) diff --git a/libc/pthread/pthread_mutexattr_getprotocol.c b/libc/pthread/pthread_mutexattr_getprotocol.c index f7851855ac..f803006bb2 100644 --- a/libc/pthread/pthread_mutexattr_getprotocol.c +++ b/libc/pthread/pthread_mutexattr_getprotocol.c @@ -68,6 +68,11 @@ int pthread_mutexattr_getprotocol(FAR const pthread_mutexattr_t *attr, { DEBUGASSERT(attr != NULL && protocol != NULL); +#ifdef CONFIG_PRIORITY_INHERITANCE linfo("Returning %d\n", attr->proto); return attr->proto; +#else + linfo("Returning %d\n", PTHREAD_PRIO_NONE); + return PTHREAD_PRIO_NONE; +#endif } diff --git a/libc/pthread/pthread_mutexattr_gettype.c b/libc/pthread/pthread_mutexattr_gettype.c index c9703f5c01..9f057ae11d 100644 --- a/libc/pthread/pthread_mutexattr_gettype.c +++ b/libc/pthread/pthread_mutexattr_gettype.c @@ -41,8 +41,6 @@ #include #include -#ifdef CONFIG_MUTEX_TYPES - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -67,13 +65,15 @@ int pthread_mutexattr_gettype(const pthread_mutexattr_t *attr, int *type) { - if (attr && type) + if (attr != NULL && type != NULL) { +#ifdef CONFIG_MUTEX_TYPES *type = attr->type; +#else + *type = PTHREAD_MUTEX_NORMAL; +#endif return 0; } return EINVAL; } - -#endif /* CONFIG_MUTEX_TYPES */ diff --git a/libc/pthread/pthread_mutexattr_setprotocol.c b/libc/pthread/pthread_mutexattr_setprotocol.c index 621a94d1c1..0d128e0480 100644 --- a/libc/pthread/pthread_mutexattr_setprotocol.c +++ b/libc/pthread/pthread_mutexattr_setprotocol.c @@ -69,6 +69,7 @@ int pthread_mutexattr_setprotocol(FAR pthread_mutexattr_t *attr, linfo("attr=0x%p protocol=%d\n", attr, protocol); DEBUGASSERT(attr != NULL); +#ifdef CONFIG_PRIORITY_INHERITANCE if (protocol >= PTHREAD_PRIO_NONE && protocol <= PTHREAD_PRIO_PROTECT) { attr->proto = protocol; @@ -76,4 +77,14 @@ int pthread_mutexattr_setprotocol(FAR pthread_mutexattr_t *attr, } return EINVAL; + +#else + if (protocol == PTHREAD_PRIO_NONE) + { + return OK; + } + + return ENOSYS; +#endif + } diff --git a/libc/pthread/pthread_mutexattr_settype.c b/libc/pthread/pthread_mutexattr_settype.c index 9922a34355..b43e86fc12 100644 --- a/libc/pthread/pthread_mutexattr_settype.c +++ b/libc/pthread/pthread_mutexattr_settype.c @@ -41,8 +41,6 @@ #include #include -#ifdef CONFIG_MUTEX_TYPES - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -69,10 +67,17 @@ int pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type) { if (attr && type >= PTHREAD_MUTEX_NORMAL && type <= PTHREAD_MUTEX_RECURSIVE) { +#ifdef CONFIG_MUTEX_TYPES attr->type = type; +#else + if (type != PTHREAD_MUTEX_NORMAL) + { + return ENOSYS; + } +#endif + return OK; } + return EINVAL; } - -#endif /* CONFIG_MUTEX_TYPES */ diff --git a/sched/pthread/pthread_condinit.c b/sched/pthread/pthread_condinit.c index 73a6423c69..7368e0fb5a 100644 --- a/sched/pthread/pthread_condinit.c +++ b/sched/pthread/pthread_condinit.c @@ -1,7 +1,7 @@ /**************************************************************************** * sched/pthread/pthread_condinit.c * - * Copyright (C) 2007-2009 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2009, 2016 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -40,9 +40,12 @@ #include #include +#include #include #include +#include + #include "pthread/pthread.h" /**************************************************************************** @@ -71,23 +74,28 @@ int pthread_cond_init(FAR pthread_cond_t *cond, FAR const pthread_condattr_t *at sinfo("cond=0x%p attr=0x%p\n", cond, attr); - if (!cond) + if (cond == NULL) { ret = EINVAL; } - /* Initialize the semaphore contained in the condition structure - * with initial count = 0 + /* Initialize the semaphore contained in the condition structure with + * initial count = 0 */ else if (sem_init((FAR sem_t *)&cond->sem, 0, 0) != OK) { ret = EINVAL; } + else + { + /* The contained semaphore is used for signaling and, hence, should + * not have priority inheritance enabled. + */ + + sem_setprotocol(&cond->sem, SEM_PRIO_NONE); + } sinfo("Returning %d\n", ret); return ret; } - - -