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 <address@hidden>

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