Good.
On Thu, Jun 12, 2014 at 10:19 AM, Sebastian Huber <sebastian.hu...@embedded-brains.de> wrote: > The _Scheduler_Yield() was called by the executing thread with thread > dispatching disabled and interrupts enabled. The rtems_task_suspend() > is explicitly allowed in ISRs: > > http://rtems.org/onlinedocs/doc-current/share/rtems/html/c_user/Interrupt-Manager-Directives-Allowed-from-an-ISR.html#Interrupt-Manager-Directives-Allowed-from-an-ISR > > Unlike the other scheduler operations the locking was performed inside > the operation. This lead to the following race condition. Suppose a > ISR suspends the executing thread right before the yield scheduler > operation. Now the executing thread is not longer in the set of ready > threads. The typical scheduler operations did not check the thread > state and will now extract the thread again and enqueue it. This > corrupted data structures. > > Add _Thread_Yield() and do the scheduler yield operation with interrupts > disabled. This has a negligible effect on the interrupt latency. > --- > cpukit/posix/src/nanosleep.c | 3 +- > cpukit/posix/src/sched_yield.c | 7 +-- > cpukit/rtems/src/taskwakeafter.c | 3 +- > cpukit/score/Makefile.am | 1 + > .../score/include/rtems/score/schedulersmpimpl.h | 21 ++++++++++ > cpukit/score/include/rtems/score/threadimpl.h | 2 + > cpukit/score/src/schedulerdefaulttick.c | 4 +- > cpukit/score/src/scheduleredfyield.c | 7 --- > cpukit/score/src/schedulerprioritysmp.c | 14 +++--- > cpukit/score/src/schedulerpriorityyield.c | 23 ++++------- > cpukit/score/src/schedulersimplesmp.c | 14 +++--- > cpukit/score/src/schedulersimpleyield.c | 15 +------ > cpukit/score/src/threadyield.c | 41 > ++++++++++++++++++++ > 13 files changed, 98 insertions(+), 57 deletions(-) > create mode 100644 cpukit/score/src/threadyield.c > > diff --git a/cpukit/posix/src/nanosleep.c b/cpukit/posix/src/nanosleep.c > index ebaef33..54902b9 100644 > --- a/cpukit/posix/src/nanosleep.c > +++ b/cpukit/posix/src/nanosleep.c > @@ -22,7 +22,6 @@ > #include <errno.h> > > #include <rtems/seterr.h> > -#include <rtems/score/schedulerimpl.h> > #include <rtems/score/threadimpl.h> > #include <rtems/score/timespec.h> > #include <rtems/score/watchdogimpl.h> > @@ -65,7 +64,7 @@ int nanosleep( > if ( !ticks ) { > _Thread_Disable_dispatch(); > executing = _Thread_Executing; > - _Scheduler_Yield( _Scheduler_Get( executing ), executing ); > + _Thread_Yield( executing ); > _Thread_Enable_dispatch(); > if ( rmtp ) { > rmtp->tv_sec = 0; > diff --git a/cpukit/posix/src/sched_yield.c b/cpukit/posix/src/sched_yield.c > index 5293b19..e398fbf 100644 > --- a/cpukit/posix/src/sched_yield.c > +++ b/cpukit/posix/src/sched_yield.c > @@ -21,16 +21,13 @@ > #include <sched.h> > > #include <rtems/score/percpu.h> > -#include <rtems/score/schedulerimpl.h> > #include <rtems/score/threaddispatch.h> > +#include <rtems/score/threadimpl.h> > > int sched_yield( void ) > { > - Thread_Control *executing; > - > _Thread_Disable_dispatch(); > - executing = _Thread_Executing; > - _Scheduler_Yield( _Scheduler_Get( executing ), executing ); > + _Thread_Yield( _Thread_Executing ); > _Thread_Enable_dispatch(); > return 0; > } > diff --git a/cpukit/rtems/src/taskwakeafter.c > b/cpukit/rtems/src/taskwakeafter.c > index 367b43a..88de6e5 100644 > --- a/cpukit/rtems/src/taskwakeafter.c > +++ b/cpukit/rtems/src/taskwakeafter.c > @@ -20,7 +20,6 @@ > > #include <rtems/rtems/tasks.h> > #include <rtems/score/threadimpl.h> > -#include <rtems/score/schedulerimpl.h> > #include <rtems/score/watchdogimpl.h> > > rtems_status_code rtems_task_wake_after( > @@ -37,7 +36,7 @@ rtems_status_code rtems_task_wake_after( > executing = _Thread_Executing; > > if ( ticks == 0 ) { > - _Scheduler_Yield( _Scheduler_Get( executing ), executing ); > + _Thread_Yield( executing ); > } else { > _Thread_Set_state( executing, STATES_DELAYING ); > _Watchdog_Initialize( > diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am > index 2fc31c5..6caefb5 100644 > --- a/cpukit/score/Makefile.am > +++ b/cpukit/score/Makefile.am > @@ -287,6 +287,7 @@ libscore_a_SOURCES += src/thread.c > src/threadchangepriority.c \ > src/threadstackallocate.c src/threadstackfree.c src/threadstart.c \ > src/threadstartmultitasking.c src/iterateoverthreads.c \ > src/threadblockingoperationcancel.c > +libscore_a_SOURCES += src/threadyield.c > > if HAS_SMP > libscore_a_SOURCES += src/smpbarrierwait.c > diff --git a/cpukit/score/include/rtems/score/schedulersmpimpl.h > b/cpukit/score/include/rtems/score/schedulersmpimpl.h > index b4126b5..9d74434 100644 > --- a/cpukit/score/include/rtems/score/schedulersmpimpl.h > +++ b/cpukit/score/include/rtems/score/schedulersmpimpl.h > @@ -672,6 +672,27 @@ static inline void _Scheduler_SMP_Change_priority( > } > } > > +static inline void _Scheduler_SMP_Yield( > + Scheduler_Context *context, > + Thread_Control *thread, > + Scheduler_SMP_Extract extract_from_ready, > + Scheduler_SMP_Enqueue enqueue_fifo, > + Scheduler_SMP_Enqueue enqueue_scheduled_fifo > +) > +{ > + Scheduler_SMP_Node *node = _Scheduler_SMP_Node_get( thread ); > + > + if ( node->state == SCHEDULER_SMP_NODE_SCHEDULED ) { > + _Scheduler_SMP_Extract_from_scheduled( thread ); > + > + ( *enqueue_scheduled_fifo )( context, thread ); > + } else { > + ( *extract_from_ready )( context, thread ); > + > + ( *enqueue_fifo )( context, thread ); > + } > +} > + > static inline void _Scheduler_SMP_Insert_scheduled_lifo( > Scheduler_Context *context, > Thread_Control *thread > diff --git a/cpukit/score/include/rtems/score/threadimpl.h > b/cpukit/score/include/rtems/score/threadimpl.h > index 7e6681b..e90346c 100644 > --- a/cpukit/score/include/rtems/score/threadimpl.h > +++ b/cpukit/score/include/rtems/score/threadimpl.h > @@ -186,6 +186,8 @@ bool _Thread_Restart( > Thread_Entry_numeric_type numeric_argument > ); > > +void _Thread_Yield( Thread_Control *executing ); > + > bool _Thread_Set_life_protection( bool protect ); > > void _Thread_Life_action_handler( > diff --git a/cpukit/score/src/schedulerdefaulttick.c > b/cpukit/score/src/schedulerdefaulttick.c > index 98cd05e..db67ca1 100644 > --- a/cpukit/score/src/schedulerdefaulttick.c > +++ b/cpukit/score/src/schedulerdefaulttick.c > @@ -29,6 +29,8 @@ void _Scheduler_default_Tick( > Thread_Control *executing > ) > { > + (void) scheduler; > + > #ifdef __RTEMS_USE_TICKS_FOR_STATISTICS__ > /* > * Increment the number of ticks this thread has been executing > @@ -69,7 +71,7 @@ void _Scheduler_default_Tick( > * currently executing thread is placed at the rear of the > * FIFO for this priority and a new heir is selected. > */ > - _Scheduler_Yield( scheduler, executing ); > + _Thread_Yield( executing ); > executing->cpu_time_budget = > rtems_configuration_get_ticks_per_timeslice(); > } > diff --git a/cpukit/score/src/scheduleredfyield.c > b/cpukit/score/src/scheduleredfyield.c > index 9eb0782..d43448b 100644 > --- a/cpukit/score/src/scheduleredfyield.c > +++ b/cpukit/score/src/scheduleredfyield.c > @@ -29,9 +29,6 @@ void _Scheduler_EDF_Yield( > Scheduler_EDF_Context *context = > _Scheduler_EDF_Get_context( scheduler ); > Scheduler_EDF_Node *node = _Scheduler_EDF_Node_get( the_thread ); > - ISR_Level level; > - > - _ISR_Disable( level ); > > /* > * The RBTree has more than one node, enqueue behind the tasks > @@ -40,9 +37,5 @@ void _Scheduler_EDF_Yield( > _RBTree_Extract( &context->Ready, &node->Node ); > _RBTree_Insert( &context->Ready, &node->Node ); > > - _ISR_Flash( level ); > - > _Scheduler_EDF_Schedule_body( scheduler, the_thread, false ); > - > - _ISR_Enable( level ); > } > diff --git a/cpukit/score/src/schedulerprioritysmp.c > b/cpukit/score/src/schedulerprioritysmp.c > index 96b1689..bcbd26d 100644 > --- a/cpukit/score/src/schedulerprioritysmp.c > +++ b/cpukit/score/src/schedulerprioritysmp.c > @@ -237,12 +237,12 @@ void _Scheduler_priority_SMP_Yield( > ) > { > Scheduler_Context *context = _Scheduler_Get_context( scheduler ); > - ISR_Level level; > > - _ISR_Disable( level ); > - > - _Scheduler_SMP_Extract_from_scheduled( thread ); > - _Scheduler_priority_SMP_Enqueue_scheduled_fifo( context, thread ); > - > - _ISR_Enable( level ); > + return _Scheduler_SMP_Yield( > + context, > + thread, > + _Scheduler_priority_SMP_Extract_from_ready, > + _Scheduler_priority_SMP_Enqueue_fifo, > + _Scheduler_priority_SMP_Enqueue_scheduled_fifo > + ); > } > diff --git a/cpukit/score/src/schedulerpriorityyield.c > b/cpukit/score/src/schedulerpriorityyield.c > index f2aeada..60bab39 100644 > --- a/cpukit/score/src/schedulerpriorityyield.c > +++ b/cpukit/score/src/schedulerpriorityyield.c > @@ -19,7 +19,6 @@ > #endif > > #include <rtems/score/schedulerpriorityimpl.h> > -#include <rtems/score/isr.h> > #include <rtems/score/threadimpl.h> > > void _Scheduler_priority_Yield( > @@ -29,23 +28,19 @@ void _Scheduler_priority_Yield( > { > Scheduler_priority_Node *node = _Scheduler_priority_Node_get( the_thread ); > Chain_Control *ready_chain = node->Ready_queue.ready_chain; > - ISR_Level level; > > (void) scheduler; > > - _ISR_Disable( level ); > - if ( !_Chain_Has_only_one_node( ready_chain ) ) { > - _Chain_Extract_unprotected( &the_thread->Object.Node ); > - _Chain_Append_unprotected( ready_chain, &the_thread->Object.Node ); > + if ( !_Chain_Has_only_one_node( ready_chain ) ) { > + _Chain_Extract_unprotected( &the_thread->Object.Node ); > + _Chain_Append_unprotected( ready_chain, &the_thread->Object.Node ); > > - _ISR_Flash( level ); > - > - if ( _Thread_Is_heir( the_thread ) ) > - _Thread_Heir = (Thread_Control *) _Chain_First( ready_chain ); > - _Thread_Dispatch_necessary = true; > + if ( _Thread_Is_heir( the_thread ) ) { > + _Thread_Heir = (Thread_Control *) _Chain_First( ready_chain ); > } > - else if ( !_Thread_Is_heir( the_thread ) ) > - _Thread_Dispatch_necessary = true; > > - _ISR_Enable( level ); > + _Thread_Dispatch_necessary = true; > + } else if ( !_Thread_Is_heir( the_thread ) ) { > + _Thread_Dispatch_necessary = true; > + } > } > diff --git a/cpukit/score/src/schedulersimplesmp.c > b/cpukit/score/src/schedulersimplesmp.c > index eb260ef..37458d6 100644 > --- a/cpukit/score/src/schedulersimplesmp.c > +++ b/cpukit/score/src/schedulersimplesmp.c > @@ -302,12 +302,12 @@ void _Scheduler_simple_SMP_Yield( > ) > { > Scheduler_Context *context = _Scheduler_Get_context( scheduler ); > - ISR_Level level; > > - _ISR_Disable( level ); > - > - _Scheduler_SMP_Extract_from_scheduled( thread ); > - _Scheduler_simple_SMP_Enqueue_scheduled_fifo( context, thread ); > - > - _ISR_Enable( level ); > + return _Scheduler_SMP_Yield( > + context, > + thread, > + _Scheduler_simple_SMP_Extract_from_ready, > + _Scheduler_simple_SMP_Enqueue_fifo, > + _Scheduler_simple_SMP_Enqueue_scheduled_fifo > + ); > } > diff --git a/cpukit/score/src/schedulersimpleyield.c > b/cpukit/score/src/schedulersimpleyield.c > index 65578d0..b807530 100644 > --- a/cpukit/score/src/schedulersimpleyield.c > +++ b/cpukit/score/src/schedulersimpleyield.c > @@ -19,7 +19,6 @@ > #endif > > #include <rtems/score/schedulersimpleimpl.h> > -#include <rtems/score/isr.h> > > void _Scheduler_simple_Yield( > const Scheduler_Control *scheduler, > @@ -28,16 +27,8 @@ void _Scheduler_simple_Yield( > { > Scheduler_simple_Context *context = > _Scheduler_simple_Get_context( scheduler ); > - ISR_Level level; > > - _ISR_Disable( level ); > - > - _Chain_Extract_unprotected( &the_thread->Object.Node ); > - _Scheduler_simple_Insert_priority_fifo( &context->Ready, the_thread ); > - > - _ISR_Flash( level ); > - > - _Scheduler_simple_Schedule_body( scheduler, the_thread, false ); > - > - _ISR_Enable( level ); > + _Chain_Extract_unprotected( &the_thread->Object.Node ); > + _Scheduler_simple_Insert_priority_fifo( &context->Ready, the_thread ); > + _Scheduler_simple_Schedule_body( scheduler, the_thread, false ); > } > diff --git a/cpukit/score/src/threadyield.c b/cpukit/score/src/threadyield.c > new file mode 100644 > index 0000000..b49e2b3 > --- /dev/null > +++ b/cpukit/score/src/threadyield.c > @@ -0,0 +1,41 @@ > +/** > + * @file > + * > + * @brief Thread Yield > + * > + * @ingroup ScoreThread > + */ > + > +/* > + * Copyright (c) 2014 embedded brains GmbH. All rights reserved. > + * > + * embedded brains GmbH > + * Dornierstr. 4 > + * 82178 Puchheim > + * Germany > + * <rt...@embedded-brains.de> > + * > + * The license and distribution terms for this file may be > + * found in the file LICENSE in this distribution or at > + * http://www.rtems.org/license/LICENSE. > + */ > + > +#if HAVE_CONFIG_H > + #include "config.h" > +#endif > + > +#include <rtems/score/threadimpl.h> > +#include <rtems/score/schedulerimpl.h> > + > +void _Thread_Yield( Thread_Control *executing ) > +{ > + ISR_Level level; > + > + _ISR_Disable( level ); > + > + if ( _States_Is_ready( executing->current_state ) ) { > + _Scheduler_Yield( _Scheduler_Get( executing ), executing ); > + } > + > + _ISR_Enable( level ); > +} > -- > 1.7.7 > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
