Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1440560 > unrolled thread

[patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine

Started byAnna-Maria Gleixner <anna-maria@linutronix.de>
First post2016-07-11 14:40 +0200
Last post2016-07-11 16:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine Anna-Maria Gleixner <anna-maria@linutronix.de> - 2016-07-11 14:40 +0200
    Re: [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state  machine Anna-Maria Gleixner <anna-maria@linutronix.de> - 2016-07-11 16:20 +0200
    Re: [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine Paolo Bonzini <pbonzini@redhat.com> - 2016-07-11 16:50 +0200

#1440560 — [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine

FromAnna-Maria Gleixner <anna-maria@linutronix.de>
Date2016-07-11 14:40 +0200
Subject[patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine
Message-ID<rTIFd-70u-67@gated-at.bofh.it>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Install the callbacks via the state machine and let the core invoke
the callbacks on the already online CPUs.

We assumed that the priority ordering was ment to invoke the online
callback as the last step. In the original code this also invoked the
down prepare callback as the last step. With the symmetric state
machine the down prepare callback is now the first step.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
---
 arch/x86/kvm/x86.c         |   35 ++++++++---------------------------
 include/linux/cpuhotplug.h |    1 +
 2 files changed, 9 insertions(+), 27 deletions(-)

--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5552,9 +5552,10 @@ int kvm_fast_pio_out(struct kvm_vcpu *vc
 }
 EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
 
-static void tsc_bad(void *info)
+static int kvmclock_cpu_down_prep(unsigned int cpu)
 {
 	__this_cpu_write(cpu_tsc_khz, 0);
+	return 0;
 }
 
 static void tsc_khz_changed(void *data)
@@ -5659,35 +5660,18 @@ static struct notifier_block kvmclock_cp
 	.notifier_call  = kvmclock_cpufreq_notifier
 };
 
-static int kvmclock_cpu_notifier(struct notifier_block *nfb,
-					unsigned long action, void *hcpu)
+static int kvmclock_cpu_online(unsigned int cpu)
 {
-	unsigned int cpu = (unsigned long)hcpu;
-
-	switch (action) {
-		case CPU_ONLINE:
-		case CPU_DOWN_FAILED:
-			tsc_khz_changed(NULL);
-			break;
-		case CPU_DOWN_PREPARE:
-			tsc_bad(NULL);
-			break;
-	}
-	return NOTIFY_OK;
+	tsc_khz_changed(NULL);
+	return 0;
 }
 
-static struct notifier_block kvmclock_cpu_notifier_block = {
-	.notifier_call  = kvmclock_cpu_notifier,
-	.priority = -INT_MAX
-};
-
 static void kvm_timer_init(void)
 {
 	int cpu;
 
 	max_tsc_khz = tsc_khz;
 
-	cpu_notifier_register_begin();
 	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
 #ifdef CONFIG_CPU_FREQ
 		struct cpufreq_policy policy;
@@ -5702,12 +5686,9 @@ static void kvm_timer_init(void)
 					  CPUFREQ_TRANSITION_NOTIFIER);
 	}
 	pr_debug("kvm: max_tsc_khz = %ld\n", max_tsc_khz);
-	for_each_online_cpu(cpu)
-		smp_call_function_single(cpu, tsc_khz_changed, NULL, 1);
-
-	__register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
-	cpu_notifier_register_done();
 
+	cpuhp_setup_state(CPUHP_AP_X86_KVM_CLK_ONLINE, "AP_X86_KVM_CLK_ONLINE",
+			  kvmclock_cpu_online, kvmclock_cpu_down_prep);
 }
 
 static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
@@ -5896,7 +5877,7 @@ void kvm_arch_exit(void)
 	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
 		cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
 					    CPUFREQ_TRANSITION_NOTIFIER);
-	unregister_hotcpu_notifier(&kvmclock_cpu_notifier_block);
+	cpuhp_remove_state_nocalls(CPUHP_X86_KVM_CLK_ONLINE);
 #ifdef CONFIG_X86_64
 	pvclock_gtod_unregister_notifier(&pvclock_gtod_notifier);
 #endif
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -56,6 +56,7 @@ enum cpuhp_state {
 	CPUHP_AP_ONLINE_DYN,
 	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
 	CPUHP_AP_X86_HPET_ONLINE,
+	CPUHP_AP_X86_KVM_CLK_ONLINE,
 	CPUHP_AP_ACTIVE,
 	CPUHP_ONLINE,
 };

[toc] | [next] | [standalone]


#1440643 — Re: [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine

FromAnna-Maria Gleixner <anna-maria@linutronix.de>
Date2016-07-11 16:20 +0200
SubjectRe: [patch 31/66] x86/kvm/kvmclock: Convert to hotplug state machine
Message-ID<rTKdY-87y-27@gated-at.bofh.it>
In reply to#1440560
On Mon, 11 Jul 2016, Anna-Maria Gleixner wrote:

> @@ -5896,7 +5877,7 @@ void kvm_arch_exit(void)
>  	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
>  		cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
>  					    CPUFREQ_TRANSITION_NOTIFIER);
> -	unregister_hotcpu_notifier(&kvmclock_cpu_notifier_block);
> +	cpuhp_remove_state_nocalls(CPUHP_X86_KVM_CLK_ONLINE);

