| Message ID | 1456210271-8119-1-git-send-email-caoj.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > > >
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.
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; }
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(-)