Use cpu ops to implement this_cpu_has and use it in various places
Subsystems (3)
cover Use cpu ops to implement this_cpu_has and use it in various places
These patches were posted earlier in the context of this_cpu_ops use cases. Tejun send them to the x86 maintainers but they seem to have gotten lost.
PATCH 1/4 x86: A fast way to check capabilities of the current cpu
Add this_cpu_has() which determines if the current cpu has a certain ability using a segment prefix and a bit test operation.
For that we need to add bit operations to x86s percpu.h.
Many uses of cpu_has use a pointer passed to a function to determine the current flags. That is no longer necessary after this patch.
However, this patch only converts the straightforward cases where cpu_has is used with this_cpu_ptr. The rest is work for later.
--- arch/x86/include/asm/cpufeature.h | 13 +++++++++---- arch/x86/include/asm/percpu.h | 27 +++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c | 2 +- arch/x86/kernel/process.c | 4 ++-- arch/x86/kernel/smpboot.c | 4 ++-- 5 files changed, 41 insertions(+), 9 deletions(-)
arch/x86/include/asm/cpufeature.h +9 -4
@@ -206,8 +206,7 @@ extern const char * const x86_power_flag#define test_cpu_cap (c, bit) \ test_bit(bit, (unsigned long *)((c)->x86_capability)) -#define cpu_has (c, bit) \ - (__builtin_constant_p(bit) && \ +#define REQUIRED_MASK_BIT_SET (bit) \ ( (((bit)>>5)==0 && (1UL<<((bit)&31) & REQUIRED_MASK0)) || \ (((bit)>>5)==1 && (1UL<<((bit)&31) & REQUIRED_MASK1)) || \ (((bit)>>5)==2 && (1UL<<((bit)&31) & REQUIRED_MASK2)) || \
@@ -217,10 +216,16 @@ extern const char * const x86_power_flag (((bit)>>5)==6 && (1UL<<((bit)&31) & REQUIRED_MASK6)) || \ (((bit)>>5)==7 && (1UL<<((bit)&31) & REQUIRED_MASK7)) || \ (((bit)>>5)==8 && (1UL<<((bit)&31) & REQUIRED_MASK8)) || \ - (((bit)>>5)==9 && (1UL<<((bit)&31) & REQUIRED_MASK9)) ) \ - ? 1 : \ + (((bit)>>5)==9 && (1UL<<((bit)&31) & REQUIRED_MASK9)) ) + +#define cpu_has (c, bit) \ + (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \ test_cpu_cap(c, bit)) +#define this_cpu_has (bit) \ + (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \ + x86_this_cpu_test_bit(bit, (unsigned long *)&cpu_info.x86_capability)) +#define boot_cpu_has (bit) cpu_has(&boot_cpu_data, bit)#define set_cpu_cap (c, bit) set_bit(bit, (unsigned long *)((c)->x86_capability))
arch/x86/kernel/apic/apic.c +1 -1
@@ -516,7 +516,7 @@ static void __cpuinit setup_APIC_timer(v {struct clock_event_device *levt = &__get_cpu_var (lapic_events); -if (cpu_has (__this_cpu_ptr (&cpu_info),X86_FEATURE_ARAT )) { +if (this_cpu_has (X86_FEATURE_ARAT )) { lapic_clockevent.features &= ~CLOCK_EVT_FEAT_C3STOP ;/* Make LAPIC timer preferrable over percpu HPET */ lapic_clockevent.rating =150 ;
arch/x86/include/asm/percpu.h +27 -0
@@ -492,6 +492,33 @@ do { \ old__; \ }) +static __always_inline int x86_this_cpu_constant_test_bit(unsigned int nr, +const unsigned long __percpu *addr) +{ +unsigned long __percpu *a = (unsigned long *)addr + nr /BITS_PER_LONG ; + +return ((1UL << (nr %BITS_PER_LONG )) &percpu_read (*a)) !=0 ; +} + +static inline int x86_this_cpu_variable_test_bit (int nr, +const unsigned long __percpu *addr) +{ +int oldbit; + + asm volatile("bt "__percpu_arg(2)",%1\n\t" + "sbb %0,%0" + : "=r" (oldbit) + : "m" (*(unsigned long *)addr), "Ir" (nr)); + +return oldbit; +} + +#define x86_this_cpu_test_bit (nr, addr) \ + (__builtin_constant_p((nr)) \ + ? x86_this_cpu_constant_test_bit((nr), (addr)) \ + : x86_this_cpu_variable_test_bit((nr), (addr))) + +#include <asm-generic/percpu.h> /* We can use this directly for local CPU (faster). */
arch/x86/kernel/process.c +2 -2
@@ -452,7 +452,7 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);void mwait_idle_with_hints (unsigned long ax,unsigned long cx) {if (!need_resched ()) { -if (cpu_has (__this_cpu_ptr (&cpu_info),X86_FEATURE_CLFLUSH_MONITOR )) +if (this_cpu_has (X86_FEATURE_CLFLUSH_MONITOR ))clflush ((void *)¤t_thread_info ()->flags);__monitor ((void *)¤t_thread_info ()->flags,0 ,0 );
@@ -468,7 +468,7 @@ static void mwait_idle(void)if (!need_resched ()) {trace_power_start (POWER_CSTATE ,1 ,smp_processor_id ());trace_cpu_idle (1 ,smp_processor_id ()); -if (cpu_has (__this_cpu_ptr (&cpu_info),X86_FEATURE_CLFLUSH_MONITOR )) +if (this_cpu_has (X86_FEATURE_CLFLUSH_MONITOR ))clflush ((void *)¤t_thread_info ()->flags);__monitor ((void *)¤t_thread_info ()->flags,0 ,0 );
arch/x86/kernel/smpboot.c +2 -2
@@ -1404,9 +1404,9 @@ static inline void mwait_play_dead(void)void *mwait_ptr;struct cpuinfo_x86 *c =__this_cpu_ptr (&cpu_info); -if (!(cpu_has (c,X86_FEATURE_MWAIT ) &&mwait_usable (c))) +if (!this_cpu_has (X86_FEATURE_MWAIT ) &&mwait_usable (c))return ; -if (!cpu_has (__this_cpu_ptr (&cpu_info),X86_FEATURE_CLFLSH )) +if (!this_cpu_has (X86_FEATURE_CLFLSH ))return ;if (__this_cpu_read (cpu_info.cpuid_level) <CPUID_MWAIT_LEAF )return ;
PATCH 2/4 x86: Avoid passing struct cpuinfo pointer to mce_available
In reply to Christoph LameterIf we do not pass the pointer to cpuinfio to mce available then its possible to use this_cpu_has.
There are two use cases of mce_available: One with the current processor and one with the boot cpu. Define a function for both cases. However, there is only one case in which boot_mce_available is used. If we somehow can get rid of that then the patch could be simplified.
--- arch/x86/include/asm/mce.h | 3 +- arch/x86/kernel/cpu/mcheck/mce.c | 41 +++++++++++++++++++-------------- arch/x86/kernel/cpu/mcheck/mce_intel.c | 2 - 3 files changed, 27 insertions(+), 19 deletions(-)
arch/x86/include/asm/mce.h +2 -1
@@ -177,7 +177,8 @@ void mce_amd_feature_init(struct cpuinfostatic inline void mce_amd_feature_init (struct cpuinfo_x86 *c) { }#endif -int mce_available (struct cpuinfo_x86 *c); +int this_cpu_mce_available (void ); +int boot_mce_available (void );DECLARE_PER_CPU (unsigned, mce_exception_count);DECLARE_PER_CPU (unsigned, mce_poll_count);
arch/x86/kernel/cpu/mcheck/mce.c +24 -17
124 lines not on this page · raw message
arch/x86/kernel/cpu/mcheck/mce_intel.c +1 -1
@@ -130,7 +130,7 @@ void cmci_recheck(void)unsigned long flags;int banks; -if (!mce_available (__this_cpu_ptr (&cpu_info)) || !cmci_supported (&banks)) +if (!this_cpu_mce_available () || !cmci_supported (&banks))return ;local_irq_save (flags);machine_check_poll (MCP_TIMESTAMP , &__get_cpu_var (mce_banks_owned));
Tejun Heo
maintainerRe: [x86 this_cpu_has V1 2/4] x86: Avoid passing struct cpuinfo pointer to mce_available
Quoted 9 lines · Christoph Lameter
If we do not pass the pointer to cpuinfio to mce available then its possible to use
this_cpu_has.There are two use cases of
mce_available: One with the current processor and one with the boot cpu. Define a function for both cases. However, there is only one case in whichboot_mce_availableis used. If we somehow can get rid of that then the patch could be simplified.Signed-off-by Christoph Lameter <[email protected]>
Christoph, I'm not sure about this one. None of the conversions is in hot path, which would be okay if the resulting code was simpler but, well, it isn't necessarily worse but not distinctively better either, so I fail to see the merits of this conversion. I'll resend the other three towards x86 tree.
Thank you.
Signature · 1 line
tejun
PATCH 3/4 x86: Use this_cpu_has for thermal_interrupt
In reply to Christoph LameterIt is more effective to use a segment prefix instead of calculating the address of the current cpu area amd then testing flags.
--- arch/x86/kernel/cpu/mcheck/therm_throt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/mcheck/therm_throt.c +3 -4
@@ -355,7 +355,6 @@ static void notify_thresholds(__u64 msr_static void intel_thermal_interrupt (void ) {__u64 msr_val; -struct cpuinfo_x86 *c = &cpu_data (smp_processor_id ());rdmsrl (MSR_IA32_THERM_STATUS , msr_val);
@@ -367,19 +366,19 @@ static void intel_thermal_interrupt(void CORE_LEVEL) != 0)mce_log_therm_throt_event (CORE_THROTTLED | msr_val); -if (cpu_has (c,X86_FEATURE_PLN )) +if (this_cpu_has (X86_FEATURE_PLN ))if (therm_throt_process (msr_val &THERM_STATUS_POWER_LIMIT ,POWER_LIMIT_EVENT ,CORE_LEVEL ) !=0 )mce_log_therm_throt_event (CORE_POWER_LIMIT | msr_val); -if (cpu_has (c,X86_FEATURE_PTS )) { +if (this_cpu_has (X86_FEATURE_PTS )) {rdmsrl (MSR_IA32_PACKAGE_THERM_STATUS , msr_val);if (therm_throt_process (msr_val &PACKAGE_THERM_STATUS_PROCHOT ,THERMAL_THROTTLING_EVENT ,PACKAGE_LEVEL ) !=0 )mce_log_therm_throt_event (PACKAGE_THROTTLED | msr_val); -if (cpu_has (c,X86_FEATURE_PLN )) +if (this_cpu_has (X86_FEATURE_PLN )) if (therm_throt_process(msr_val & PACKAGE_THERM_STATUS_POWER_LIMIT, POWER_LIMIT_EVENT,
PATCH 4/4 acpi throttling: Use this_cpu_has and simplify code
In reply to Christoph LameterWith the this_cpu information we no longer need to pass an acpi structure to the msr management code. Simplifies code and improves performance.
NOTE: This code is x86 specific (see #ifdef CONFIG_X86) but not under arch/x86.
--- drivers/acpi/processor_throttling.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-)
drivers/acpi/processor_throttling.c +10 -22
@@ -710,20 +710,14 @@ static int acpi_processor_get_throttling }#ifdef CONFIG_X86 -static int acpi_throttling_rdmsr (struct acpi_processor *pr, -u64 *value) +static int acpi_throttling_rdmsr (u64 *value) { -struct cpuinfo_x86 *c;u64 msr_high, msr_low; -unsigned int cpu;u64 msr =0 ;int ret =-1 ; - cpu = pr->id; - c = &cpu_data (cpu); - -if ((c->x86_vendor !=X86_VENDOR_INTEL ) || - !cpu_has (c,X86_FEATURE_ACPI )) { +if ((this_cpu_read (cpu_info.x86_vendor) !=X86_VENDOR_INTEL ) || + !this_cpu_has (X86_FEATURE_ACPI )) { printk(KERN_ERR PREFIX "HARDWARE addr space,NOT supported yet\n"); }else {
@@ -738,18 +732,13 @@ static int acpi_throttling_rdmsr(structreturn ret; } -static int acpi_throttling_wrmsr (struct acpi_processor *pr,u64 value) +static int acpi_throttling_wrmsr (u64 value) { -struct cpuinfo_x86 *c; -unsigned int cpu;int ret =-1 ;u64 msr; - cpu = pr->id; - c = &cpu_data (cpu); - -if ((c->x86_vendor !=X86_VENDOR_INTEL ) || - !cpu_has (c,X86_FEATURE_ACPI )) { +if ((this_cpu_read (cpu_info.x86_vendor) !=X86_VENDOR_INTEL ) || + !this_cpu_has (X86_FEATURE_ACPI )) { printk(KERN_ERR PREFIX "HARDWARE addr space,NOT supported yet\n"); }else {
@@ -761,15 +750,14 @@ static int acpi_throttling_wrmsr(structreturn ret; }#else -static int acpi_throttling_rdmsr (struct acpi_processor *pr, -u64 *value) +static int acpi_throttling_rdmsr (u64 *value) { printk(KERN_ERR PREFIX "HARDWARE addr space,NOT supported yet\n");return -1 ; } -static int acpi_throttling_wrmsr (struct acpi_processor *pr,u64 value) +static int acpi_throttling_wrmsr (u64 value) { printk(KERN_ERR PREFIX "HARDWARE addr space,NOT supported yet\n");
@@ -801,7 +789,7 @@ static int acpi_read_throttling_status(s ret =0 ;break ;case ACPI_ADR_SPACE_FIXED_HARDWARE : - ret =acpi_throttling_rdmsr (pr, value); + ret =acpi_throttling_rdmsr (value);break ;default : printk(KERN_ERR PREFIX "Unknown addr space %d\n",
@@ -834,7 +822,7 @@ static int acpi_write_throttling_state(s ret =0 ;break ;case ACPI_ADR_SPACE_FIXED_HARDWARE : - ret =acpi_throttling_wrmsr (pr, value); + ret =acpi_throttling_wrmsr (value);break ;default : printk(KERN_ERR PREFIX "Unknown addr space %d\n",