Re: [Powerpc-utils-devel] [PATCH] Fix ppc64_cpu --threads-per-core when --smt value set

[Nathan F. <nf...@li...>]

On 01/07/2015 11:41 PM, Mamatha Inamdar wrote:
> 
> On 01/08/2015 01:33 AM, Nathan Fontenot wrote:
>> On 01/07/2015 01:01 AM, Vasant Hegde wrote:
>>> On 12/23/2014 12:08 PM, Mamatha Inamdar wrote:
>>>> Problem: ppc64_cpu --threads-per-core gives wrong data when --smt value set
>>>>
>>>> This patch will resolve the above issue and display the correct values of threads-per-core
>>>> for the ppc64_cpu command when --smt value is set.
>>>>
>>>> I have used the following equation to calculate threads_per_core as per lscpu command.
>>>>
>>>>     threads_per_core = nthreads / ncores
>> I think there may be a slight misunderstanding between the threads-per-core
>> option for the ppc64_cou command and what lscpu reports.
>>
>> The threads-per-core option to the ppc64_cpu command reports the number of
>> threads that a core contains, not the number of online threads per core.
>>
>> If you want to know the number of online threads per core you need to use the
>> --info or --smt option.
>>
>> -Nathan
> 
> You mean threads-per-core option to the ppc64_cpu is working as expected.
> 
> If yes then
> 
> 1) Test team has to change there test case because they are comparing
>      results with "ppc64_cpu  --threads-per-core" with "lscpu threads(s) per core".
> 
> Case 2: FAIL
> [root@lop826 ~]# ppc64_cpu --smt=4
> [root@lop826 ~]# ppc64_cpu --threads-per-core
> Threads per core: 8
> 
> Verify with "lscpu" output:
> Thread(s) per core: 4
> 
> 2) Can we change option name for ppc64_cpu..
> because  "--threads-per-core" and  "threads(s) per core" both are same but gives different results
>

I don't think changing the name of the option is needed, the option asks
for the number of threads per core and that is what it reports. The total
number of threads per core and the number of online threads in a core, usually
referred to as the smt state, are separate things that should be reported
separately. 

The lscpu command reports the number of online threads per core but labels it
as threads per core which I find slightly misleading. This doesn't tell you
how threads per core the system has, it tells you how threads are online, or
the smt state.

-Nathan
 
>>>> ***********************************************
>>>> Test results:
>>>>
>>>> CASE 1:
>>>>
>>>> ./src/ppc64_cpu --smt=on
>>>> [root@llmjuno03b powerpc-utils]# ./src/ppc64_cpu --threads-per-core
>>>> Threads per core: 4
>>>>
>>>> lscpu output:
>>>> Thread(s) per core:    4
>>>> ------------------------------------------
>>>> CASE 2:
>>>>
>>>> ./src/ppc64_cpu --smt=2
>>>> [root@llmjuno03b powerpc-utils]# ./src/ppc64_cpu --threads-per-core
>>>> Threads per core: 2
>>>>
>>>> lscpu output:
>>>> Thread(s) per core:    2
>>>> --------------------------------------------------
>>>> CASE 3:
>>>>
>>>> ./src/ppc64_cpu --smt=1
>>>> [root@llmjuno03b powerpc-utils]# ./src/ppc64_cpu --threads-per-core
>>>> Threads per core: 1
>>>>
>>>> lscpu output:
>>>> Thread(s) per core:    1
>>>> -------------------------------------------------
>>>>
>>>> Signed-off-by: Mamatha Inamdar <mam...@li...>
>>>> ---
>>>>    src/ppc64_cpu.c |   15 ++++++++++++++-
>>>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/ppc64_cpu.c b/src/ppc64_cpu.c
>>>> index 4df4cdc..a3cfed0 100644
>>>> --- a/src/ppc64_cpu.c
>>>> +++ b/src/ppc64_cpu.c
>>>> @@ -56,6 +56,7 @@ static int counters[MAX_NR_CPUS];
>>>>    static int threads_per_cpu = 0;
>>>>    static int cpus_in_system = 0;
>>>>    static int threads_in_system = 0;
>>>> +static int threads_per_core = 0;
>>> Why do you need another global variable ? can't you just print " nthreads /
>>> cpus_in_system" ?
>>>
>>>>    static int do_info(void);
>>>>
>>>> @@ -529,9 +530,21 @@ static int do_smt(char *state)
>>>>        return rc;
>>>>    }
>>>>
>>>> +/* Threads per core */
>>>>    static inline void do_threads_per_core()
>>>>    {
>>>> -    printf("Threads per core: %d\n", threads_per_cpu);
>>>> +    int i, j;
>>>> +    int nthreads;    /* number of threads online */
>>> initialize nthreads ?
>>>
>>>
>>>> +
>>>> +    /* Calculate number of threads present on the system */
>>>> +    for (j = 0; j < threads_in_system; j += threads_per_cpu) {
>>>> +        for (i = 0; i < threads_per_cpu; i++)
>>>> +            nthreads += cpu_online(j + i);
>>>> +    }
>>> Can we have different cores with different SMT level?
>>>
>>> -Vasant
>>>
>>>> +
>>>> +    /* Calculate threads per core = nthreads / ncores */
>>>> +    threads_per_core = nthreads / cpus_in_system;
>>>> +    printf("Threads per core: %d\n", threads_per_core);
>>>>    }
>>>>
>>>>    static int do_subcores_per_core(char *state)
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Dive into the World of Parallel Programming! The Go Parallel Website,
>>>> sponsored by Intel and developed in partnership with Slashdot Media, is your
>>>> hub for all things parallel software development, from weekly thought
>>>> leadership blogs to news, videos, case studies, tutorials and more. Take a
>>>> look and join the conversation now. http://goparallel.sourceforge.net
>>>> _______________________________________________
>>>> Powerpc-utils-devel mailing list
>>>> Pow...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/powerpc-utils-devel
>>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Dive into the World of Parallel Programming! The Go Parallel Website,
>>> sponsored by Intel and developed in partnership with Slashdot Media, is your
>>> hub for all things parallel software development, from weekly thought
>>> leadership blogs to news, videos, case studies, tutorials and more. Take a
>>> look and join the conversation now. http://goparallel.sourceforge.net
>>> _______________________________________________
>>> Powerpc-utils-devel mailing list
>>> Pow...@li...
>>> https://lists.sourceforge.net/lists/listinfo/powerpc-utils-devel
>>>
>