[PATCH v6 0/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable for CPU offlining prevention

[PATCH v6 0/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable for CPU offlining prevention

[Vitaly Kuznetsov]

Changes since v5:
- Split PATCH 1 into two (PATCH 1/3 and 3/3), rewrite changelogs. [Thomas
  Gleixner]

Changes since v4:
- In disable_nonboot_cpus() do cpu_hotplug_disabled++ unconditionally as
  its users are doing enable_nonboot_cpus() on their failure paths.

Changes since v3:
- add WARN_ON when decreasing cpu_hotplug_disabled [Peter Zijlstra]

Changes since v2:
- Rebase on top of current Greg's char-misc-next tree [K. Y. Srinivasan]

Changes since v1:
- Make cpu_hotplug_disabled a counter [Radim Krčmář]

Export cpu_hotplug_enable/cpu_hotplug_disable functions from cpu.c and use
them instead of altering smp_ops.cpu_disable in Hyper-V vmbus module.

Vitaly Kuznetsov (3):
  cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable
  Drivers: hv: vmbus: use cpu_hotplug_enable/disable
  cpu-hotplug: convert cpu_hotplug_disabled to a counter

 Documentation/power/suspend-and-cpuhotplug.txt |  6 ++--
 drivers/hv/vmbus_drv.c                         | 38 +++-----------------------
 kernel/cpu.c                                   | 24 ++++++++++------
 3 files changed, 22 insertions(+), 46 deletions(-)

-- 
2.4.3

[PATCH v6 1/3] cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable

[Vitaly Kuznetsov]

Hyper-V module needs to disable cpu hotplug (offlining) as there is no
support from hypervisor side to reassing already opened event channels
to a different CPU. Currently it is been done by altering
smp_ops.cpu_disable but it is hackish.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 kernel/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 94bbe46..dc005e7 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -193,6 +193,7 @@ void cpu_hotplug_disable(void)
 	cpu_hotplug_disabled = 1;
 	cpu_maps_update_done();
 }
+EXPORT_SYMBOL_GPL(cpu_hotplug_disable);
 
 void cpu_hotplug_enable(void)
 {
@@ -200,7 +201,7 @@ void cpu_hotplug_enable(void)
 	cpu_hotplug_disabled = 0;
 	cpu_maps_update_done();
 }
-
+EXPORT_SYMBOL_GPL(cpu_hotplug_enable);
 #endif	/* CONFIG_HOTPLUG_CPU */
 
 /* Need to know about CPUs going up/down? */
-- 
2.4.3

Re: [PATCH v6 1/3] cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable

[Thomas Gleixner]

On Fri, 26 Jun 2015, Vitaly Kuznetsov wrote:

> Hyper-V module needs to disable cpu hotplug (offlining) as there is no
> support from hypervisor side to reassing already opened event channels

s/reassing/reassign/

> to a different CPU. Currently it is been done by altering
> smp_ops.cpu_disable but it is hackish.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Other than that:

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[PATCH v6 2/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable

[Vitaly Kuznetsov]

Commit e513229b4c38 ("Drivers: hv: vmbus: prevent cpu offlining on newer
hypervisors") was altering smp_ops.cpu_disable to prevent CPU offlining.
We can bo better by using cpu_hotplug_enable/disable functions instead of
such hard-coding.

Reported-by: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/vmbus_drv.c | 38 ++++----------------------------------
 1 file changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index cf20400..6de65fb 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -763,38 +763,6 @@ static void vmbus_isr(void)
 	}
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
-static int hyperv_cpu_disable(void)
-{
-	return -ENOSYS;
-}
-
-static void hv_cpu_hotplug_quirk(bool vmbus_loaded)
-{
-	static void *previous_cpu_disable;
-
-	/*
-	 * Offlining a CPU when running on newer hypervisors (WS2012R2, Win8,
-	 * ...) is not supported at this moment as channel interrupts are
-	 * distributed across all of them.
-	 */
-
-	if ((vmbus_proto_version == VERSION_WS2008) ||
-	    (vmbus_proto_version == VERSION_WIN7))
-		return;
-
-	if (vmbus_loaded) {
-		previous_cpu_disable = smp_ops.cpu_disable;
-		smp_ops.cpu_disable = hyperv_cpu_disable;
-		pr_notice("CPU offlining is not supported by hypervisor\n");
-	} else if (previous_cpu_disable)
-		smp_ops.cpu_disable = previous_cpu_disable;
-}
-#else
-static void hv_cpu_hotplug_quirk(bool vmbus_loaded)
-{
-}
-#endif
 
 /*
  * vmbus_bus_init -Main vmbus driver initialization routine.
@@ -836,7 +804,8 @@ static int vmbus_bus_init(int irq)
 	if (ret)
 		goto err_alloc;
 
-	hv_cpu_hotplug_quirk(true);
+	if (vmbus_proto_version > VERSION_WIN7)
+		cpu_hotplug_disable();
 
 	/*
 	 * Only register if the crash MSRs are available
@@ -1121,7 +1090,8 @@ static void __exit vmbus_exit(void)
 		smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
 	}
 	acpi_bus_unregister_driver(&vmbus_acpi_driver);
-	hv_cpu_hotplug_quirk(false);
+	if (vmbus_proto_version > VERSION_WIN7)
+		cpu_hotplug_enable();
 }
 
 
-- 
2.4.3

[PATCH v6 3/3] cpu-hotplug: convert cpu_hotplug_disabled to a counter

[Vitaly Kuznetsov]

As cpu_hotplug_enable/cpu_hotplug_disable functions are now available to
modules we need to convert cpu_hotplug_disabled to a counter to properly
support disable -> disable -> enable call sequences. E.g. after Hyper-V
vmbus module did cpu_hotplug_disable() hibernate path calls
disable_nonboot_cpus() and if we hit an error in _cpu_down()
enable_nonboot_cpus() will be called on the failure path (thus making
cpu_hotplug_disabled = 0 and leaving cpu hotplug in 'enabled' state).
Same problem is possible if more than 1 module use cpu_hotplug_disable/
cpu_hotplug_enable on their load/unload paths. When one of these modules
is been unloaded it is logical to leave cpu hotplug in 'disabled' state.

To support the change we need to increse cpu_hotplug_disabled counter
in disable_nonboot_cpus() unconditionally as all users of
disable_nonboot_cpus() are supposed to do enable_nonboot_cpus() in case
an error was returned.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 Documentation/power/suspend-and-cpuhotplug.txt |  6 +++---
 kernel/cpu.c                                   | 21 +++++++++++++--------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/Documentation/power/suspend-and-cpuhotplug.txt b/Documentation/power/suspend-and-cpuhotplug.txt
index 2850df3..2fc9095 100644
--- a/Documentation/power/suspend-and-cpuhotplug.txt
+++ b/Documentation/power/suspend-and-cpuhotplug.txt
@@ -72,7 +72,7 @@ More details follow:
                                         |
                                         v
                            Disable regular cpu hotplug
-                        by setting cpu_hotplug_disabled=1
+                        by increasing cpu_hotplug_disabled
                                         |
                                         v
                             Release cpu_add_remove_lock
@@ -89,7 +89,7 @@ Resuming back is likewise, with the counterparts being (in the order of
 execution during resume):
 * enable_nonboot_cpus() which involves:
    |  Acquire cpu_add_remove_lock
-   |  Reset cpu_hotplug_disabled to 0, thereby enabling regular cpu hotplug
+   |  Decrease cpu_hotplug_disabled, thereby enabling regular cpu hotplug
    |  Call _cpu_up() [for all those cpus in the frozen_cpus mask, in a loop]
    |  Release cpu_add_remove_lock
    v
@@ -120,7 +120,7 @@ after the entire cycle is complete (i.e., suspend + resume).
                            Acquire cpu_add_remove_lock
                                         |
                                         v
-                          If cpu_hotplug_disabled is 1
+                          If cpu_hotplug_disabled > 0
                                 return gracefully
                                         |
                                         |
diff --git a/kernel/cpu.c b/kernel/cpu.c
index dc005e7..cab3c92 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -190,7 +190,7 @@ void cpu_hotplug_done(void)
 void cpu_hotplug_disable(void)
 {
 	cpu_maps_update_begin();
-	cpu_hotplug_disabled = 1;
+	cpu_hotplug_disabled++;
 	cpu_maps_update_done();
 }
 EXPORT_SYMBOL_GPL(cpu_hotplug_disable);
@@ -198,7 +198,7 @@ EXPORT_SYMBOL_GPL(cpu_hotplug_disable);
 void cpu_hotplug_enable(void)
 {
 	cpu_maps_update_begin();
-	cpu_hotplug_disabled = 0;
+	WARN_ON(--cpu_hotplug_disabled < 0);
 	cpu_maps_update_done();
 }
 EXPORT_SYMBOL_GPL(cpu_hotplug_enable);
@@ -598,13 +598,18 @@ int disable_nonboot_cpus(void)
 		}
 	}
 
-	if (!error) {
+	if (!error)
 		BUG_ON(num_online_cpus() > 1);
-		/* Make sure the CPUs won't be enabled by someone else */
-		cpu_hotplug_disabled = 1;
-	} else {
+	else
 		pr_err("Non-boot CPUs are not disabled\n");
-	}
+
+	/*
+	 * Make sure the CPUs won't be enabled by someone else. We need to do
+	 * this even in case of failure as all disable_nonboot_cpus() users are
+	 * supposed to do enable_nonboot_cpus() on the failure path.
+	 */
+	cpu_hotplug_disabled++;
+
 	cpu_maps_update_done();
 	return error;
 }
