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

[Chris Johns]

On 26/06/2015 5:01 pm, Sebastian Huber wrote:
> 
> 
> On 26/06/15 01:45, Chris Johns wrote:
>> On 25/06/2015 11:17 pm, Sebastian Huber 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
>>>   
>> My only comment is the ability to get any SMP status is difficult. It is
>> either all on or you get nothing. Moving all stats related defines
>> inside #ifdefs and into the global RTEMS_PROFILING define make this
>> harder to do. This is not specifically addressing this lock stats change
>> but on a more general level and this change reminded me.
> 
> This patch changes nothing in terms of functionality.  It just avoids
> zero size structures which unfortunately are implementation-defined in C
> (size is zero with GCC) and C++ (size must be non-zero and is 1 in GCC).

I understand the purpose of the patch and it is fine.

> 
>>
>> I looked into adding some SMP stats to top not long ago I found there
>> was nothing I report on unless profiling was on and if I turned on
>> profiling I got too much such as timing and other nice features.
>>
>> I could not see how to show how many context switches each core had, how
>> many interrupts each core serviced.
> 
> We can enable such things individually.
> 

Then may I suggest we default them to always so code that references
these values can avoid the condition compile options to get at them.

>>
>> I understand the desire to push the limits and not waste a single cycle
>> if we can help it but I actually found it difficult to know if I had SMP
>> built correctly and if it was working from a shell command line. I feel
>> we need some standard stats users can depend on that help us know things
>> are basically working.
>>
>> Chris
> 
> Is this not something that can be done with the linker based tracing?
> 

It is about working production system with an issue. Being able to run
top and see things are happy is a real bonus.

Chris