| Message ID | 20241218124900.60886-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/sched: Untangle credit2 vs cpu_nr_siblings() | expand |
On 18.12.24 13:49, Andrew Cooper wrote: > Credit2 has no buisness including asm/cpufeature.h or asm/processor.h. > > This was caused by a bad original abstraction, and an even less wise attempt > to fix the build on my behalf. It is also the sole reason why PPC and RISC-V > need cpufeature.h header. > > Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same > meaning between vendors on x86 CPUS. > > Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO. Drop > the stub from each architecture. > > Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") > Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 12/18/24 1:49 PM, Andrew Cooper wrote: > Credit2 has no buisness including asm/cpufeature.h or asm/processor.h. > > This was caused by a bad original abstraction, and an even less wise attempt > to fix the build on my behalf. It is also the sole reason why PPC and RISC-V > need cpufeature.h header. > > Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same > meaning between vendors on x86 CPUS. > > Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO. Drop > the stub from each architecture. > > Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") > Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)") > Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com> Reviewed-By: Oleksii Kurochko <oleksii.kurochko@gmail.com> Thanks. ~ Oleksii > --- > CC: Jan Beulich<JBeulich@suse.com> > CC: Roger Pau Monné<roger.pau@citrix.com> > CC: Stefano Stabellini<sstabellini@kernel.org> > CC: Julien Grall<julien@xen.org> > CC: Volodymyr Babchuk<Volodymyr_Babchuk@epam.com> > CC: Bertrand Marquis<bertrand.marquis@arm.com> > CC: Michal Orzel<michal.orzel@amd.com> > CC: Oleksii Kurochko<oleksii.kurochko@gmail.com> > CC: Shawn Anastasio<sanastasio@raptorengineering.com> > CC: Juergen Gross<jgross@suse.com> > --- > xen/arch/arm/include/asm/cpufeature.h | 5 ----- > xen/arch/ppc/include/asm/cpufeature.h | 10 ---------- > xen/arch/riscv/include/asm/cpufeature.h | 23 ----------------------- > xen/arch/x86/include/asm/processor.h | 5 ----- > xen/common/sched/credit2.c | 15 +++++++++++++-- > 5 files changed, 13 insertions(+), 45 deletions(-) > delete mode 100644 xen/arch/ppc/include/asm/cpufeature.h > delete mode 100644 xen/arch/riscv/include/asm/cpufeature.h > > diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h > index 969e043f5bda..50297e53d90e 100644 > --- a/xen/arch/arm/include/asm/cpufeature.h > +++ b/xen/arch/arm/include/asm/cpufeature.h > @@ -98,11 +98,6 @@ static inline bool cpus_have_cap(unsigned int num) > return test_bit(num, cpu_hwcaps); > } > > -static inline int cpu_nr_siblings(unsigned int cpu) > -{ > - return 1; > -} > - > /* System capability check for constant cap */ > #define cpus_have_const_cap(num) ({ \ > register_t __ret; \ > diff --git a/xen/arch/ppc/include/asm/cpufeature.h b/xen/arch/ppc/include/asm/cpufeature.h > deleted file mode 100644 > index 1c5946512bc5..000000000000 > --- a/xen/arch/ppc/include/asm/cpufeature.h > +++ /dev/null > @@ -1,10 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-only */ > -#ifndef __ASM_PPC_CPUFEATURE_H__ > -#define __ASM_PPC_CPUFEATURE_H__ > - > -static inline int cpu_nr_siblings(unsigned int cpu) > -{ > - return 1; > -} > - > -#endif /* __ASM_PPC_CPUFEATURE_H__ */ > diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h > deleted file mode 100644 > index 41a792b0b273..000000000000 > --- a/xen/arch/riscv/include/asm/cpufeature.h > +++ /dev/null > @@ -1,23 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-only */ > -#ifndef ASM__RISCV__CPUFEATURE_H > -#define ASM__RISCV__CPUFEATURE_H > - > -#ifndef __ASSEMBLY__ > - > -static inline unsigned int cpu_nr_siblings(unsigned int cpu) > -{ > - return 1; > -} > - > -#endif /* __ASSEMBLY__ */ > - > -#endif /* ASM__RISCV__CPUFEATURE_H */ > - > -/* > - * Local variables: > - * mode: C > - * c-file-style: "BSD" > - * c-basic-offset: 4 > - * indent-tabs-mode: nil > - * End: > - */ > diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h > index 877651212273..d247ef8dd226 100644 > --- a/xen/arch/x86/include/asm/processor.h > +++ b/xen/arch/x86/include/asm/processor.h > @@ -118,11 +118,6 @@ extern void init_intel_cacheinfo(struct cpuinfo_x86 *c); > > unsigned int apicid_to_socket(unsigned int apicid); > > -static inline int cpu_nr_siblings(unsigned int cpu) > -{ > - return cpu_data[cpu].x86_num_siblings; > -} > - > /* Some CPUID calls want 'count' to be placed in ecx */ > static inline void cpuid_count( > unsigned int op, > diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c > index 76a273d4f6d5..4b2ef034ca84 100644 > --- a/xen/common/sched/credit2.c > +++ b/xen/common/sched/credit2.c > @@ -26,8 +26,6 @@ > #include <xen/trace.h> > #include <xen/cpu.h> > #include <xen/keyhandler.h> > -#include <asm/cpufeature.h> > -#include <asm/processor.h> > > #include "private.h" > > @@ -35,6 +33,19 @@ > /* #define d2printk printk */ > #define d2printk(x...) > > +/* > + * TODO: Abstract this properly, and figure out what Credit2 wants to do with > + * the fact that x86_num_siblings doesn't even have the same meaning > + * between x86 vendors. > + */ > +static unsigned int cpu_nr_siblings(unsigned int cpu) > +{ > +#ifdef CONFIG_X86 > + return cpu_data[cpu].x86_num_siblings; > +#else > + return 1; > +#endif > +} > > /* > * Credit2 tracing events ("only" 512 available!). Check
Hi Andrew, On 12/18/24 6:49 AM, Andrew Cooper wrote: > Credit2 has no buisness including asm/cpufeature.h or asm/processor.h. > > This was caused by a bad original abstraction, and an even less wise attempt > to fix the build on my behalf. It is also the sole reason why PPC and RISC-V > need cpufeature.h header. > > Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same > meaning between vendors on x86 CPUS. > > Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO. Drop > the stub from each architecture. > > Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") > Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com> Thanks, Shawn
diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h index 969e043f5bda..50297e53d90e 100644 --- a/xen/arch/arm/include/asm/cpufeature.h +++ b/xen/arch/arm/include/asm/cpufeature.h @@ -98,11 +98,6 @@ static inline bool cpus_have_cap(unsigned int num) return test_bit(num, cpu_hwcaps); } -static inline int cpu_nr_siblings(unsigned int cpu) -{ - return 1; -} - /* System capability check for constant cap */ #define cpus_have_const_cap(num) ({ \ register_t __ret; \ diff --git a/xen/arch/ppc/include/asm/cpufeature.h b/xen/arch/ppc/include/asm/cpufeature.h deleted file mode 100644 index 1c5946512bc5..000000000000 --- a/xen/arch/ppc/include/asm/cpufeature.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef __ASM_PPC_CPUFEATURE_H__ -#define __ASM_PPC_CPUFEATURE_H__ - -static inline int cpu_nr_siblings(unsigned int cpu) -{ - return 1; -} - -#endif /* __ASM_PPC_CPUFEATURE_H__ */ diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h deleted file mode 100644 index 41a792b0b273..000000000000 --- a/xen/arch/riscv/include/asm/cpufeature.h +++ /dev/null @@ -1,23 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef ASM__RISCV__CPUFEATURE_H -#define ASM__RISCV__CPUFEATURE_H - -#ifndef __ASSEMBLY__ - -static inline unsigned int cpu_nr_siblings(unsigned int cpu) -{ - return 1; -} - -#endif /* __ASSEMBLY__ */ - -#endif /* ASM__RISCV__CPUFEATURE_H */ - -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h index 877651212273..d247ef8dd226 100644 --- a/xen/arch/x86/include/asm/processor.h +++ b/xen/arch/x86/include/asm/processor.h @@ -118,11 +118,6 @@ extern void init_intel_cacheinfo(struct cpuinfo_x86 *c); unsigned int apicid_to_socket(unsigned int apicid); -static inline int cpu_nr_siblings(unsigned int cpu) -{ - return cpu_data[cpu].x86_num_siblings; -} - /* Some CPUID calls want 'count' to be placed in ecx */ static inline void cpuid_count( unsigned int op, diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c index 76a273d4f6d5..4b2ef034ca84 100644 --- a/xen/common/sched/credit2.c +++ b/xen/common/sched/credit2.c @@ -26,8 +26,6 @@ #include <xen/trace.h> #include <xen/cpu.h> #include <xen/keyhandler.h> -#include <asm/cpufeature.h> -#include <asm/processor.h> #include "private.h" @@ -35,6 +33,19 @@ /* #define d2printk printk */ #define d2printk(x...) +/* + * TODO: Abstract this properly, and figure out what Credit2 wants to do with + * the fact that x86_num_siblings doesn't even have the same meaning + * between x86 vendors. + */ +static unsigned int cpu_nr_siblings(unsigned int cpu) +{ +#ifdef CONFIG_X86 + return cpu_data[cpu].x86_num_siblings; +#else + return 1; +#endif +} /* * Credit2 tracing events ("only" 512 available!). Check
Credit2 has no buisness including asm/cpufeature.h or asm/processor.h. This was caused by a bad original abstraction, and an even less wise attempt to fix the build on my behalf. It is also the sole reason why PPC and RISC-V need cpufeature.h header. Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same meaning between vendors on x86 CPUS. Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO. Drop the stub from each architecture. Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: Bertrand Marquis <bertrand.marquis@arm.com> CC: Michal Orzel <michal.orzel@amd.com> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> CC: Shawn Anastasio <sanastasio@raptorengineering.com> CC: Juergen Gross <jgross@suse.com> --- xen/arch/arm/include/asm/cpufeature.h | 5 ----- xen/arch/ppc/include/asm/cpufeature.h | 10 ---------- xen/arch/riscv/include/asm/cpufeature.h | 23 ----------------------- xen/arch/x86/include/asm/processor.h | 5 ----- xen/common/sched/credit2.c | 15 +++++++++++++-- 5 files changed, 13 insertions(+), 45 deletions(-) delete mode 100644 xen/arch/ppc/include/asm/cpufeature.h delete mode 100644 xen/arch/riscv/include/asm/cpufeature.h