[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