[PATCH 7/7] score: Hide SMP lock profiling impl if disabled

Gedare Bloom gedare at gwu.edu
Thu Jun 25 13:32:10 UTC 2015


This code is quite a pain to read with the interleaved ifdefs and I'm
not that happy about it, but I don't have any better suggestions right
now. The set of patches looks fine to commit.

On Thu, Jun 25, 2015 at 9:17 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> The problem is that empty structures have a different size in C and C++.
> ---
>  cpukit/score/include/rtems/score/percpu.h  |  17 ++-
>  cpukit/score/include/rtems/score/smplock.h | 167 +++++++++++++++++------------
>  cpukit/score/src/profilingsmplock.c        |  18 ++--
>  cpukit/score/src/smp.c                     |   5 +-
>  4 files changed, 127 insertions(+), 80 deletions(-)
>
> diff --git a/cpukit/score/include/rtems/score/percpu.h b/cpukit/score/include/rtems/score/percpu.h
> index a6f7a25..088c31f 100644
> --- a/cpukit/score/include/rtems/score/percpu.h
> +++ b/cpukit/score/include/rtems/score/percpu.h
> @@ -319,10 +319,17 @@ typedef struct Per_CPU_Control {
>       */
>      SMP_ticket_lock_Control Lock;
>
> -    /**
> -     * @brief Lock statistics context for the per-CPU lock.
> -     */
> -    SMP_lock_Stats_context Lock_stats_context;
> +    #if defined( RTEMS_PROFILING )
> +      /**
> +       * @brief Lock statistics for the per-CPU lock.
> +       */
> +      SMP_lock_Stats Lock_stats;
> +
> +      /**
> +       * @brief Lock statistics context for the per-CPU lock.
> +       */
> +      SMP_lock_Stats_context Lock_stats_context;
> +    #endif
>
>      /**
>       * @brief Context for the Giant lock acquire and release pair of this
> @@ -385,6 +392,7 @@ extern Per_CPU_Control_envelope _Per_CPU_Information[] CPU_STRUCTURE_ALIGNMENT;
>  #define _Per_CPU_Acquire( cpu ) \
>    _SMP_ticket_lock_Acquire( \
>      &( cpu )->Lock, \
> +    &( cpu )->Lock_stats, \
>      &( cpu )->Lock_stats_context \
>    )
>  #else
> @@ -398,6 +406,7 @@ extern Per_CPU_Control_envelope _Per_CPU_Information[] CPU_STRUCTURE_ALIGNMENT;
>  #define _Per_CPU_Release( cpu ) \
>    _SMP_ticket_lock_Release( \
>      &( cpu )->Lock, \
> +    &( cpu )->Lock_stats, \
>      &( cpu )->Lock_stats_context \
>    )
>  #else
> diff --git a/cpukit/score/include/rtems/score/smplock.h b/cpukit/score/include/rtems/score/smplock.h
> index 50a0662..b56a64b 100644
> --- a/cpukit/score/include/rtems/score/smplock.h
> +++ b/cpukit/score/include/rtems/score/smplock.h
> @@ -58,6 +58,8 @@ extern "C" {
>   * @{
>   */
>
> +#if defined( RTEMS_PROFILING )
> +
>  /**
>   * @brief Count of lock contention counters for lock statistics.
>   */
> @@ -82,7 +84,6 @@ extern "C" {
>   * instant and the lock acquire instant.
>   */
>  typedef struct {
> -#if defined( RTEMS_PROFILING )
>    /**
>     * @brief Node for SMP lock statistics chain.
>     */
> @@ -142,33 +143,25 @@ typedef struct {
>     * @brief The lock name.
>     */
>    const char *name;
> -#endif /* defined( RTEMS_PROFILING ) */
>  } SMP_lock_Stats;
>
>  /**
>   * @brief Local context for SMP lock statistics.
>   */
>  typedef struct {
> -#if defined( RTEMS_PROFILING )
>    /**
>     * @brief The last lock acquire instant in CPU counter ticks.
>     *
>     * This value is used to measure the lock section time.
>     */
>    CPU_Counter_ticks acquire_instant;
> -#endif
>  } SMP_lock_Stats_context;
>
>  /**
>   * @brief SMP lock statistics initializer for static initialization.
>   */
> -#if defined( RTEMS_PROFILING )
>  #define SMP_LOCK_STATS_INITIALIZER( name ) \
>    { { NULL, NULL }, 0, 0, 0, 0, { 0, 0, 0, 0 }, 0, name }
> -#else
> -#define SMP_LOCK_STATS_INITIALIZER( name ) \
> -  { }
> -#endif
>
>  /**
>   * @brief Initializes an SMP lock statistics block.
> @@ -190,14 +183,14 @@ static inline void _SMP_lock_Stats_initialize(
>  /**
>   * @brief Destroys an SMP lock statistics block.
>   *
> - * @param[in,out] stats The SMP lock statistics block.
> + * @param[in] stats The SMP lock statistics block.
>   */
>  static inline void _SMP_lock_Stats_destroy( SMP_lock_Stats *stats );
>
>  /**
>   * @brief Destroys an SMP lock statistics block.
>   *
> - * @param[in,out] stats The SMP lock statistics block.
> + * @param[in] stats The SMP lock statistics block.
>   * @param[in] stats_context The SMP lock statistics context.
>   */
>  static inline void _SMP_lock_Stats_release_update(
> @@ -205,23 +198,23 @@ static inline void _SMP_lock_Stats_release_update(
>    const SMP_lock_Stats_context *stats_context
>  );
>
> +#endif /* RTEMS_PROFILING */
> +
>  /**
>   * @brief SMP ticket lock control.
>   */
>  typedef struct {
>    Atomic_Uint next_ticket;
>    Atomic_Uint now_serving;
> -  SMP_lock_Stats Stats;
>  } SMP_ticket_lock_Control;
>
>  /**
>   * @brief SMP ticket lock control initializer for static initialization.
>   */
> -#define SMP_TICKET_LOCK_INITIALIZER( name ) \
> +#define SMP_TICKET_LOCK_INITIALIZER \
>    { \
>      ATOMIC_INITIALIZER_UINT( 0U ), \
> -    ATOMIC_INITIALIZER_UINT( 0U ), \
> -    SMP_LOCK_STATS_INITIALIZER( name ) \
> +    ATOMIC_INITIALIZER_UINT( 0U ) \
>    }
>
>  /**
> @@ -229,18 +222,16 @@ typedef struct {
>   *
>   * Concurrent initialization leads to unpredictable results.
>   *
> - * @param[in,out] lock The SMP ticket lock control.
> + * @param[in] lock The SMP ticket lock control.
>   * @param[in] name The name for the SMP ticket lock.  This name must be
>   * persistent throughout the life time of this lock.
>   */
>  static inline void _SMP_ticket_lock_Initialize(
> -  SMP_ticket_lock_Control *lock,
> -  const char *name
> +  SMP_ticket_lock_Control *lock
>  )
>  {
>    _Atomic_Init_uint( &lock->next_ticket, 0U );
>    _Atomic_Init_uint( &lock->now_serving, 0U );
> -  _SMP_lock_Stats_initialize( &lock->Stats, name );
>  }
>
>  /**
> @@ -248,33 +239,26 @@ static inline void _SMP_ticket_lock_Initialize(
>   *
>   * Concurrent destruction leads to unpredictable results.
>   *
> - * @param[in,out] lock The SMP ticket lock control.
> + * @param[in] lock The SMP ticket lock control.
>   */
>  static inline void _SMP_ticket_lock_Destroy( SMP_ticket_lock_Control *lock )
>  {
> -  _SMP_lock_Stats_destroy( &lock->Stats );
> +  (void) lock;
>  }
>
> -/**
> - * @brief Acquires an SMP ticket lock.
> - *
> - * This function will not disable interrupts.  The caller must ensure that the
> - * current thread of execution is not interrupted indefinite once it obtained
> - * the SMP ticket lock.
> - *
> - * @param[in,out] lock The SMP ticket lock control.
> - * @param[out] stats_context The SMP lock statistics context.
> - */
> -static inline void _SMP_ticket_lock_Acquire(
> -  SMP_ticket_lock_Control *lock,
> +static inline void _SMP_ticket_lock_Do_acquire(
> +  SMP_ticket_lock_Control *lock
> +#if defined( RTEMS_PROFILING )
> +  ,
> +  SMP_lock_Stats *stats,
>    SMP_lock_Stats_context *stats_context
> +#endif
>  )
>  {
>    unsigned int my_ticket;
>    unsigned int now_serving;
>
>  #if defined( RTEMS_PROFILING )
> -  SMP_lock_Stats *stats = &lock->Stats;
>    CPU_Counter_ticks first;
>    CPU_Counter_ticks second;
>    CPU_Counter_ticks delta;
> @@ -324,30 +308,67 @@ static inline void _SMP_ticket_lock_Acquire(
>  }
>
>  /**
> - * @brief Releases an SMP ticket lock.
> + * @brief Acquires an SMP ticket lock.
>   *
> - * @param[in,out] lock The SMP ticket lock control.
> - * @param[in] stats_context The SMP lock statistics context.
> + * This function will not disable interrupts.  The caller must ensure that the
> + * current thread of execution is not interrupted indefinite once it obtained
> + * the SMP ticket lock.
> + *
> + * @param[in] lock The SMP ticket lock control.
> + * @param[in] stats The SMP lock statistics.
> + * @param[out] stats_context The SMP lock statistics context.
>   */
> -static inline void _SMP_ticket_lock_Release(
> -  SMP_ticket_lock_Control *lock,
> +#if defined( RTEMS_PROFILING )
> +  #define _SMP_ticket_lock_Acquire( lock, stats, stats_context ) \
> +    _SMP_ticket_lock_Do_acquire( lock, stats, stats_context )
> +#else
> +  #define _SMP_ticket_lock_Acquire( lock, stats, stats_context ) \
> +    _SMP_ticket_lock_Do_acquire( lock )
> +#endif
> +
> +static inline void _SMP_ticket_lock_Do_release(
> +  SMP_ticket_lock_Control *lock
> +#if defined( RTEMS_PROFILING )
> +  ,
> +  SMP_lock_Stats *stats,
>    const SMP_lock_Stats_context *stats_context
> +#endif
>  )
>  {
>    unsigned int current_ticket =
>      _Atomic_Load_uint( &lock->now_serving, ATOMIC_ORDER_RELAXED );
>    unsigned int next_ticket = current_ticket + 1U;
>
> -  _SMP_lock_Stats_release_update( &lock->Stats, stats_context );
> +#if defined( RTEMS_PROFILING )
> +  _SMP_lock_Stats_release_update( stats, stats_context );
> +#endif
>
>    _Atomic_Store_uint( &lock->now_serving, next_ticket, ATOMIC_ORDER_RELEASE );
>  }
>
>  /**
> + * @brief Releases an SMP ticket lock.
> + *
> + * @param[in] lock The SMP ticket lock control.
> + * @param[in] stats The SMP lock statistics.
> + * @param[in] stats_context The SMP lock statistics context.
> + */
> +#if defined( RTEMS_PROFILING )
> +  #define _SMP_ticket_lock_Release( lock, stats, stats_context ) \
> +    _SMP_ticket_lock_Do_release( lock, stats, stats_context )
> +#else
> +  #define _SMP_ticket_lock_Release( lock, stats, stats_context ) \
> +    _SMP_ticket_lock_Do_release( lock )
> +#endif
> +
> +/**
>   * @brief SMP lock control.
>   */
>  typedef struct {
> -  SMP_ticket_lock_Control ticket_lock;
> +  SMP_ticket_lock_Control Ticket_lock;
> +#if defined( RTEMS_PROFILING )
> +  SMP_lock_Stats Stats;
> +#endif
>  } SMP_lock_Control;
>
>  /**
> @@ -355,20 +376,27 @@ typedef struct {
>   */
>  typedef struct {
>    ISR_Level isr_level;
> +#if defined( RTEMS_PROFILING )
>    SMP_lock_Stats_context Stats_context;
> +#endif
>  } SMP_lock_Context;
>
>  /**
>   * @brief SMP lock control initializer for static initialization.
>   */
> -#define SMP_LOCK_INITIALIZER( name ) { SMP_TICKET_LOCK_INITIALIZER( name ) }
> +#if defined( RTEMS_PROFILING )
> +  #define SMP_LOCK_INITIALIZER( name ) \
> +    { SMP_TICKET_LOCK_INITIALIZER, SMP_LOCK_STATS_INITIALIZER( name ) }
> +#else
> +  #define SMP_LOCK_INITIALIZER( name ) { SMP_TICKET_LOCK_INITIALIZER }
> +#endif
>
>  /**
>   * @brief Initializes an SMP lock.
>   *
>   * Concurrent initialization leads to unpredictable results.
>   *
> - * @param[in,out] lock The SMP lock control.
> + * @param[in] lock The SMP lock control.
>   * @param[in] name The name for the SMP lock statistics.  This name must be
>   * persistent throughout the life time of this statistics block.
>   */
> @@ -386,7 +414,12 @@ static inline void _SMP_lock_Initialize(
>    const char *name
>  )
>  {
> -  _SMP_ticket_lock_Initialize( &lock->ticket_lock, name );
> +  _SMP_ticket_lock_Initialize( &lock->Ticket_lock );
> +#if defined( RTEMS_PROFILING )
> +  _SMP_lock_Stats_initialize( &lock->Stats, name );
> +#else
> +  (void) name;
> +#endif
>  }
>
>  /**
> @@ -394,7 +427,7 @@ static inline void _SMP_lock_Initialize(
>   *
>   * Concurrent destruction leads to unpredictable results.
>   *
> - * @param[in,out] lock The SMP lock control.
> + * @param[in] lock The SMP lock control.
>   */
>  #if defined( RTEMS_SMP_LOCK_DO_NOT_INLINE )
>  void _SMP_lock_Destroy( SMP_lock_Control *lock );
> @@ -404,7 +437,8 @@ static inline void _SMP_lock_Destroy_body( SMP_lock_Control *lock )
>  static inline void _SMP_lock_Destroy( SMP_lock_Control *lock )
>  #endif
>  {
> -  _SMP_ticket_lock_Destroy( &lock->ticket_lock );
> +  _SMP_ticket_lock_Destroy( &lock->Ticket_lock );
> +  _SMP_lock_Stats_destroy( &lock->Stats );
>  }
>
>  /**
> @@ -414,8 +448,8 @@ static inline void _SMP_lock_Destroy( SMP_lock_Control *lock )
>   * current thread of execution is not interrupted indefinite once it obtained
>   * the SMP lock.
>   *
> - * @param[in,out] lock The SMP lock control.
> - * @param[in,out] context The local SMP lock context for an acquire and release
> + * @param[in] lock The SMP lock control.
> + * @param[in] context The local SMP lock context for an acquire and release
>   * pair.
>   */
>  #if defined( RTEMS_SMP_LOCK_DO_NOT_INLINE )
> @@ -433,14 +467,18 @@ static inline void _SMP_lock_Acquire(
>  )
>  {
>    (void) context;
> -  _SMP_ticket_lock_Acquire( &lock->ticket_lock, &context->Stats_context );
> +  _SMP_ticket_lock_Acquire(
> +    &lock->Ticket_lock,
> +    &lock->Stats,
> +    &context->Stats_context
> +  );
>  }
>
>  /**
>   * @brief Releases an SMP lock.
>   *
> - * @param[in,out] lock The SMP lock control.
> - * @param[in,out] context The local SMP lock context for an acquire and release
> + * @param[in] lock The SMP lock control.
> + * @param[in] context The local SMP lock context for an acquire and release
>   * pair.
>   */
>  #if defined( RTEMS_SMP_LOCK_DO_NOT_INLINE )
> @@ -458,14 +496,18 @@ static inline void _SMP_lock_Release(
>  )
>  {
>    (void) context;
> -  _SMP_ticket_lock_Release( &lock->ticket_lock, &context->Stats_context );
> +  _SMP_ticket_lock_Release(
> +    &lock->Ticket_lock,
> +    &lock->Stats,
> +    &context->Stats_context
> +  );
>  }
>
>  /**
>   * @brief Disables interrupts and acquires the SMP lock.
>   *
> - * @param[in,out] lock The SMP lock control.
> - * @param[in,out] context The local SMP lock context for an acquire and release
> + * @param[in] lock The SMP lock control.
> + * @param[in] context The local SMP lock context for an acquire and release
>   * pair.
>   */
>  #if defined( RTEMS_SMP_LOCK_DO_NOT_INLINE )
> @@ -489,8 +531,8 @@ static inline void _SMP_lock_ISR_disable_and_acquire(
>  /**
>   * @brief Releases the SMP lock and enables interrupts.
>   *
> - * @param[in,out] lock The SMP lock control.
> - * @param[in,out] context The local SMP lock context for an acquire and release
> + * @param[in] lock The SMP lock control.
> + * @param[in] context The local SMP lock context for an acquire and release
>   * pair.
>   */
>  #if defined( RTEMS_SMP_LOCK_DO_NOT_INLINE )
> @@ -512,6 +554,7 @@ static inline void _SMP_lock_Release_and_ISR_enable(
>  }
>
>  #if defined( RTEMS_PROFILING )
> +
>  typedef struct {
>    SMP_lock_Control Lock;
>    Chain_Control Stats_chain;
> @@ -596,11 +639,9 @@ static inline void _SMP_lock_Stats_iteration_stop(
>    _Chain_Extract_unprotected( &iteration_context->Node );
>    _SMP_lock_Release_and_ISR_enable( &control->Lock, &lock_context );
>  }
> -#endif
>
>  static inline void _SMP_lock_Stats_destroy( SMP_lock_Stats *stats )
>  {
> -#if defined( RTEMS_PROFILING )
>    if ( !_Chain_Is_node_off_chain( &stats->Node ) ) {
>      SMP_lock_Stats_control *control = &_SMP_lock_Stats_control;
>      SMP_lock_Context lock_context;
> @@ -629,9 +670,6 @@ static inline void _SMP_lock_Stats_destroy( SMP_lock_Stats *stats )
>
>      _SMP_lock_Release_and_ISR_enable( &control->Lock, &lock_context );
>    }
> -#else
> -  (void) stats;
> -#endif
>  }
>
>  static inline void _SMP_lock_Stats_release_update(
> @@ -639,7 +677,6 @@ static inline void _SMP_lock_Stats_release_update(
>    const SMP_lock_Stats_context *stats_context
>  )
>  {
> -#if defined( RTEMS_PROFILING )
>    CPU_Counter_ticks first = stats_context->acquire_instant;
>    CPU_Counter_ticks second = _CPU_Counter_read();
>    CPU_Counter_ticks delta = _CPU_Counter_difference( second, first );
> @@ -658,12 +695,10 @@ static inline void _SMP_lock_Stats_release_update(
>        _SMP_lock_Release_and_ISR_enable( &control->Lock, &lock_context );
>      }
>    }
> -#else
> -  (void) stats;
> -  (void) stats_context;
> -#endif
>  }
>
> +#endif /* RTEMS_PROFILING */
> +
>  /**@}*/
>
>  #ifdef __cplusplus
> diff --git a/cpukit/score/src/profilingsmplock.c b/cpukit/score/src/profilingsmplock.c
> index be60ba9..a77e1a1 100644
> --- a/cpukit/score/src/profilingsmplock.c
> +++ b/cpukit/score/src/profilingsmplock.c
> @@ -21,19 +21,19 @@
>  #if defined( RTEMS_PROFILING )
>  SMP_lock_Stats_control _SMP_lock_Stats_control = {
>    .Lock = {
> -    .ticket_lock = {
> +    .Ticket_lock = {
>        .next_ticket = ATOMIC_INITIALIZER_UINT( 0U ),
> -      .now_serving = ATOMIC_INITIALIZER_UINT( 0U ),
> -      .Stats = {
> -        .Node = CHAIN_NODE_INITIALIZER_ONE_NODE_CHAIN(
> -          &_SMP_lock_Stats_control.Stats_chain
> -        ),
> -        .name = "SMP lock stats"
> -      }
> +      .now_serving = ATOMIC_INITIALIZER_UINT( 0U )
> +    },
> +    .Stats = {
> +      .Node = CHAIN_NODE_INITIALIZER_ONE_NODE_CHAIN(
> +        &_SMP_lock_Stats_control.Stats_chain
> +      ),
> +      .name = "SMP Lock Stats"
>      }
>    },
>    .Stats_chain = CHAIN_INITIALIZER_ONE_NODE(
> -    &_SMP_lock_Stats_control.Lock.ticket_lock.Stats.Node
> +    &_SMP_lock_Stats_control.Lock.Stats.Node
>    ),
>    .Iterator_chain = CHAIN_INITIALIZER_EMPTY(
>      _SMP_lock_Stats_control.Iterator_chain
> diff --git a/cpukit/score/src/smp.c b/cpukit/score/src/smp.c
> index 8ffeb1d..8e01034 100644
> --- a/cpukit/score/src/smp.c
> +++ b/cpukit/score/src/smp.c
> @@ -80,7 +80,10 @@ void _SMP_Handler_initialize( void )
>    for ( cpu_index = 0 ; cpu_index < cpu_max; ++cpu_index ) {
>      Per_CPU_Control *cpu = _Per_CPU_Get_by_index( cpu_index );
>
> -    _SMP_ticket_lock_Initialize( &cpu->Lock, "per-CPU" );
> +    _SMP_ticket_lock_Initialize( &cpu->Lock );
> +#if defined(RTEMS_PROFILING)
> +    _SMP_lock_Stats_initialize( &cpu->Lock_stats, "Per-CPU" );
> +#endif
>    }
>
>    /*
> --
> 1.8.4.5
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel



More information about the devel mailing list