On Wed, Feb 17, 2021 at 12:30 PM Sebastian Huber
<sebastian.hu...@embedded-brains.de> wrote:
>
> Do the mode changes necessary for the ASR processing directly under
> protection of the thread state lock to avoid the recursive calls to
> thread dispatching done in rtems_task_mode().
>
> Update #4244.
> ---
> cpukit/rtems/src/signalsend.c | 89 ++++++++++++++++++++++++++++++++---
> 1 file changed, 82 insertions(+), 7 deletions(-)
>
> diff --git a/cpukit/rtems/src/signalsend.c b/cpukit/rtems/src/signalsend.c
> index 606ddfcb53..dd2f70ec53 100644
> --- a/cpukit/rtems/src/signalsend.c
> +++ b/cpukit/rtems/src/signalsend.c
> @@ -21,7 +21,9 @@
> #endif
>
> #include <rtems/rtems/signalimpl.h>
> +#include <rtems/rtems/modesimpl.h>
> #include <rtems/rtems/tasksdata.h>
> +#include <rtems/score/schedulerimpl.h>
> #include <rtems/score/threaddispatch.h>
> #include <rtems/score/threadimpl.h>
>
> @@ -33,10 +35,17 @@ static void _Signal_Action_handler(
> ISR_lock_Context *lock_context
> )
> {
> - RTEMS_API_Control *api;
> - ASR_Information *asr;
> - rtems_signal_set signal_set;
> - rtems_mode prev_mode;
> + RTEMS_API_Control *api;
> + ASR_Information *asr;
> + rtems_signal_set signal_set;
> + bool normal_is_preemptible;
> + uint32_t normal_cpu_time_budget;
> + Thread_CPU_budget_algorithms normal_budget_algorithm;
> + bool normal_asr_is_enabled;
> + uint32_t normal_isr_level;
> + uint32_t asr_isr_level;
> + bool prev_is_preemptible;
> + bool prev_asr_is_enabled;
>
> (void) action;
>
> @@ -48,16 +57,82 @@ static void _Signal_Action_handler(
> _Assert( signal_set != 0 );
> asr->signals_pending = 0;
>
> - _Thread_State_release( executing, lock_context );
> + /* Save normal mode */
> +
> + normal_is_preemptible = executing->is_preemptible;
> + normal_asr_is_enabled = asr->is_enabled;
> + normal_cpu_time_budget = executing->cpu_time_budget;
> + normal_budget_algorithm = executing->budget_algorithm;
> +
> + /* Set mode for ASR processing */
> +
> + executing->is_preemptible = _Modes_Is_preempt( asr->mode_set );
> + asr->is_enabled = !_Modes_Is_asr_disabled( asr->mode_set );
> + _Modes_Set_timeslice( executing, asr->mode_set );
> + asr_isr_level = _Modes_Get_interrupt_level( asr->mode_set );
>
> - rtems_task_mode( asr->mode_set, RTEMS_ALL_MODE_MASKS, &prev_mode );
> + if ( executing->is_preemptible && !normal_is_preemptible ) {
> + Per_CPU_Control *cpu_self;
> +
> + cpu_self = _Thread_Dispatch_disable_critical( lock_context );
> + _Scheduler_Schedule( executing );
> + _Thread_State_release( executing, lock_context );
> + _Thread_Dispatch_direct( cpu_self );
> + } else {
> + _Thread_State_release( executing, lock_context );
> + }
> +
> + normal_isr_level = _ISR_Get_level();
> + _ISR_Set_level( asr_isr_level );
>
> /* Call the ASR handler in the ASR processing mode */
> ( *asr->handler )( signal_set );
>
> - rtems_task_mode( prev_mode, RTEMS_ALL_MODE_MASKS, &prev_mode );
> + /* Restore normal mode */
> +
> + _ISR_Set_level( normal_isr_level );
>
> _Thread_State_acquire( executing, lock_context );
> +
> + executing->cpu_time_budget = normal_cpu_time_budget ;
> + executing->budget_algorithm = normal_budget_algorithm ;
> + prev_is_preemptible = executing->is_preemptible;
'prev' is unclear here, especially since we have "normal" as a
previous mode also. Maybe, 'asr_is_preemptible' is better?
> + executing->is_preemptible = normal_is_preemptible;
> +
> + /*
> + * We do the best to avoid recursion in the ASR processing. A well behaved
> + * application will disable ASR processing during ASR processing. In this
> + * case, ASR processing is currently disabled. We do now the thread
> dispatch
> + * necessary due to a re-enabled preemption mode. This helps to avoid
> doing
> + * the next round of ASR processing recursively in
> _Thread_Dispatch_direct().
> + */
> + if ( normal_is_preemptible && !prev_is_preemptible ) {
> + Per_CPU_Control *cpu_self;
> +
> + cpu_self = _Thread_Dispatch_disable_critical( lock_context );
> + _Scheduler_Schedule( executing );
> + _Thread_State_release( executing, lock_context );
> + _Thread_Dispatch_direct( cpu_self );
> + _Thread_State_acquire( executing, lock_context );
> + }
> +
> + /*
> + * Restore the normal ASR processing mode. If we enable ASR processing and
> + * there are pending signals, then add us as a post-switch action. The
> loop
> + * in _Thread_Run_post_switch_actions() will continue after our return and
> + * call us again. This avoids a recursion.
> + */
> +
> + prev_asr_is_enabled = asr->is_enabled;
ditto, just use 'asr_is_enabled' variable here.
> + asr->is_enabled = normal_asr_is_enabled;
> +
> + if (
> + normal_asr_is_enabled &&
> + !prev_asr_is_enabled &&
no indent, align
only indent within the compound expression if you have to break the
simple expression (e.g., breaking at an inequality comparison)
https://docs.rtems.org/branches/master/eng/coding-formatting.html#breaking-long-lines
willing to buy: style formatter.
> + asr->signals_pending != 0
> + ) {
> + _Thread_Append_post_switch_action( executing, action );
> + }
> }
>
> rtems_status_code rtems_signal_send(
> --
> 2.26.2
>
> _______________________________________________
> 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
