[PATCH 11/12] cpukit/libblock: Workaround SMP problem in bdbuf
Ralf Kirchner
ralf.kirchner at embedded-brains.de
Tue May 27 14:48:51 UTC 2014
Enabling and disabling preemption as done for single core will not work for SMP.
Thus as a temporary workaround use POSIX mutexes and POSIX condition variables for SMP instead of the combination of semaphores and preemption handling used for single core.
---
cpukit/libblock/src/bdbuf.c | 348 +++++++++++++++++++++++++++++++++++--------
1 Datei geändert, 282 Zeilen hinzugefügt(+), 66 Zeilen entfernt(-)
diff --git a/cpukit/libblock/src/bdbuf.c b/cpukit/libblock/src/bdbuf.c
index 1e4887c..0caf8f5 100644
--- a/cpukit/libblock/src/bdbuf.c
+++ b/cpukit/libblock/src/bdbuf.c
@@ -43,6 +43,23 @@
#include "rtems/bdbuf.h"
+#if defined( RTEMS_SMP )
+ #if defined( RTEMS_POSIX_API )
+ /* The single core implementation with enabling and disabling preemption
+ * will not work under SMP. Thus we need to use POSIX mutexes and POSIX
+ * condition variables as a workaround
+ * Required POSIX mutexes and POSIX condition variables will be allocated
+ * automatically in confdefs.h if RTEMS_SMP and RTEMS_POSIX_API
+ * are #defined
+ */
+ #define RTEMS_BDBUF_SMP_WORKAROUND
+ #endif /* defined( RTEMS_POSIX_API ) */
+#endif /* defined( RTEMS_SMP ) */
+
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ #include <rtems/posix/priorityimpl.h>
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
+
#define BDBUF_INVALID_DEV NULL
/*
@@ -77,12 +94,22 @@ typedef struct rtems_bdbuf_swapout_worker
* thread. */
} rtems_bdbuf_swapout_worker;
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ typedef pthread_mutex_t rtems_bdbuf_lock_type;
+#else
+ typedef rtems_id rtems_bdbuf_lock_type;
+#endif /* defined( BDBUF_SMP_WORKAROUND ) */
+
/**
* Buffer waiters synchronization.
*/
typedef struct rtems_bdbuf_waiters {
- unsigned count;
- rtems_id sema;
+ unsigned count;
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ pthread_cond_t cond_var;
+#else
+ rtems_id sema;
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
} rtems_bdbuf_waiters;
/**
@@ -106,9 +133,9 @@ typedef struct rtems_bdbuf_cache
* buffer size that fit in a group. */
uint32_t flags; /**< Configuration flags. */
- rtems_id lock; /**< The cache lock. It locks all
+ rtems_bdbuf_lock_type lock; /**< The cache lock. It locks all
* cache data, BD and lists. */
- rtems_id sync_lock; /**< Sync calls block writes. */
+ rtems_bdbuf_lock_type sync_lock; /**< Sync calls block writes. */
bool sync_active; /**< True if a sync is active. */
rtems_id sync_requester; /**< The sync requester. */
rtems_disk_device *sync_device; /**< The device to sync and
@@ -231,6 +258,138 @@ static rtems_task rtems_bdbuf_read_ahead_task(rtems_task_argument arg);
*/
static rtems_bdbuf_cache bdbuf_cache;
+static rtems_status_code rtems_bdbuf_lock_create(
+ rtems_name name,
+ uint32_t count,
+ rtems_attribute attribute_set,
+ rtems_task_priority priority_ceiling,
+ rtems_bdbuf_lock_type *lock
+)
+{
+ rtems_status_code sc = RTEMS_SUCCESSFUL;
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ {
+ int eno;
+ pthread_mutexattr_t attr;
+
+ (void)name;
+ (void)priority_ceiling;
+
+ eno = pthread_mutexattr_init (&attr);
+ if (eno == 0) {
+ if ((attribute_set & RTEMS_INHERIT_PRIORITY) != 0) {
+ eno = pthread_mutexattr_setprotocol (
+ &attr,
+ PTHREAD_PRIO_INHERIT
+ );
+ }
+ if (eno == 0) {
+ eno = pthread_mutexattr_settype (
+ &attr,
+ PTHREAD_MUTEX_RECURSIVE
+ );
+ }
+ if (eno == 0) {
+ eno = pthread_mutex_init (lock, &attr);
+ }
+ if (eno == 0) {
+ if (count == 0) {
+ eno = pthread_mutex_lock (lock);
+ if (eno == EBUSY) {
+ eno = 0;
+ }
+ }
+ }
+ (void)pthread_mutexattr_destroy (&attr);
+ }
+
+ if (eno != 0) {
+ sc = RTEMS_UNSATISFIED;
+ }
+ }
+#else
+ sc = rtems_semaphore_create(
+ name,
+ count,
+ attribute_set,
+ priority_ceiling,
+ lock
+ );
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
+ return sc;
+}
+
+
+static rtems_status_code rtems_bdbuf_lock_delete(
+ rtems_bdbuf_lock_type *lock
+)
+{
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ {
+ int eno = pthread_mutex_destroy (lock);
+ if (eno != 0) {
+ return RTEMS_UNSATISFIED;
+ } else {
+ return RTEMS_SUCCESSFUL;
+ }
+ }
+#else
+ return rtems_semaphore_delete( *lock );
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
+}
+
+static rtems_status_code rtems_bdbuf_waiter_create(
+ rtems_name name,
+ uint32_t count,
+ rtems_attribute attribute_set,
+ rtems_task_priority priority_ceiling,
+ rtems_bdbuf_waiters *waiter
+)
+{
+ rtems_status_code sc = RTEMS_SUCCESSFUL;
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ {
+ int eno = pthread_cond_init (&waiter->cond_var, NULL);
+
+ if (eno != 0) {
+ sc = RTEMS_UNSATISFIED;
+ }
+ }
+#else
+ sc = rtems_semaphore_create(
+ name,
+ count,
+ attribute_set,
+ priority_ceiling,
+ &waiter->sema
+ );
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
+ return sc;
+}
+
+static rtems_status_code rtems_bdbuf_waiter_delete(
+ rtems_bdbuf_waiters *waiter
+)
+{
+ rtems_status_code sc = RTEMS_SUCCESSFUL;
+
+#if defined( RTEMS_BDBUF_SMP_WORKAROUND )
+ {
+ int eno = pthread_cond_destroy (&waiter->cond_var);
+
+ if (eno != 0) {
+ sc = RTEMS_UNSATISFIED;
+ }
+ }
+#else
+ sc = rtems_semaphore_delete(
+ waiter->sema
+ );
+#endif /* defined( RTEMS_BDBUF_SMP_WORKAROUND ) */
+
+ return sc;
+}
+
#if RTEMS_BDBUF_TRACE
/**
* If true output the trace message.
@@ -845,13 +1004,28 @@ rtems_bdbuf_media_block (const rtems_disk_device *dd, rtems_blkdev_bnum block)
* @param fatal_error_code The error code if the call fails.
*/
static void
-rtems_bdbuf_lock (rtems_id lock, uint32_t fatal_error_code)
+rtems_bdbuf_lock (rtems_bdbuf_lock_type *lock, uint32_t fatal_error_code)
{
- rtems_status_code sc = rtems_semaphore_obtain (lock,
- RTEMS_WAIT,
- RTEMS_NO_TIMEOUT);
- if (sc != RTEMS_SUCCESSFUL)
- rtems_bdbuf_fatal (fatal_error_code);
+#ifndef RTEMS_BDBUF_SMP_WORKAROUND
+ {
+ rtems_status_code sc = rtems_semaphore_obtain (*lock,
+ RTEMS_WAIT,
+ RTEMS_NO_TIMEOUT);
+ if (sc != RTEMS_SUCCESSFUL)
+ rtems_bdbuf_fatal (fatal_error_code);
+ }
+#else
+ {
+ int eno = pthread_mutex_lock (lock);
+ if (eno == EBUSY) {
+ eno = 0;
+ }
+
+ if (eno != 0) {
+ rtems_bdbuf_fatal (fatal_error_code);
+ }
+ }
+#endif /* #ifndef RTEMS_BDBUF_SMP_WORKAROUND */
}
/**
@@ -861,11 +1035,23 @@ rtems_bdbuf_lock (rtems_id lock, uint32_t fatal_error_code)
* @param fatal_error_code The error code if the call fails.
*/
static void
-rtems_bdbuf_unlock (rtems_id lock, uint32_t fatal_error_code)
+rtems_bdbuf_unlock (rtems_bdbuf_lock_type *lock, uint32_t fatal_error_code)
{
- rtems_status_code sc = rtems_semaphore_release (lock);
- if (sc != RTEMS_SUCCESSFUL)
- rtems_bdbuf_fatal (fatal_error_code);
+#ifndef RTEMS_BDBUF_SMP_WORKAROUND
+ {
+ rtems_status_code sc = rtems_semaphore_release (*lock);
+ if (sc != RTEMS_SUCCESSFUL)
+ rtems_bdbuf_fatal (fatal_error_code);
+ }
+#else
+ {
+ int eno = pthread_mutex_unlock (lock);
+
+ if (eno != 0) {
+ rtems_bdbuf_fatal (fatal_error_code);
+ }
+ }
+#endif /* #ifndef RTEMS_BDBUF_SMP_WORKAROUND */
}
/**
@@ -874,7 +1060,7 @@ rtems_bdbuf_unlock (rtems_id lock, uint32_t fatal_error_code)
static void
rtems_bdbuf_lock_cache (void)
{
- rtems_bdbuf_lock (bdbuf_cache.lock, RTEMS_BDBUF_FATAL_CACHE_LOCK);
+ rtems_bdbuf_lock (&bdbuf_cache.lock, RTEMS_BDBUF_FATAL_CACHE_LOCK);
}
/**
@@ -883,7 +1069,7 @@ rtems_bdbuf_lock_cache (void)
static void
rtems_bdbuf_unlock_cache (void)
{
- rtems_bdbuf_unlock (bdbuf_cache.lock, RTEMS_BDBUF_FATAL_CACHE_UNLOCK);
+ rtems_bdbuf_unlock (&bdbuf_cache.lock, RTEMS_BDBUF_FATAL_CACHE_UNLOCK);
}
/**
@@ -892,7 +1078,7 @@ rtems_bdbuf_unlock_cache (void)
static void
rtems_bdbuf_lock_sync (void)
{
- rtems_bdbuf_lock (bdbuf_cache.sync_lock, RTEMS_BDBUF_FATAL_SYNC_LOCK);
+ rtems_bdbuf_lock (&bdbuf_cache.sync_lock, RTEMS_BDBUF_FATAL_SYNC_LOCK);
}
/**
@@ -901,7 +1087,7 @@ rtems_bdbuf_lock_sync (void)
static void
rtems_bdbuf_unlock_sync (void)
{
- rtems_bdbuf_unlock (bdbuf_cache.sync_lock,
+ rtems_bdbuf_unlock (&bdbuf_cache.sync_lock,
RTEMS_BDBUF_FATAL_SYNC_UNLOCK);
}
@@ -917,6 +1103,7 @@ rtems_bdbuf_group_release (rtems_bdbuf_buffer *bd)
--bd->group->users;
}
+#ifndef RTEMS_BDBUF_SMP_WORKAROUND
static rtems_mode
rtems_bdbuf_disable_preemption (void)
{
@@ -939,6 +1126,7 @@ rtems_bdbuf_restore_preemption (rtems_mode prev_mode)
if (sc != RTEMS_SUCCESSFUL)
rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_PREEMPT_RST);
}
+#endif /* #ifndef RTEMS_BDBUF_SMP_WORKAROUND */
/**
* Wait until woken. Semaphores are used so a number of tasks can wait and can
@@ -958,42 +1146,56 @@ rtems_bdbuf_restore_preemption (rtems_mode prev_mode)
static void
rtems_bdbuf_anonymous_wait (rtems_bdbuf_waiters *waiters)
{
- rtems_status_code sc;
- rtems_mode prev_mode;
-
/*
* Indicate we are waiting.
*/
++waiters->count;
- /*
- * Disable preemption then unlock the cache and block. There is no POSIX
- * condition variable in the core API so this is a work around.
- *
- * The issue is a task could preempt after the cache is unlocked because it is
- * blocking or just hits that window, and before this task has blocked on the
- * semaphore. If the preempting task flushes the queue this task will not see
- * the flush and may block for ever or until another transaction flushes this
- * semaphore.
- */
- prev_mode = rtems_bdbuf_disable_preemption ();
+#ifndef RTEMS_BDBUF_SMP_WORKAROUND
+ {
+ rtems_status_code sc;
+ rtems_mode prev_mode;
+ /*
+ * Disable preemption then unlock the cache and block. There is no POSIX
+ * condition variable in the core API so this is a work around.
+ *
+ * The issue is a task could preempt after the cache is unlocked because it is
+ * blocking or just hits that window, and before this task has blocked on the
+ * semaphore. If the preempting task flushes the queue this task will not see
+ * the flush and may block for ever or until another transaction flushes this
+ * semaphore.
+ */
+ prev_mode = rtems_bdbuf_disable_preemption();
- /*
- * Unlock the cache, wait, and lock the cache when we return.
- */
- rtems_bdbuf_unlock_cache ();
+ /*
+ * Unlock the cache, wait, and lock the cache when we return.
+ */
+ rtems_bdbuf_unlock_cache ();
- sc = rtems_semaphore_obtain (waiters->sema, RTEMS_WAIT, RTEMS_BDBUF_WAIT_TIMEOUT);
+ sc = rtems_semaphore_obtain (waiters->sema, RTEMS_WAIT, RTEMS_BDBUF_WAIT_TIMEOUT);
- if (sc == RTEMS_TIMEOUT)
- rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAIT_TO);
+ if (sc == RTEMS_TIMEOUT)
+ rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAIT_TO);
- if (sc != RTEMS_UNSATISFIED)
- rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAIT_2);
+ if (sc != RTEMS_UNSATISFIED)
+ rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAIT_2);
- rtems_bdbuf_lock_cache ();
+ rtems_bdbuf_lock_cache ();
+
+ rtems_bdbuf_restore_preemption (prev_mode);
+ }
+#else
+ {
+ int eno = pthread_cond_wait (
+ &waiters->cond_var,
+ &bdbuf_cache.lock
+ );
- rtems_bdbuf_restore_preemption (prev_mode);
+ if (eno != 0) {
+ rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAIT_TO);
+ }
+ }
+#endif /* #ifndef RTEMS_BDBUF_SMP_WORKAROUND */
--waiters->count;
}
@@ -1013,13 +1215,23 @@ rtems_bdbuf_wait (rtems_bdbuf_buffer *bd, rtems_bdbuf_waiters *waiters)
* there are any waiters.
*/
static void
-rtems_bdbuf_wake (const rtems_bdbuf_waiters *waiters)
+rtems_bdbuf_wake (rtems_bdbuf_waiters *waiters)
{
rtems_status_code sc = RTEMS_SUCCESSFUL;
if (waiters->count > 0)
{
+#ifndef RTEMS_BDBUF_SMP_WORKAROUND
sc = rtems_semaphore_flush (waiters->sema);
+#else
+ {
+ int eno = pthread_cond_broadcast (&waiters->cond_var);
+
+ if (eno != 0) {
+ sc = RTEMS_UNSATISFIED;
+ }
+ }
+#endif /* #ifndef RTEMS_BDBUF_SMP_WORKAROUND */
if (sc != RTEMS_SUCCESSFUL)
rtems_bdbuf_fatal (RTEMS_BDBUF_FATAL_CACHE_WAKE);
}
@@ -1411,6 +1623,7 @@ rtems_bdbuf_init_once (void)
size_t b;
size_t cache_aligment;
rtems_status_code sc;
+ rtems_status_code sc_lock;
if (rtems_bdbuf_tracer)
printf ("bdbuf:init\n");
@@ -1453,35 +1666,38 @@ rtems_bdbuf_init_once (void)
/*
* Create the locks for the cache.
*/
- sc = rtems_semaphore_create (rtems_build_name ('B', 'D', 'C', 'l'),
- 1, RTEMS_BDBUF_CACHE_LOCK_ATTRIBS, 0,
- &bdbuf_cache.lock);
- if (sc != RTEMS_SUCCESSFUL)
+
+ sc_lock = rtems_bdbuf_lock_create (rtems_build_name ('B', 'D', 'C', 'l'),
+ 1, RTEMS_BDBUF_CACHE_LOCK_ATTRIBS, 0,
+ &bdbuf_cache.lock);
+ if (sc_lock != RTEMS_SUCCESSFUL) {
+ sc = sc_lock;
goto error;
+ }
rtems_bdbuf_lock_cache ();
- sc = rtems_semaphore_create (rtems_build_name ('B', 'D', 'C', 's'),
- 1, RTEMS_BDBUF_CACHE_LOCK_ATTRIBS, 0,
- &bdbuf_cache.sync_lock);
+ sc = rtems_bdbuf_lock_create (rtems_build_name ('B', 'D', 'C', 's'),
+ 1, RTEMS_BDBUF_CACHE_LOCK_ATTRIBS, 0,
+ &bdbuf_cache.sync_lock);
if (sc != RTEMS_SUCCESSFUL)
goto error;
- sc = rtems_semaphore_create (rtems_build_name ('B', 'D', 'C', 'a'),
- 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
- &bdbuf_cache.access_waiters.sema);
+ sc = rtems_bdbuf_waiter_create (rtems_build_name ('B', 'D', 'C', 'a'),
+ 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
+ &bdbuf_cache.access_waiters);
if (sc != RTEMS_SUCCESSFUL)
goto error;
- sc = rtems_semaphore_create (rtems_build_name ('B', 'D', 'C', 't'),
- 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
- &bdbuf_cache.transfer_waiters.sema);
+ sc = rtems_bdbuf_waiter_create (rtems_build_name ('B', 'D', 'C', 't'),
+ 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
+ &bdbuf_cache.transfer_waiters);
if (sc != RTEMS_SUCCESSFUL)
goto error;
- sc = rtems_semaphore_create (rtems_build_name ('B', 'D', 'C', 'b'),
- 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
- &bdbuf_cache.buffer_waiters.sema);
+ sc = rtems_bdbuf_waiter_create (rtems_build_name ('B', 'D', 'C', 'b'),
+ 0, RTEMS_BDBUF_CACHE_WAITER_ATTRIBS, 0,
+ &bdbuf_cache.buffer_waiters);
if (sc != RTEMS_SUCCESSFUL)
goto error;
@@ -1641,15 +1857,15 @@ error:
free (bdbuf_cache.swapout_transfer);
free (bdbuf_cache.swapout_workers);
- rtems_semaphore_delete (bdbuf_cache.buffer_waiters.sema);
- rtems_semaphore_delete (bdbuf_cache.access_waiters.sema);
- rtems_semaphore_delete (bdbuf_cache.transfer_waiters.sema);
- rtems_semaphore_delete (bdbuf_cache.sync_lock);
+ (void)rtems_bdbuf_waiter_delete (&bdbuf_cache.buffer_waiters);
+ (void)rtems_bdbuf_waiter_delete (&bdbuf_cache.access_waiters);
+ (void)rtems_bdbuf_waiter_delete (&bdbuf_cache.transfer_waiters);
+ (void)rtems_bdbuf_lock_delete (&bdbuf_cache.sync_lock);
- if (bdbuf_cache.lock != 0)
+ if (sc_lock == RTEMS_SUCCESSFUL)
{
rtems_bdbuf_unlock_cache ();
- rtems_semaphore_delete (bdbuf_cache.lock);
+ (void)rtems_bdbuf_lock_delete (&bdbuf_cache.lock);
}
rtems_bdbuf_data.return_status = RTEMS_UNSATISFIED;
--
1.7.10.4
More information about the devel
mailing list