[Qemu-devel] [PATCH] topology.h: cleanup

[Qemu-devel] [PATCH] topology.h: cleanup

[Cao jin]

remove useless parameter of several functions

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 include/hw/i386/topology.h | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 148cc1b..d0f473c 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -64,32 +64,31 @@ static unsigned apicid_bitwidth_for_count(unsigned count)
 
 /* Bit width of the SMT_ID (thread ID) field on the APIC ID
  */
-static inline unsigned apicid_smt_width(unsigned nr_cores, unsigned nr_threads)
+static inline unsigned apicid_smt_width(unsigned nr_threads)
 {
     return apicid_bitwidth_for_count(nr_threads);
 }
 
 /* Bit width of the Core_ID field
  */
-static inline unsigned apicid_core_width(unsigned nr_cores, unsigned nr_threads)
+static inline unsigned apicid_core_width(unsigned nr_cores)
 {
     return apicid_bitwidth_for_count(nr_cores);
 }
 
-/* Bit offset of the Core_ID field
+/* Bit offset of the Core_ID field, equals bit width of SMT_ID
  */
-static inline unsigned apicid_core_offset(unsigned nr_cores,
-                                          unsigned nr_threads)
+static inline unsigned apicid_core_offset(unsigned nr_threads)
 {
-    return apicid_smt_width(nr_cores, nr_threads);
+    return apicid_smt_width(nr_threads);
 }
 
 /* Bit offset of the Pkg_ID (socket ID) field
  */
 static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads)
 {
-    return apicid_core_offset(nr_cores, nr_threads) +
-           apicid_core_width(nr_cores, nr_threads);
+    return apicid_core_offset(nr_threads) +
+           apicid_core_width(nr_cores);
 }
 
 /* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
@@ -101,7 +100,7 @@ static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
                                              const X86CPUTopoInfo *topo)
 {
     return (topo->pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
-           (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
+           (topo->core_id << apicid_core_offset(nr_threads)) |
            topo->smt_id;
 }
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH] topology.h: cleanup

[Eduardo Habkost]

On Tue, Feb 23, 2016 at 02:51:11PM +0800, Cao jin wrote:
> remove useless parameter of several functions
> 
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>

I believe this makes the API much easier to be misused. Currently
you can safely use (smp_cores, smp_threads) every time. With this
patch, you need to always check the function definition to find
out what's the correct argument.

> ---
>  include/hw/i386/topology.h | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index 148cc1b..d0f473c 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -64,32 +64,31 @@ static unsigned apicid_bitwidth_for_count(unsigned count)
>  
>  /* Bit width of the SMT_ID (thread ID) field on the APIC ID
>   */
> -static inline unsigned apicid_smt_width(unsigned nr_cores, unsigned nr_threads)
> +static inline unsigned apicid_smt_width(unsigned nr_threads)
>  {
>      return apicid_bitwidth_for_count(nr_threads);
>  }
>  
>  /* Bit width of the Core_ID field
>   */
> -static inline unsigned apicid_core_width(unsigned nr_cores, unsigned nr_threads)
> +static inline unsigned apicid_core_width(unsigned nr_cores)
>  {
>      return apicid_bitwidth_for_count(nr_cores);
>  }
>  
> -/* Bit offset of the Core_ID field
> +/* Bit offset of the Core_ID field, equals bit width of SMT_ID
>   */
> -static inline unsigned apicid_core_offset(unsigned nr_cores,
> -                                          unsigned nr_threads)
> +static inline unsigned apicid_core_offset(unsigned nr_threads)
>  {
> -    return apicid_smt_width(nr_cores, nr_threads);
> +    return apicid_smt_width(nr_threads);
>  }
>  
>  /* Bit offset of the Pkg_ID (socket ID) field
>   */
>  static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads)
>  {
> -    return apicid_core_offset(nr_cores, nr_threads) +
> -           apicid_core_width(nr_cores, nr_threads);
> +    return apicid_core_offset(nr_threads) +
> +           apicid_core_width(nr_cores);
>  }
>  
>  /* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
> @@ -101,7 +100,7 @@ static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
>                                               const X86CPUTopoInfo *topo)
>  {
>      return (topo->pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
> -           (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
> +           (topo->core_id << apicid_core_offset(nr_threads)) |
>             topo->smt_id;
>  }
>  
> -- 
> 2.1.0
> 
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] topology.h: cleanup

[Cao jin]

On 02/24/2016 05:16 AM, Eduardo Habkost wrote:
> On Tue, Feb 23, 2016 at 02:51:11PM +0800, Cao jin wrote:
>> remove useless parameter of several functions
>>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>
> I believe this makes the API much easier to be misused. Currently
> you can safely use (smp_cores, smp_threads) every time. With this
> patch, you need to always check the function definition to find
> out what's the correct argument.
>

Yes, you do have a point here, and very interesting to me. A caller need 
to know/check how to use a api if it want to use it, seems a nature 
thing to me.

Thanks for your explanation.

-- 
Yours Sincerely,

Cao jin