Re: [RFC PATCH v2 1/2] powerpc: cleanup APIs for cpu/thread/core mappings

2010-05-17 Thread Vaidyanathan Srinivasan
* Vaidyanathan Srinivasan sva...@linux.vnet.ibm.com [2010-05-10 11:18:01]:

 * Paul Mackerras pau...@samba.org [2010-05-10 09:05:22]:
 
  On Fri, May 07, 2010 at 05:18:42PM +0530, Vaidyanathan Srinivasan wrote:
  
   These APIs take logical cpu number as input
   Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling()
   Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling()
   
   These APIs convert core number (index) to logical cpu/thread numbers
   Add cpu_first_thread_of_core(int core)
   Changed cpu_thread_to_core() to cpu_core_of_thread(int cpu)
  
  Why make all these changes?  The end result doesn't seem any cleaner
  or better than how it was before, and your patch description doesn't
  give any reason for us to think yes, we should make this change.
  I assume you think this is a good change to make, so you need to
  explain why it's a good idea and convince the rest of us.
 
 Sure Paul.. let me explain.  The crux of the issue is to make
 'threads_per_core' accessible to the pseries_energy module.  In the
 first RFC, I had directly exported the variable which is not a good
 design.  Ben H suggested to make an API around it and then export the
 function:
 http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-April/081610.html
 
 Instead of making an API to read threads_per_core, as Ben suggested,
 I have made a wrapper at a higher level to make an API to convert from
 logical cpu number to core number.
 
 The current APIs cpu_first_thread_in_core() and
 cpu_last_thread_in_core() returns logical CPU number while
 cpu_thread_to_core() returns core number or index which is not
 a logical CPU number.
 
 Ben recommended to clearly name them to distinguish 'core number'
 versus first and last 'logical cpu number' in that core.
 
 Hence in the new scheme, I have:
 
   Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling()
   Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling()
 
 which work on logical cpu numbers.  
 While cpu_first_thread_of_core() and cpu_core_of_thread() work on core
 index.
 
 Example usage:  (4 threads per core system)
 
 cpu_leftmost_thread_sibling(5) = 4
 cpu_rightmost_thread_sibling(5) = 7
 cpu_core_of_thread(5) = 1
 cpu_first_thread_of_core(1) = 4
 
 cpu_core_of_thread() is used in cpu_to_drc_index() in the module and
 cpu_first_thread_of_core() is used in drc_index_to_cpu() in the
 module.  These APIs may be useful in other modules in future, and the
 proposed design is a good method to export these APIs to modules.
 
 An alternative approach could be to move both the base functions
 cpu_to_drc_index() and drc_index_to_cpu() into the kernel like in
 arch/powerpc/kernel/smp.c
 
 Thanks for the review, I hope I have explained the requirements and
 design for this cleanup.

Hi Paul and Ben,

Do you have any further comments on this patch series and related
cleanup?  I will post the next iteration in a day or two.

Thanks,
Vaidy

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 1/2] powerpc: cleanup APIs for cpu/thread/core mappings

2010-05-09 Thread Paul Mackerras
On Fri, May 07, 2010 at 05:18:42PM +0530, Vaidyanathan Srinivasan wrote:

 These APIs take logical cpu number as input
 Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling()
 Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling()
 
 These APIs convert core number (index) to logical cpu/thread numbers
 Add cpu_first_thread_of_core(int core)
 Changed cpu_thread_to_core() to cpu_core_of_thread(int cpu)

Why make all these changes?  The end result doesn't seem any cleaner
or better than how it was before, and your patch description doesn't
give any reason for us to think yes, we should make this change.
I assume you think this is a good change to make, so you need to
explain why it's a good idea and convince the rest of us.

Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 1/2] powerpc: cleanup APIs for cpu/thread/core mappings

2010-05-09 Thread Vaidyanathan Srinivasan
* Paul Mackerras pau...@samba.org [2010-05-10 09:05:22]:

 On Fri, May 07, 2010 at 05:18:42PM +0530, Vaidyanathan Srinivasan wrote:
 
  These APIs take logical cpu number as input
  Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling()
  Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling()
  
  These APIs convert core number (index) to logical cpu/thread numbers
  Add cpu_first_thread_of_core(int core)
  Changed cpu_thread_to_core() to cpu_core_of_thread(int cpu)
 
 Why make all these changes?  The end result doesn't seem any cleaner
 or better than how it was before, and your patch description doesn't
 give any reason for us to think yes, we should make this change.
 I assume you think this is a good change to make, so you need to
 explain why it's a good idea and convince the rest of us.

Sure Paul.. let me explain.  The crux of the issue is to make
'threads_per_core' accessible to the pseries_energy module.  In the
first RFC, I had directly exported the variable which is not a good
design.  Ben H suggested to make an API around it and then export the
function:
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-April/081610.html

Instead of making an API to read threads_per_core, as Ben suggested,
I have made a wrapper at a higher level to make an API to convert from
logical cpu number to core number.

The current APIs cpu_first_thread_in_core() and
cpu_last_thread_in_core() returns logical CPU number while
cpu_thread_to_core() returns core number or index which is not
a logical CPU number.

Ben recommended to clearly name them to distinguish 'core number'
versus first and last 'logical cpu number' in that core.

Hence in the new scheme, I have:

  Change cpu_first_thread_in_core() to cpu_leftmost_thread_sibling()
  Change cpu_last_thread_in_core() to cpu_rightmost_thread_sibling()

which work on logical cpu numbers.  
While cpu_first_thread_of_core() and cpu_core_of_thread() work on core
index.

Example usage:  (4 threads per core system)

cpu_leftmost_thread_sibling(5) = 4
cpu_rightmost_thread_sibling(5) = 7
cpu_core_of_thread(5) = 1
cpu_first_thread_of_core(1) = 4

cpu_core_of_thread() is used in cpu_to_drc_index() in the module and
cpu_first_thread_of_core() is used in drc_index_to_cpu() in the
module.  These APIs may be useful in other modules in future, and the
proposed design is a good method to export these APIs to modules.

An alternative approach could be to move both the base functions
cpu_to_drc_index() and drc_index_to_cpu() into the kernel like in
arch/powerpc/kernel/smp.c

Thanks for the review, I hope I have explained the requirements and
design for this cleanup.

--Vaidy

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev