diff options
author | Sebastian Huber <sebastian.huber@embedded-brains.de> | 2015-04-15 10:53:29 +0200 |
---|---|---|
committer | Sebastian Huber <sebastian.huber@embedded-brains.de> | 2015-05-19 12:00:43 +0200 |
commit | 1ccbd052910ed16131c74b0d5595c8a94066942d (patch) | |
tree | a0c675b888d112efe455165bde2d9f0803392d3f /cpukit/score | |
parent | score: Add _Watchdog_Acquire|Release|Flash() (diff) | |
download | rtems-1ccbd052910ed16131c74b0d5595c8a94066942d.tar.bz2 |
score: Add Watchdog_Iterator
Rewrite the _Watchdog_Insert(), _Watchdog_Remove() and
_Watchdog_Tickle() functions to use iterator items to synchronize
concurrent operations. This makes it possible to get rid of the global
variables _Watchdog_Sync_level and _Watchdog_Sync_count which are a
blocking point for scalable SMP solutions.
Update #2307.
Diffstat (limited to 'cpukit/score')
-rw-r--r-- | cpukit/score/include/rtems/score/watchdogimpl.h | 58 | ||||
-rw-r--r-- | cpukit/score/src/watchdog.c | 2 | ||||
-rw-r--r-- | cpukit/score/src/watchdoginsert.c | 112 | ||||
-rw-r--r-- | cpukit/score/src/watchdogremove.c | 71 | ||||
-rw-r--r-- | cpukit/score/src/watchdogtickle.c | 139 |
5 files changed, 216 insertions, 166 deletions
diff --git a/cpukit/score/include/rtems/score/watchdogimpl.h b/cpukit/score/include/rtems/score/watchdogimpl.h index f52b55d83c..304392b46c 100644 --- a/cpukit/score/include/rtems/score/watchdogimpl.h +++ b/cpukit/score/include/rtems/score/watchdogimpl.h @@ -46,6 +46,28 @@ extern "C" { } /** + * @brief Iterator item to synchronize concurrent insert, remove and tickle + * operations. + */ +typedef struct { + /** + * @brief A node for a Watchdog_Header::Iterators chain. + */ + Chain_Node Node; + + /** + * @brief The current delta interval of the new watchdog to insert. + */ + Watchdog_Interval delta_interval; + + /** + * @brief The current watchdog of the chain on the way to insert the new + * watchdog. + */ + Chain_Node *current; +} Watchdog_Iterator; + +/** * @brief Watchdog header. */ typedef struct { @@ -58,23 +80,15 @@ typedef struct { * @brief The chain of active or transient watchdogs. */ Chain_Control Watchdogs; -} Watchdog_Header; - -/** - * @brief Watchdog synchronization level. - * - * This used for synchronization purposes - * during an insert on a watchdog delta chain. - */ -SCORE_EXTERN volatile uint32_t _Watchdog_Sync_level; -/** - * @brief Watchdog synchronization count. - * - * This used for synchronization purposes - * during an insert on a watchdog delta chain. - */ -SCORE_EXTERN volatile uint32_t _Watchdog_Sync_count; + /** + * @brief Currently active iterators. + * + * The iterators are registered in _Watchdog_Insert() and updated in case the + * watchdog chain changes. + */ + Chain_Control Iterators; +} Watchdog_Header; /** * @brief Watchdog chain which is managed at ticks. @@ -139,6 +153,16 @@ Watchdog_States _Watchdog_Remove ( ); /** + * @brief Actually removes an WATCHDOG_ACTIVE or WATCHDOG_REMOVE_IT watchdog. + * + * @see _Watchdog_Remove() and _Watchdog_Tickle(). + */ +void _Watchdog_Remove_it( + Watchdog_Header *header, + Watchdog_Control *the_watchdog +); + +/** * @brief Adjusts the header watchdog chain in the backward direction for * units ticks. * @@ -437,7 +461,9 @@ RTEMS_INLINE_ROUTINE void _Watchdog_Header_initialize( Watchdog_Header *header ) { + _ISR_lock_Initialize( &header->Lock, "Watchdog" ); _Chain_Initialize_empty( &header->Watchdogs ); + _Chain_Initialize_empty( &header->Iterators ); } /** @} */ diff --git a/cpukit/score/src/watchdog.c b/cpukit/score/src/watchdog.c index 0db60efe6b..11d3cf289f 100644 --- a/cpukit/score/src/watchdog.c +++ b/cpukit/score/src/watchdog.c @@ -25,8 +25,6 @@ void _Watchdog_Handler_initialization( void ) { - _Watchdog_Sync_count = 0; - _Watchdog_Sync_level = 0; _Watchdog_Ticks_since_boot = 0; _Watchdog_Header_initialize( &_Watchdog_Ticks_header ); diff --git a/cpukit/score/src/watchdoginsert.c b/cpukit/score/src/watchdoginsert.c index 0ad59ff61a..6d2df8222f 100644 --- a/cpukit/score/src/watchdoginsert.c +++ b/cpukit/score/src/watchdoginsert.c @@ -19,76 +19,92 @@ #endif #include <rtems/score/watchdogimpl.h> -#include <rtems/score/isrlevel.h> -#include <rtems/score/percpu.h> -void _Watchdog_Insert( - Watchdog_Header *header, - Watchdog_Control *the_watchdog +static void _Watchdog_Insert_fixup( + Watchdog_Header *header, + Watchdog_Control *next_watchdog, + Watchdog_Interval delta ) { - ISR_lock_Context lock_context; - Watchdog_Control *after; - uint32_t insert_isr_nest_level; - Watchdog_Interval delta_interval; + const Chain_Node *iterator_tail; + Chain_Node *iterator_node; + next_watchdog->delta_interval -= delta; - insert_isr_nest_level = _ISR_Nest_level; + iterator_node = _Chain_First( &header->Iterators ); + iterator_tail = _Chain_Immutable_tail( &header->Iterators ); - _Watchdog_Acquire( header, &lock_context ); + while ( iterator_node != iterator_tail ) { + Watchdog_Iterator *iterator; - /* - * Check to see if the watchdog has just been inserted by a - * higher priority interrupt. If so, abandon this insert. - */ + iterator = (Watchdog_Iterator *) iterator_node; - if ( the_watchdog->state != WATCHDOG_INACTIVE ) { - _Watchdog_Release( header, &lock_context ); - return; + if ( iterator->current == &next_watchdog->Node ) { + iterator->delta_interval -= delta; + } + + iterator_node = _Chain_Next( iterator_node ); } +} - the_watchdog->state = WATCHDOG_BEING_INSERTED; - _Watchdog_Sync_count++; +void _Watchdog_Insert( + Watchdog_Header *header, + Watchdog_Control *the_watchdog +) +{ + ISR_lock_Context lock_context; -restart: - delta_interval = the_watchdog->initial; + _Watchdog_Acquire( header, &lock_context ); - for ( after = _Watchdog_First( header ) ; - ; - after = _Watchdog_Next( after ) ) { + if ( the_watchdog->state == WATCHDOG_INACTIVE ) { + Watchdog_Iterator iterator; + Chain_Node *current; + Chain_Node *next; + Watchdog_Interval delta; - if ( delta_interval == 0 || !_Watchdog_Next( after ) ) - break; + the_watchdog->state = WATCHDOG_BEING_INSERTED; - if ( delta_interval < after->delta_interval ) { - after->delta_interval -= delta_interval; - break; - } + _Chain_Append_unprotected( &header->Iterators, &iterator.Node ); - delta_interval -= after->delta_interval; + delta = the_watchdog->initial; + current = _Chain_Head( &header->Watchdogs ); - _Watchdog_Flash( header, &lock_context ); + while ( + ( next = _Chain_Next( current ) ) != _Chain_Tail( &header->Watchdogs ) + ) { + Watchdog_Control *next_watchdog; + Watchdog_Interval delta_next; - if ( the_watchdog->state != WATCHDOG_BEING_INSERTED ) { - goto exit_insert; - } + next_watchdog = (Watchdog_Control *) next; + delta_next = next_watchdog->delta_interval; - if ( _Watchdog_Sync_level > insert_isr_nest_level ) { - _Watchdog_Sync_level = insert_isr_nest_level; - goto restart; - } - } + if ( delta < delta_next ) { + _Watchdog_Insert_fixup( header, next_watchdog, delta ); + break; + } + + iterator.delta_interval = delta - delta_next; + iterator.current = next; - _Watchdog_Activate( the_watchdog ); + _Watchdog_Flash( header, &lock_context ); - the_watchdog->delta_interval = delta_interval; + if ( the_watchdog->state != WATCHDOG_BEING_INSERTED ) { + goto abort_insert; + } - _Chain_Insert_unprotected( after->Node.previous, &the_watchdog->Node ); + delta = iterator.delta_interval; + current = iterator.current; + } - the_watchdog->start_time = _Watchdog_Ticks_since_boot; + the_watchdog->delta_interval = delta; + the_watchdog->start_time = _Watchdog_Ticks_since_boot; + _Watchdog_Activate( the_watchdog ); + _Chain_Insert_unprotected( current, &the_watchdog->Node ); + +abort_insert: + + _Chain_Extract_unprotected( &iterator.Node ); + } -exit_insert: - _Watchdog_Sync_level = insert_isr_nest_level; - _Watchdog_Sync_count--; _Watchdog_Release( header, &lock_context ); } diff --git a/cpukit/score/src/watchdogremove.c b/cpukit/score/src/watchdogremove.c index c765ac55c7..c896fbb00f 100644 --- a/cpukit/score/src/watchdogremove.c +++ b/cpukit/score/src/watchdogremove.c @@ -18,9 +18,58 @@ #include "config.h" #endif -#include <rtems/system.h> -#include <rtems/score/isr.h> #include <rtems/score/watchdogimpl.h> +#include <rtems/score/assert.h> + +void _Watchdog_Remove_it( + Watchdog_Header *header, + Watchdog_Control *the_watchdog +) +{ + Chain_Node *next; + Watchdog_Interval delta; + const Chain_Node *iterator_tail; + Chain_Node *iterator_node; + + _Assert( + the_watchdog->state == WATCHDOG_ACTIVE + || the_watchdog->state == WATCHDOG_REMOVE_IT + ); + + the_watchdog->state = WATCHDOG_INACTIVE; + the_watchdog->stop_time = _Watchdog_Ticks_since_boot; + + next = _Chain_Next( &the_watchdog->Node ); + delta = the_watchdog->delta_interval; + + if ( next != _Chain_Tail( &header->Watchdogs ) ) { + Watchdog_Control *next_watchdog; + + next_watchdog = (Watchdog_Control *) next; + next_watchdog->delta_interval += delta; + } + + _Chain_Extract_unprotected( &the_watchdog->Node ); + + iterator_node = _Chain_First( &header->Iterators ); + iterator_tail = _Chain_Immutable_tail( &header->Iterators ); + + while ( iterator_node != iterator_tail ) { + Watchdog_Iterator *iterator; + + iterator = (Watchdog_Iterator *) iterator_node; + + if ( iterator->current == next ) { + iterator->delta_interval += delta; + } + + if ( iterator->current == &the_watchdog->Node ) { + iterator->current = _Chain_Previous( &the_watchdog->Node ); + } + + iterator_node = _Chain_Next( iterator_node ); + } +} Watchdog_States _Watchdog_Remove( Watchdog_Header *header, @@ -29,7 +78,7 @@ Watchdog_States _Watchdog_Remove( { ISR_lock_Context lock_context; Watchdog_States previous_state; - Watchdog_Control *next_watchdog; + Watchdog_Interval now; _Watchdog_Acquire( header, &lock_context ); previous_state = the_watchdog->state; @@ -44,24 +93,16 @@ Watchdog_States _Watchdog_Remove( * the Insert operation we interrupted will be aborted. */ the_watchdog->state = WATCHDOG_INACTIVE; + now = _Watchdog_Ticks_since_boot; + the_watchdog->start_time = now; + the_watchdog->stop_time = now; break; case WATCHDOG_ACTIVE: case WATCHDOG_REMOVE_IT: - - the_watchdog->state = WATCHDOG_INACTIVE; - next_watchdog = _Watchdog_Next( the_watchdog ); - - if ( _Watchdog_Next(next_watchdog) ) - next_watchdog->delta_interval += the_watchdog->delta_interval; - - if ( _Watchdog_Sync_count ) - _Watchdog_Sync_level = _ISR_Nest_level; - - _Chain_Extract_unprotected( &the_watchdog->Node ); + _Watchdog_Remove_it( header, the_watchdog ); break; } - the_watchdog->stop_time = _Watchdog_Ticks_since_boot; _Watchdog_Release( header, &lock_context ); return( previous_state ); diff --git a/cpukit/score/src/watchdogtickle.c b/cpukit/score/src/watchdogtickle.c index 2092010dcc..7a80008c4d 100644 --- a/cpukit/score/src/watchdogtickle.c +++ b/cpukit/score/src/watchdogtickle.c @@ -19,99 +19,68 @@ #endif #include <rtems/score/watchdogimpl.h> -#include <rtems/score/isrlevel.h> void _Watchdog_Tickle( Watchdog_Header *header ) { - ISR_lock_Context lock_context; - Watchdog_Control *the_watchdog; - Watchdog_States watchdog_state; - - /* - * See the comment in watchdoginsert.c and watchdogadjust.c - * about why it's safe not to declare header a pointer to - * volatile data - till, 2003/7 - */ + ISR_lock_Context lock_context; _Watchdog_Acquire( header, &lock_context ); - if ( _Watchdog_Is_empty( header ) ) - goto leave; - - the_watchdog = _Watchdog_First( header ); - - /* - * For some reason, on rare occasions the_watchdog->delta_interval - * of the head of the watchdog chain is 0. Before this test was - * added, on these occasions an event (which usually was supposed - * to have a timeout of 1 tick would have a delta_interval of 0, which - * would be decremented to 0xFFFFFFFF by the unprotected - * "the_watchdog->delta_interval--;" operation. - * This would mean the event would not timeout, and also the chain would - * be blocked, because a timeout with a very high number would be at the - * head, rather than at the end. - * The test "if (the_watchdog->delta_interval != 0)" - * here prevents this from occuring. - * - * We were not able to categorically identify the situation that causes - * this, but proved it to be true empirically. So this check causes - * correct behaviour in this circumstance. - * - * The belief is that a race condition exists whereby an event at the head - * of the chain is removed (by a pending ISR or higher priority task) - * during the _ISR_Flash( level ); in _Watchdog_Insert, but the watchdog - * to be inserted has already had its delta_interval adjusted to 0, and - * so is added to the head of the chain with a delta_interval of 0. - * - * Steven Johnson - 12/2005 (gcc-3.2.3 -O3 on powerpc) - */ - if (the_watchdog->delta_interval != 0) { - the_watchdog->delta_interval--; - if ( the_watchdog->delta_interval != 0 ) - goto leave; + if ( !_Watchdog_Is_empty( header ) ) { + Watchdog_Control *first; + Watchdog_Interval delta; + + first = _Watchdog_First( header ); + delta = first->delta_interval; + + /* + * Although it is forbidden to insert watchdogs with a delta interval of + * zero it is possible to observe watchdogs with a delta interval of zero + * at this point. For example lets have a watchdog chain of one watchdog + * with a delta interval of one and insert a new one with an initial value + * of one. At the start of the insert procedure it will advance one step + * and reduce its delta interval by one yielding zero. Now a tick happens. + * This will remove the watchdog on the chain and update the insert + * iterator. Now the insert operation continues and will insert the new + * watchdog with a delta interval of zero. + */ + if ( delta > 0 ) { + --delta; + first->delta_interval = delta; + } + + while ( delta == 0 ) { + bool run; + Watchdog_Service_routine_entry routine; + Objects_Id id; + void *user_data; + + run = ( first->state == WATCHDOG_ACTIVE ); + + _Watchdog_Remove_it( header, first ); + + routine = first->routine; + id = first->id; + user_data = first->user_data; + + _Watchdog_Release( header, &lock_context ); + + if ( run ) { + (*routine)( id, user_data ); + } + + _Watchdog_Acquire( header, &lock_context ); + + if ( _Watchdog_Is_empty( header ) ) { + break; + } + + first = _Watchdog_First( header ); + delta = first->delta_interval; + } } - do { - watchdog_state = _Watchdog_Remove( header, the_watchdog ); - - _Watchdog_Release( header, &lock_context ); - - switch( watchdog_state ) { - case WATCHDOG_ACTIVE: - (*the_watchdog->routine)( - the_watchdog->id, - the_watchdog->user_data - ); - break; - - case WATCHDOG_INACTIVE: - /* - * This state indicates that the watchdog is not on any chain. - * Thus, it is NOT on a chain being tickled. This case should - * never occur. - */ - break; - - case WATCHDOG_BEING_INSERTED: - /* - * This state indicates that the watchdog is in the process of - * BEING inserted on the chain. Thus, it can NOT be on a chain - * being tickled. This case should never occur. - */ - break; - - case WATCHDOG_REMOVE_IT: - break; - } - - _Watchdog_Acquire( header, &lock_context ); - - the_watchdog = _Watchdog_First( header ); - } while ( !_Watchdog_Is_empty( header ) && - (the_watchdog->delta_interval == 0) ); - -leave: - _Watchdog_Release( header, &lock_context ); + _Watchdog_Release( header, &lock_context ); } |