[PATCH 2/2] rtems: Fix task restart within interrupt context
Gedare Bloom
gedare at rtems.org
Fri May 14 14:52:33 UTC 2021
These 2 patches are fine. Although it is an internal interface, you
might make the 'ignore' parameter 'ignore_mask' maybe. My initial
thought when I see 'ignore' is that variable is ignored/unused, not
that it contains the states to ignore. Since the function is internal
and has no documentation, this might make it easier for future
maintainers to understand.
On Fri, May 14, 2021 at 7:11 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> rtems_task_restart() may be called from within interrupt context. So
> checking only that the thread to restart is equal to the executing
> thread is insufficient to determine a self restart. We have to also
> check that no ISR is in progress. Merge _Thread_Restart_other() and
> _Thread_Restart_self() into one _Thread_Restart() since they share a lot
> of common code.
>
> Close #4412.
> ---
> cpukit/include/rtems/score/threadimpl.h | 15 +-----
> cpukit/rtems/src/taskrestart.c | 8 +--
> cpukit/score/src/threadrestart.c | 68 ++++++-------------------
> 3 files changed, 17 insertions(+), 74 deletions(-)
>
> diff --git a/cpukit/include/rtems/score/threadimpl.h b/cpukit/include/rtems/score/threadimpl.h
> index 8674a3f2b3..44785cd937 100644
> --- a/cpukit/include/rtems/score/threadimpl.h
> +++ b/cpukit/include/rtems/score/threadimpl.h
> @@ -270,19 +270,6 @@ Status_Control _Thread_Start(
> ISR_lock_Context *lock_context
> );
>
> -/**
> - * @brief Restarts the currently executing thread.
> - *
> - * @param[in, out] executing The currently executing thread.
> - * @param entry The start entry information for @a executing.
> - * @param lock_context The lock context.
> - */
> -RTEMS_NO_RETURN void _Thread_Restart_self(
> - Thread_Control *executing,
> - const Thread_Entry_information *entry,
> - ISR_lock_Context *lock_context
> -);
> -
> /**
> * @brief Restarts the thread.
> *
> @@ -296,7 +283,7 @@ RTEMS_NO_RETURN void _Thread_Restart_self(
> *
> * @retval STATUS_INCORRECT_STATE The thread was dormant.
> */
> -Status_Control _Thread_Restart_other(
> +Status_Control _Thread_Restart(
> Thread_Control *the_thread,
> const Thread_Entry_information *entry,
> ISR_lock_Context *lock_context
> diff --git a/cpukit/rtems/src/taskrestart.c b/cpukit/rtems/src/taskrestart.c
> index 00b0635cef..6bf7358384 100644
> --- a/cpukit/rtems/src/taskrestart.c
> +++ b/cpukit/rtems/src/taskrestart.c
> @@ -48,13 +48,7 @@ rtems_status_code rtems_task_restart(
>
> entry = the_thread->Start.Entry;
> entry.Kinds.Numeric.argument = argument;
> -
> - if ( the_thread == _Thread_Executing ) {
> - _Thread_Restart_self( the_thread, &entry, &lock_context );
> - RTEMS_UNREACHABLE();
> - }
> -
> - status = _Thread_Restart_other( the_thread, &entry, &lock_context );
> + status = _Thread_Restart( the_thread, &entry, &lock_context );
>
> return _Status_Get( status );
> }
> diff --git a/cpukit/score/src/threadrestart.c b/cpukit/score/src/threadrestart.c
> index d99c14afc0..7b8c481b80 100644
> --- a/cpukit/score/src/threadrestart.c
> +++ b/cpukit/score/src/threadrestart.c
> @@ -5,8 +5,7 @@
> *
> * @brief This source file contains the implementation of _Thread_Cancel(),
> * _Thread_Change_life(), _Thread_Close(), _Thread_Exit(), _Thread_Join(),
> - * _Thread_Kill_zombies(), _Thread_Restart_other(), _Thread_Restart_self(),
> - * and _Thread_Set_life_protection().
> + * _Thread_Kill_zombies(), _Thread_Restart(), and _Thread_Set_life_protection().
> */
>
> /*
> @@ -517,7 +516,7 @@ void _Thread_Exit(
> _Thread_State_release( executing, &lock_context );
> }
>
> -Status_Control _Thread_Restart_other(
> +Status_Control _Thread_Restart(
> Thread_Control *the_thread,
> const Thread_Entry_information *entry,
> ISR_lock_Context *lock_context
> @@ -525,6 +524,7 @@ Status_Control _Thread_Restart_other(
> {
> Thread_Life_state previous;
> Per_CPU_Control *cpu_self;
> + Thread_Life_state ignore;
> Thread_queue_Context queue_context;
>
> _Thread_State_acquire_critical( the_thread, lock_context );
> @@ -534,16 +534,25 @@ Status_Control _Thread_Restart_other(
> return STATUS_INCORRECT_STATE;
> }
>
> + cpu_self = _Thread_Dispatch_disable_critical( lock_context );
> +
> + if (
> + the_thread == _Per_CPU_Get_executing( cpu_self ) &&
> + !_ISR_Is_in_progress()
> + ) {
> + ignore = THREAD_LIFE_PROTECTED | THREAD_LIFE_CHANGE_DEFERRED;
> + } else {
> + ignore = 0;
> + }
> +
> the_thread->Start.Entry = *entry;
> previous = _Thread_Change_life_locked(
> the_thread,
> 0,
> THREAD_LIFE_RESTARTING,
> - 0
> + ignore
> );
>
> - cpu_self = _Thread_Dispatch_disable_critical( lock_context );
> -
> if ( _Thread_Is_life_change_allowed( previous ) ) {
> _Thread_Add_life_change_request( the_thread );
> _Thread_State_release( the_thread, lock_context );
> @@ -573,53 +582,6 @@ Status_Control _Thread_Restart_other(
> return STATUS_SUCCESSFUL;
> }
>
> -void _Thread_Restart_self(
> - Thread_Control *executing,
> - const Thread_Entry_information *entry,
> - ISR_lock_Context *lock_context
> -)
> -{
> - Per_CPU_Control *cpu_self;
> - Thread_queue_Context queue_context;
> -
> - _Assert(
> - _Watchdog_Get_state( &executing->Timer.Watchdog ) == WATCHDOG_INACTIVE
> - );
> - _Assert(
> - executing->current_state == STATES_READY
> - || executing->current_state == STATES_SUSPENDED
> - );
> -
> - _Thread_queue_Context_initialize( &queue_context );
> - _Thread_queue_Context_clear_priority_updates( &queue_context );
> - _Thread_State_acquire_critical( executing, lock_context );
> -
> - executing->Start.Entry = *entry;
> - _Thread_Change_life_locked(
> - executing,
> - 0,
> - THREAD_LIFE_RESTARTING,
> - THREAD_LIFE_PROTECTED | THREAD_LIFE_CHANGE_DEFERRED
> - );
> -
> - cpu_self = _Thread_Dispatch_disable_critical( lock_context );
> - _Thread_State_release( executing, lock_context );
> -
> - _Thread_Wait_acquire_default( executing, lock_context );
> - _Thread_Priority_change(
> - executing,
> - &executing->Real_priority,
> - executing->Start.initial_priority,
> - false,
> - &queue_context
> - );
> - _Thread_Wait_release_default( executing, lock_context );
> -
> - _Thread_Priority_update( &queue_context );
> - _Thread_Dispatch_direct_no_return( cpu_self );
> - RTEMS_UNREACHABLE();
> -}
> -
> Thread_Life_state _Thread_Change_life(
> Thread_Life_state clear,
> Thread_Life_state set,
> --
> 2.26.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list