2016-12-15 14:44:33

by Boris Ostrovsky

[permalink] [raw]
Subject: [PATCH 0/2] __cpuhp_setup_state() returns positive value for dynamic state

The first patch corrects description of __cpuhp_setup_state()'s possible
return values and the second fixes a bug (which causes Xen guests to crash).

As a follow-up question for the first patch --- should we allow calling
__cpuhp_setup_state() with state in [CPUHP_AP_ONLINE_DYN + 1 .. CPUHP_AP_ONLINE_DYN_END]
range?

Boris Ostrovsky (2):
cpu/hotplug: Clarify description of __cpuhp_setup_state() return
value
cpufreq: Remove cpu hotplug callbacks only if they were initialized

drivers/cpufreq/acpi-cpufreq.c | 2 +-
kernel/cpu.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)


2016-12-15 14:44:34

by Boris Ostrovsky

[permalink] [raw]
Subject: [PATCH 1/2] cpu/hotplug: Clarify description of __cpuhp_setup_state() return value

When ivoked with CPUHP_AP_ONLINE_DYN state __cpuhp_setup_state()
is expected to return positive value which is the hotplug state that
the routine assigns.

Signed-off-by: Boris Ostrovsky <[email protected]>
---

kernel/cpu.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 217fd2e..5339aca 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1586,7 +1586,11 @@ int __cpuhp_state_add_instance(enum cpuhp_state state, struct hlist_node *node,
* @startup: startup callback function
* @teardown: teardown callback function
*
- * Returns 0 if successful, otherwise a proper error code
+ * Returns:
+ * On success:
+ * Positive state number if @state is CPUHP_AP_ONLINE_DYN
+ * 0 for all other states
+ * On failure: proper (negative) error code
*/
int __cpuhp_setup_state(enum cpuhp_state state,
const char *name, bool invoke,
--
1.7.1

2016-12-15 14:44:32

by Boris Ostrovsky

[permalink] [raw]
Subject: [PATCH 2/2] cpufreq: Remove cpu hotplug callbacks only if they were initialized

Since cpu hotplug callbacks are requested for CPUHP_AP_ONLINE_DYN state,
successful callback initialization will result in cpuhp_setup_state()
returning a positive value. Therefore acpi_cpufreq_online being zero
indicates that callbacks have not been installed.

This means that acpi_cpufreq_boost_exit() should only remove them if
acpi_cpufreq_online is positive. Trying to call
cpuhp_remove_state_nocalls(0) will cause a BUG().

Signed-off-by: Boris Ostrovsky <[email protected]>
---
drivers/cpufreq/acpi-cpufreq.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 3a98702..3a2ca0f 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -930,7 +930,7 @@ static void __init acpi_cpufreq_boost_init(void)

static void acpi_cpufreq_boost_exit(void)
{
- if (acpi_cpufreq_online >= 0)
+ if (acpi_cpufreq_online > 0)
cpuhp_remove_state_nocalls(acpi_cpufreq_online);
}

--
1.7.1

2016-12-15 16:51:01

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 2/2] cpufreq: Remove cpu hotplug callbacks only if they were initialized

On Thu, 15 Dec 2016, Boris Ostrovsky wrote:

> Since cpu hotplug callbacks are requested for CPUHP_AP_ONLINE_DYN state,
> successful callback initialization will result in cpuhp_setup_state()
> returning a positive value. Therefore acpi_cpufreq_online being zero
> indicates that callbacks have not been installed.
>
> This means that acpi_cpufreq_boost_exit() should only remove them if
> acpi_cpufreq_online is positive. Trying to call
> cpuhp_remove_state_nocalls(0) will cause a BUG().
>
> Signed-off-by: Boris Ostrovsky <[email protected]>

Reviewed-by: Thomas Gleixner <[email protected]>

> ---
> drivers/cpufreq/acpi-cpufreq.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 3a98702..3a2ca0f 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -930,7 +930,7 @@ static void __init acpi_cpufreq_boost_init(void)
>
> static void acpi_cpufreq_boost_exit(void)
> {
> - if (acpi_cpufreq_online >= 0)
> + if (acpi_cpufreq_online > 0)
> cpuhp_remove_state_nocalls(acpi_cpufreq_online);
> }
>
> --
> 1.7.1
>
>

Subject: [tip:smp/urgent] cpu/hotplug: Clarify description of __cpuhp_setup_state() return value

Commit-ID: 512f09801b356c54baef62543e51169f03b2e642
Gitweb: http://git.kernel.org/tip/512f09801b356c54baef62543e51169f03b2e642
Author: Boris Ostrovsky <[email protected]>
AuthorDate: Thu, 15 Dec 2016 10:00:57 -0500
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 15 Dec 2016 17:48:20 +0100

cpu/hotplug: Clarify description of __cpuhp_setup_state() return value

When invoked with CPUHP_AP_ONLINE_DYN state __cpuhp_setup_state()
is expected to return positive value which is the hotplug state that
the routine assigns.

Signed-off-by: Boris Ostrovsky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
kernel/cpu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 217fd2e..5339aca 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1586,7 +1586,11 @@ EXPORT_SYMBOL_GPL(__cpuhp_state_add_instance);
* @startup: startup callback function
* @teardown: teardown callback function
*
- * Returns 0 if successful, otherwise a proper error code
+ * Returns:
+ * On success:
+ * Positive state number if @state is CPUHP_AP_ONLINE_DYN
+ * 0 for all other states
+ * On failure: proper (negative) error code
*/
int __cpuhp_setup_state(enum cpuhp_state state,
const char *name, bool invoke,