@@ -623,7 +628,7 @@ void __ref enable_nonboot_cpus(void)
 
 	/* Allow everyone to use the CPU hotplug again */
 	cpu_maps_update_begin();
-	cpu_hotplug_disabled = 0;
+	WARN_ON(--cpu_hotplug_disabled < 0);
 	if (cpumask_empty(frozen_cpus))
 		goto out;
 
-- 
2.4.3

Re: [PATCH v6 3/3] cpu-hotplug: convert cpu_hotplug_disabled to a counter

[Thomas Gleixner]

On Fri, 26 Jun 2015, Vitaly Kuznetsov wrote:

> As cpu_hotplug_enable/cpu_hotplug_disable functions are now available to
> modules we need to convert cpu_hotplug_disabled to a counter to properly

Actually this patch should be the first in the series. We don't expose
known to be broken infrastructure to fix it after the fact.

Other than that:

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH v6 0/3] Drivers: hv: vmbus: use cpu_hotplug_enable/disable for CPU offlining prevention

[Vitaly Kuznetsov]

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Changes since v5:
> - Split PATCH 1 into two (PATCH 1/3 and 3/3), rewrite changelogs. [Thomas
>   Gleixner]

Sorry for the ping but Greg won't probably take this into his tree
without some feedback on patches 1 and 3 from core kernel people. Could
you please take a look?

