[PATCH 14/17] score: Thread life cycle re-implementation

[Gedare Bloom]

On Tue, Mar 25, 2014 at 8:49 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
[...]
> --- a/cpukit/score/include/rtems/score/thread.h
> +++ b/cpukit/score/include/rtems/score/thread.h
> @@ -11,6 +11,8 @@
>   *  COPYRIGHT (c) 1989-2014.
>   *  On-Line Applications Research Corporation (OAR).
>   *
> + *  Copyright (c) 2014 embedded brains GmbH.
> + *
>   *  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.
> @@ -412,8 +414,47 @@ typedef struct {
>    Chain_Control Chain;
>  } Thread_Action_control;
>
> +/**
> + * @brief Thread life states.
> + *
> + * The thread life states are orthogonal to the thread states used for
> + * synchronization primitives and blocking operations.  They reflect the state
> + * changes triggered with thread restart and delete requests.
> + */
> +typedef enum {
> +  THREAD_LIFE_NORMAL = 0x0,
> +  THREAD_LIFE_PROTECTED = 0x1,
> +  THREAD_LIFE_RESTARTED = 0x2,
> +  THREAD_LIFE_PROTECTED_RESTARTED = 0x3,
> +  THREAD_LIFE_TERMINATED = 0x4,
> +  THREAD_LIFE_PROTECTED_TERMINATED = 0x5,
> +  THREAD_LIFE_RESTARTED_TERMINATED = 0x6,
> +  THREAD_LIFE_PROTECTED_RESTARTED_TERMINATED = 0x7
> +} Thread_Life_state;
> +
I would prefer the present tense (restarting, terminating) instead of
the past tense for restarted and terminated. I think it is easier to
follow then, because the life state flags are set while the
restart/terminate is happening. Protected is fine.

> +/**
> + * @brief Thread life control.
> + */
>  typedef struct {
> +  /**
> +   * @brief Thread life action used to react upon thread restart and delete
> +   * requests.
> +   */
>    Thread_Action      Action;
> +
> +  /**
> +   * @brief The current thread life state.
> +   */
> +  Thread_Life_state  state;
> +
> +  /**
> +   * @brief The terminator thread of this thread.
> +   *
> +   * In case the thread is terminated and another thread (the terminator) waits
> +   * for the actual termination completion, then this field references the
> +   * terminator thread.
> +   */
> +  Thread_Control    *terminator;
>  } Thread_Life_control;
>
>  /**
> @@ -486,9 +527,10 @@ struct Thread_Control_struct {
>     * thread and thread dispatching is necessary.  On SMP a thread dispatch on a
>     * remote processor needs help from an inter-processor interrupt, thus it
>     * will take some time to complete the state change.  A lot of things can
> -   * happen in the meantime.
> +   * happen in the meantime.  This field is volatile since it is polled in
> +   * _Thread_Kill_zombies().
>     */
> -  bool                                  is_executing;
> +  volatile bool                         is_executing;
>
>  #if __RTEMS_HAVE_SYS_CPUSET_H__
>    /**
> diff --git a/cpukit/score/include/rtems/score/threadimpl.h b/cpukit/score/include/rtems/score/threadimpl.h
> index 2e31753..9166e9d 100644
> --- a/cpukit/score/include/rtems/score/threadimpl.h
> +++ b/cpukit/score/include/rtems/score/threadimpl.h
> @@ -11,6 +11,8 @@
>   *  COPYRIGHT (c) 1989-2008.
>   *  On-Line Applications Research Corporation (OAR).
>   *
> + *  Copyright (c) 2014 embedded brains GmbH.
> + *
>   *  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.
> @@ -194,6 +196,8 @@ bool _Thread_Restart(
>    Thread_Entry_numeric_type  numeric_argument
>  );
>
> +bool _Thread_Set_life_protection( bool protect );
> +
>  void _Thread_Life_action_handler(
>    Thread_Control  *executing,
>    Thread_Action   *action,
> @@ -201,17 +205,9 @@ void _Thread_Life_action_handler(
>    ISR_Level        level
>  );
>
> -/**
> - *  @brief Frees all memory associated with the specified thread.
> - *
> - *  This routine frees all memory associated with the specified
> - *  thread and removes it from the local object table so no further
> - *  operations on this thread are allowed.
> - */
> -void _Thread_Close(
> -  Objects_Information  *information,
> -  Thread_Control       *the_thread
> -);
> +void _Thread_Kill_zombies( void );
> +
> +void _Thread_Close( Thread_Control *the_thread, Thread_Control *executing );
>
Add doxygen for these two functions. Can be done in separate patch.

>  /**
>   *  @brief Removes any set states for @a the_thread.
> @@ -710,6 +706,34 @@ RTEMS_INLINE_ROUTINE void _Thread_Add_post_switch_action(
>    _Thread_Action_release_and_ISR_enable( cpu, level );
>  }
>
> +RTEMS_INLINE_ROUTINE bool _Thread_Is_life_restarted(
> +  Thread_Life_state life_state
> +)
> +{
> +  return ( life_state & THREAD_LIFE_RESTARTED ) != 0;
> +}
> +
> +RTEMS_INLINE_ROUTINE bool _Thread_Is_life_terminated(
> +  Thread_Life_state life_state
> +)
> +{
> +  return ( life_state & THREAD_LIFE_TERMINATED ) != 0;
> +}
> +
> +RTEMS_INLINE_ROUTINE bool _Thread_Is_life_protected(
> +  Thread_Life_state life_state
> +)
> +{
> +  return ( life_state & THREAD_LIFE_PROTECTED ) != 0;
> +}
> +
> +RTEMS_INLINE_ROUTINE bool _Thread_Is_life_change_requested(
> +  Thread_Life_state life_state
> +)
> +{
> +  return ( life_state & THREAD_LIFE_RESTARTED_TERMINATED ) != 0;
> +}
> +
And also doxygen for these functions, and consider using the present
tense for restarting, terminating

Otherwise this looks OK to me.
Gedare