I'm sorry, this is buggy and wants to be:

    cpuhp_remove_state_nocalls(CPUHP_AP_X86_KVM_CLK_ONLINE);

See delta patch below.

    Anna-Maria

8<-----------------

--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5877,7 +5877,7 @@ void kvm_arch_exit(void)
 	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
 		cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
 					    CPUFREQ_TRANSITION_NOTIFIER);
-	cpuhp_remove_state_nocalls(CPUHP_X86_KVM_CLK_ONLINE);
+	cpuhp_remove_state_nocalls(CPUHP_AP_X86_KVM_CLK_ONLINE);
 #ifdef CONFIG_X86_64
 	pvclock_gtod_unregister_notifier(&pvclock_gtod_notifier);
 #endif

[toc] | [prev] | [next] | [standalone]


#1440681

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-07-11 16:50 +0200
Message-ID<rTKH1-8jQ-39@gated-at.bofh.it>
In reply to#1440560
On 11/07/2016 14:28, Anna-Maria Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Install the callbacks via the state machine and let the core invoke
> the callbacks on the already online CPUs.
> 
> We assumed that the priority ordering was ment to invoke the online
> callback as the last step. In the original code this also invoked the
> down prepare callback as the last step. With the symmetric state
> machine the down prepare callback is now the first step.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> ---
>  arch/x86/kvm/x86.c         |   35 ++++++++---------------------------
>  include/linux/cpuhotplug.h |    1 +
>  2 files changed, 9 insertions(+), 27 deletions(-)
> 
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5552,9 +5552,10 @@ int kvm_fast_pio_out(struct kvm_vcpu *vc
>  }
>  EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
>  
> -static void tsc_bad(void *info)
> +static int kvmclock_cpu_down_prep(unsigned int cpu)
>  {
>  	__this_cpu_write(cpu_tsc_khz, 0);
> +	return 0;
>  }
>  
>  static void tsc_khz_changed(void *data)
> @@ -5659,35 +5660,18 @@ static struct notifier_block kvmclock_cp
>  	.notifier_call  = kvmclock_cpufreq_notifier
>  };
>  
> -static int kvmclock_cpu_notifier(struct notifier_block *nfb,
> -					unsigned long action, void *hcpu)
> +static int kvmclock_cpu_online(unsigned int cpu)
>  {
> -	unsigned int cpu = (unsigned long)hcpu;
> -
> -	switch (action) {
> -		case CPU_ONLINE:
> -		case CPU_DOWN_FAILED:
> -			tsc_khz_changed(NULL);
> -			break;
> -		case CPU_DOWN_PREPARE:
> -			tsc_bad(NULL);
> -			break;
> -	}
> -	return NOTIFY_OK;
> +	tsc_khz_changed(NULL);
> +	return 0;
>  }
>  
> -static struct notifier_block kvmclock_cpu_notifier_block = {
> -	.notifier_call  = kvmclock_cpu_notifier,
> -	.priority = -INT_MAX
> -};
> -
>  static void kvm_timer_init(void)
>  {
>  	int cpu;
>  
>  	max_tsc_khz = tsc_khz;
>  
> -	cpu_notifier_register_begin();
>  	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
>  #ifdef CONFIG_CPU_FREQ
>  		struct cpufreq_policy policy;
> @@ -5702,12 +5686,9 @@ static void kvm_timer_init(void)
>  					  CPUFREQ_TRANSITION_NOTIFIER);
>  	}
>  	pr_debug("kvm: max_tsc_khz = %ld\n", max_tsc_khz);
> -	for_each_online_cpu(cpu)
> -		smp_call_function_single(cpu, tsc_khz_changed, NULL, 1);
> -
> -	__register_hotcpu_notifier(&kvmclock_cpu_notifier_block);
> -	cpu_notifier_register_done();
>  
> +	cpuhp_setup_state(CPUHP_AP_X86_KVM_CLK_ONLINE, "AP_X86_KVM_CLK_ONLINE",
> +			  kvmclock_cpu_online, kvmclock_cpu_down_prep);
>  }
>  
>  static DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
> @@ -5896,7 +5877,7 @@ void kvm_arch_exit(void)
>  	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
>  		cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
>  					    CPUFREQ_TRANSITION_NOTIFIER);
> -	unregister_hotcpu_notifier(&kvmclock_cpu_notifier_block);
> +	cpuhp_remove_state_nocalls(CPUHP_X86_KVM_CLK_ONLINE);
>  #ifdef CONFIG_X86_64
>  	pvclock_gtod_unregister_notifier(&pvclock_gtod_notifier);
>  #endif
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -56,6 +56,7 @@ enum cpuhp_state {
>  	CPUHP_AP_ONLINE_DYN,
>  	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
>  	CPUHP_AP_X86_HPET_ONLINE,
> +	CPUHP_AP_X86_KVM_CLK_ONLINE,
>  	CPUHP_AP_ACTIVE,
>  	CPUHP_ONLINE,
>  };
> 
> 
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

(I don't have problems with patches sent as attachments, but others might).

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web