Thanks,

>
> Changes since v4:
> - In disable_nonboot_cpus() do cpu_hotplug_disabled++ unconditionally as
>   its users are doing enable_nonboot_cpus() on their failure paths.
>
> Changes since v3:
> - add WARN_ON when decreasing cpu_hotplug_disabled [Peter Zijlstra]
>
> Changes since v2:
> - Rebase on top of current Greg's char-misc-next tree [K. Y. Srinivasan]
>
> Changes since v1:
> - Make cpu_hotplug_disabled a counter [Radim Krčmář]
>
> Export cpu_hotplug_enable/cpu_hotplug_disable functions from cpu.c and use
> them instead of altering smp_ops.cpu_disable in Hyper-V vmbus module.
>
> Vitaly Kuznetsov (3):
>   cpu-hotplug: export cpu_hotplug_enable/cpu_hotplug_disable
>   Drivers: hv: vmbus: use cpu_hotplug_enable/disable
>   cpu-hotplug: convert cpu_hotplug_disabled to a counter
>
>  Documentation/power/suspend-and-cpuhotplug.txt |  6 ++--
>  drivers/hv/vmbus_drv.c                         | 38 +++-----------------------
>  kernel/cpu.c                                   | 24 ++++++++++------
>  3 files changed, 22 insertions(+), 46 deletions(-)

-- 
  Vitaly