On 2013-08-14 02:21, Chris Johns wrote:
Module: rtems
Branch: master
Commit: 03acc5915e002f0b03eee9e86212209705cca6d6
Changeset:
http://git.rtems.org/rtems/commit/?id=03acc5915e002f0b03eee9e86212209705cca6d6
Author: Chris Johns <chr...@rtems.org>
Date: Wed Aug 14 10:21:41 2013 +1000
posix: Change pthread_once to be SMP safe.
Change pthread_once from using disabled pre-emption to using a
pthread mutex making it SMP safe. GCC using a posix threading
model uses pthread_once.
The pthread mutex requires at least 1 mutex is configured so
confdefs.h has been updated to account for the internal
mutex.
---
cpukit/posix/Makefile.am | 6 ++-
cpukit/posix/include/rtems/posix/onceimpl.h | 57 +++++++++++++++++++++++++++
cpukit/posix/preinstall.am | 4 ++
cpukit/posix/src/once.c | 49 +++++++++++++++++++++++
cpukit/posix/src/pthreadonce.c | 50 +++++++++++++++++++-----
cpukit/sapi/include/confdefs.h | 7 +++
cpukit/sapi/src/posixapi.c | 2 +
7 files changed, 164 insertions(+), 11 deletions(-)
[...]
diff --git a/cpukit/posix/include/rtems/posix/onceimpl.h
b/cpukit/posix/include/rtems/posix/onceimpl.h
new file mode 100644
index 0000000..f023810
--- /dev/null
+++ b/cpukit/posix/include/rtems/posix/onceimpl.h
@@ -0,0 +1,57 @@
+/**
+ * @file
+ *
+ * @brief Private Inlined Routines for POSIX Once
+ *
+ * This include file contains the static inline implementation of the private
+ * inlined routines for POSIX once.
+ */
+
+/*
+ * COPYRIGHT (c) 1989-1999.
+ * On-Line Applications Research Corporation (OAR).
+ *
+ * COPYRIGHT (c) 2013.
+ * Chris Johns <chr...@rtems.org>
+ *
+ * The license and distribution terms for this file may be
+ * found in the file LICENSE in this distribution or at
+ * http://www.rtems.com/license/LICENSE.
+ */
+
+#include <rtems/score/objectimpl.h>
+#include <rtems/score/percpu.h>
+
+#ifndef _RTEMS_POSIX_ONCEIMPL_H
+#define _RTEMS_POSIX_ONCEMPL_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @addtogroup POSIX_ONCE
+ *
+ * @{
+ */
+
+/**
+ * @brief Lock to allow once mutex's to be initialized.
+ */
+POSIX_EXTERN pthread_mutex_t _POSIX_Once_Lock;
The POSIX_EXTERN in combination with the definition in once.c is wrong.
+
+/**
+ * @brief POSIX once manager initialization.
+ *
+ * This routine performs the initialization necessary for this manager.
+ */
+void _POSIX_Once_Manager_initialization(void);
+
+/** @} */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
+/* end of include file */
diff --git a/cpukit/posix/preinstall.am b/cpukit/posix/preinstall.am
index 093a9df..d0e238c 100644
--- a/cpukit/posix/preinstall.am
+++ b/cpukit/posix/preinstall.am
@@ -84,6 +84,10 @@ $(PROJECT_INCLUDE)/rtems/posix/muteximpl.h:
include/rtems/posix/muteximpl.h $(PR
$(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/posix/muteximpl.h
PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/posix/muteximpl.h
+$(PROJECT_INCLUDE)/rtems/posix/onceimpl.h: include/rtems/posix/onceimpl.h
$(PROJECT_INCLUDE)/rtems/posix/$(dirstamp)
+ $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/posix/onceimpl.h
+PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/posix/onceimpl.h
+
$(PROJECT_INCLUDE)/rtems/posix/posixapi.h: include/rtems/posix/posixapi.h
$(PROJECT_INCLUDE)/rtems/posix/$(dirstamp)
$(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/posix/posixapi.h
PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/posix/posixapi.h
diff --git a/cpukit/posix/src/once.c b/cpukit/posix/src/once.c
new file mode 100644
index 0000000..d77c3c1
--- /dev/null
+++ b/cpukit/posix/src/once.c
@@ -0,0 +1,49 @@
+/**
+ * @file
+ *
+ * @brief POSIX Once Manager Initialization
+ * @ingroup POSIX_ONCE POSIX Once Support
+ */
+
+/*
+ * COPYRIGHT (c) 2013
+ * Chris Johns <chr...@rtems.org>
+ *
+ * The license and distribution terms for this file may be
+ * found in the file LICENSE in this distribution or at
+ * http://www.rtems.com/license/LICENSE.
+ */
+
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <pthread.h>
+#include <errno.h>
+
+#include <rtems.h>
+#include <rtems/posix/onceimpl.h>
+
+pthread_mutex_t _POSIX_Once_Lock;
+
+void _POSIX_Once_Manager_initialization(void)
+{
+ pthread_mutexattr_t mattr;
+ int r;
I would name this "eno" since the status is encoded as an errno number.
+
+ _POSIX_Once_Lock = PTHREAD_MUTEX_INITIALIZER;
+
+ r = pthread_mutexattr_init( &mattr );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa0000 | r );
Errors in the pthread_mutexattr_init(), pthread_mutexattr_settype(), and
pthread_mutexattr_destroy() are pure programming errors, so I should use
_Assert() here.
+
+ r = pthread_mutexattr_settype( &mattr, PTHREAD_MUTEX_RECURSIVE );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa1000 | r );
+
+ r = pthread_mutex_init( &_POSIX_Once_Lock, &mattr );
+ if ( r != 0 )
+ rtems_fatal( RTEMS_FATAL_SOURCE_ASSERT, 0x80aa2000 | r );
This is a configuration error. We need a proper error code for this with
source INTERNAL_ERROR_POSIX_API. We need also a new test for this fatal error,
e.g. psxtests/psxfatal03.
+
+ pthread_mutexattr_destroy( &mattr );
Check return value with _Assert()?
+}
diff --git a/cpukit/posix/src/pthreadonce.c b/cpukit/posix/src/pthreadonce.c
index 0700a22..2b02f1e 100644
--- a/cpukit/posix/src/pthreadonce.c
+++ b/cpukit/posix/src/pthreadonce.c
@@ -25,25 +25,55 @@
#include <rtems.h>
#include <rtems/system.h>
-#include <rtems/score/thread.h>
+#include <rtems/posix/onceimpl.h>
+
+#define PTHREAD_ONCE_INIT_NOT_RUN 0
+#define PTHREAD_ONCE_INIT_RUNNING 1
+#define PTHREAD_ONCE_INIT_RUN 2
I would rename PTHREAD_ONCE_INIT_RUN to PTHREAD_ONCE_INIT_COMPLETE to avoid too
many run combinations.
int pthread_once(
pthread_once_t *once_control,
void (*init_routine)(void)
)
{
+ int r = 0;
+
if ( !once_control || !init_routine )
return EINVAL;
- if ( !once_control->init_executed ) {
- rtems_mode saveMode;
- rtems_task_mode(RTEMS_NO_PREEMPT, RTEMS_PREEMPT_MASK, &saveMode);
- if ( !once_control->init_executed ) {
- once_control->is_initialized = true;
- once_control->init_executed = true;
- (*init_routine)();
+ if ( once_control->is_initialized != 1 )
+ return EINVAL;
+
+ if ( once_control->init_executed != PTHREAD_ONCE_INIT_RUN ) {
+ r = pthread_mutex_lock( &_POSIX_Once_Lock );
You get an error here,
1. a pure programming error, e.g. call of this function before
_POSIX_Once_Manager_initialization(), or
2. a corrupt system.
I would simply use _Assert() here.
+ if ( r == 0 ) {
+ int rr;
+
+ /*
+ * Getting to here means the once_control is locked so we have:
+ * 1. The init has not run and the state is PTHREAD_ONCE_INIT_NOT_RUN.
+ * 2. The init has finished and the state is PTHREAD_ONCE_INIT_RUN.
+ * 3. The init is being run by this thread and the state
+ * PTHREAD_ONCE_INIT_RUNNING so we are nesting. This is an error.
+ */
+
+ switch ( once_control->init_executed ) {
+ case PTHREAD_ONCE_INIT_NOT_RUN:
+ once_control->init_executed = PTHREAD_ONCE_INIT_RUNNING;
+ (*init_routine)();
+ once_control->init_executed = PTHREAD_ONCE_INIT_RUN;
+ break;
+ case PTHREAD_ONCE_INIT_RUNNING:
+ r = EINVAL;
+ break;
+ default:
+ break;
+ }
+ rr = pthread_mutex_unlock( &_POSIX_Once_Lock );
+ if ( r == 0 )
+ r = rr;
}
- rtems_task_mode(saveMode, RTEMS_PREEMPT_MASK, &saveMode);
}
- return 0;
+
+ return r;
}
[...]
I think the nesting case is not covered by a test.
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
