When notifiers were there, we were using `skip_onerr` to avoid
calling particular step startup/teardown callback in CPU up/down
rollback path, which made the hotplug a bit asymmetric.
As notifiers are gone now after state machine introduction. So,
`skip_onerr` field is no longer valid.
Remove it from the structure and its usage.
Signed-off-by: Mukesh Ojha <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
kernel/cpu.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index ed44d7d..c544baf 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -102,8 +102,6 @@ static inline void cpuhp_lock_release(bool bringup) { }
* @name: Name of the step
* @startup: Startup function of the step
* @teardown: Teardown function of the step
- * @skip_onerr: Do not invoke the functions on error rollback
- * Will go away once the notifiers are gone
* @cant_stop: Bringup/teardown can't be stopped at this step
*/
struct cpuhp_step {
@@ -119,7 +117,6 @@ struct cpuhp_step {
struct hlist_node *node);
} teardown;
struct hlist_head list;
- bool skip_onerr;
bool cant_stop;
bool multi_instance;
};
@@ -550,12 +547,8 @@ static int bringup_cpu(unsigned int cpu)
static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state--; st->state > st->target; st->state--) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
- }
+ for (st->state--; st->state > st->target; st->state--)
+ cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
}
static int cpuhp_up_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
@@ -644,12 +637,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
WARN_ON_ONCE(!cpuhp_is_ap_state(state));
- if (st->rollback) {
- struct cpuhp_step *step = cpuhp_get_step(state);
- if (step->skip_onerr)
- goto next;
- }
-
if (cpuhp_is_atomic_state(state)) {
local_irq_disable();
st->result = cpuhp_invoke_callback(cpu, state, bringup, st->node, &st->last);
@@ -916,12 +903,8 @@ void cpuhp_report_idle_dead(void)
static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state++; st->state < st->target; st->state++) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
- }
+ for (st->state++; st->state < st->target; st->state++)
+ cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
}
static int cpuhp_down_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On Tue, Aug 28, 2018 at 12:24:54PM +0530, Mukesh Ojha wrote:
> When notifiers were there, we were using `skip_onerr` to avoid
> calling particular step startup/teardown callback in CPU up/down
> rollback path, which made the hotplug a bit asymmetric.
>
> As notifiers are gone now after state machine introduction. So,
> `skip_onerr` field is no longer valid.
>
> Remove it from the structure and its usage.
There are indeed no users left.
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Commit-ID: 935aad3015fb4afc0b7ef6e6c18175b95461de47
Gitweb: https://git.kernel.org/tip/935aad3015fb4afc0b7ef6e6c18175b95461de47
Author: Mukesh Ojha <[email protected]>
AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 30 Aug 2018 12:59:30 +0200
cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
When notifiers were there, `skip_onerr` was used to avoid calling
particular step startup/teardown callbacks in the CPU up/down rollback
path, which made the hotplug asymmetric.
As notifiers are gone now after the full state machine conversion, the
`skip_onerr` field is no longer required.
Remove it from the structure and its usage.
Signed-off-by: Mukesh Ojha <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/cpu.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index ed44d7d34c2d..c544baf75e77 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -102,8 +102,6 @@ static inline void cpuhp_lock_release(bool bringup) { }
* @name: Name of the step
* @startup: Startup function of the step
* @teardown: Teardown function of the step
- * @skip_onerr: Do not invoke the functions on error rollback
- * Will go away once the notifiers are gone
* @cant_stop: Bringup/teardown can't be stopped at this step
*/
struct cpuhp_step {
@@ -119,7 +117,6 @@ struct cpuhp_step {
struct hlist_node *node);
} teardown;
struct hlist_head list;
- bool skip_onerr;
bool cant_stop;
bool multi_instance;
};
@@ -550,12 +547,8 @@ static int bringup_cpu(unsigned int cpu)
static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state--; st->state > st->target; st->state--) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
- }
+ for (st->state--; st->state > st->target; st->state--)
+ cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
}
static int cpuhp_up_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
@@ -644,12 +637,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
WARN_ON_ONCE(!cpuhp_is_ap_state(state));
- if (st->rollback) {
- struct cpuhp_step *step = cpuhp_get_step(state);
- if (step->skip_onerr)
- goto next;
- }
-
if (cpuhp_is_atomic_state(state)) {
local_irq_disable();
st->result = cpuhp_invoke_callback(cpu, state, bringup, st->node, &st->last);
@@ -916,12 +903,8 @@ void cpuhp_report_idle_dead(void)
static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state++; st->state < st->target; st->state++) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
- }
+ for (st->state++; st->state < st->target; st->state++)
+ cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
}
static int cpuhp_down_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
On Thu, Aug 30, 2018 at 04:03:58AM -0700, tip-bot for Mukesh Ojha wrote:
> Commit-ID: 935aad3015fb4afc0b7ef6e6c18175b95461de47
> Gitweb: https://git.kernel.org/tip/935aad3015fb4afc0b7ef6e6c18175b95461de47
> Author: Mukesh Ojha <[email protected]>
> AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 30 Aug 2018 12:59:30 +0200
> @@ -644,12 +637,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
>
> WARN_ON_ONCE(!cpuhp_is_ap_state(state));
>
> - if (st->rollback) {
> - struct cpuhp_step *step = cpuhp_get_step(state);
> - if (step->skip_onerr)
> - goto next;
> - }
> -
> if (cpuhp_is_atomic_state(state)) {
> local_irq_disable();
> st->result = cpuhp_invoke_callback(cpu, state, bringup, st->node, &st->last);
Can you fold this in?
../kernel/cpu.c: In function ‘cpuhp_thread_fun’:
../kernel/cpu.c:663:1: warning: label ‘next’ defined but not used [-Wunused-label]
---
kernel/cpu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index c544baf75e77..aa7fe85ad62e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -660,7 +660,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
st->should_run = false;
}
-next:
cpuhp_lock_release(bringup);
if (!st->should_run)
On Fri, 31 Aug 2018, Peter Zijlstra wrote:
> On Thu, Aug 30, 2018 at 04:03:58AM -0700, tip-bot for Mukesh Ojha wrote:
> > Commit-ID: 935aad3015fb4afc0b7ef6e6c18175b95461de47
> > Gitweb: https://git.kernel.org/tip/935aad3015fb4afc0b7ef6e6c18175b95461de47
> > Author: Mukesh Ojha <[email protected]>
> > AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
> > Committer: Thomas Gleixner <[email protected]>
> > CommitDate: Thu, 30 Aug 2018 12:59:30 +0200
>
> > @@ -644,12 +637,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
> >
> > WARN_ON_ONCE(!cpuhp_is_ap_state(state));
> >
> > - if (st->rollback) {
> > - struct cpuhp_step *step = cpuhp_get_step(state);
> > - if (step->skip_onerr)
> > - goto next;
> > - }
> > -
> > if (cpuhp_is_atomic_state(state)) {
> > local_irq_disable();
> > st->result = cpuhp_invoke_callback(cpu, state, bringup, st->node, &st->last);
>
>
> Can you fold this in?
>
> ../kernel/cpu.c: In function ‘cpuhp_thread_fun’:
> ../kernel/cpu.c:663:1: warning: label ‘next’ defined but not used [-Wunused-label]
Bah. I even saw the warning and wanted to fix it before pushing it
out. That's what you get for doing 20 things at once....
Folded in and force pushed. Thanks Peter!
Commit-ID: 6fb86d97207880c1286cd4cb3a7e6a598afbc727
Gitweb: https://git.kernel.org/tip/6fb86d97207880c1286cd4cb3a7e6a598afbc727
Author: Mukesh Ojha <[email protected]>
AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
Committer: Thomas Gleixner <[email protected]>
CommitDate: Fri, 31 Aug 2018 14:13:03 +0200
cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
When notifiers were there, `skip_onerr` was used to avoid calling
particular step startup/teardown callbacks in the CPU up/down rollback
path, which made the hotplug asymmetric.
As notifiers are gone now after the full state machine conversion, the
`skip_onerr` field is no longer required.
Remove it from the structure and its usage.
Signed-off-by: Mukesh Ojha <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/cpu.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index ed44d7d34c2d..aa7fe85ad62e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -102,8 +102,6 @@ static inline void cpuhp_lock_release(bool bringup) { }
* @name: Name of the step
* @startup: Startup function of the step
* @teardown: Teardown function of the step
- * @skip_onerr: Do not invoke the functions on error rollback
- * Will go away once the notifiers are gone
* @cant_stop: Bringup/teardown can't be stopped at this step
*/
struct cpuhp_step {
@@ -119,7 +117,6 @@ struct cpuhp_step {
struct hlist_node *node);
} teardown;
struct hlist_head list;
- bool skip_onerr;
bool cant_stop;
bool multi_instance;
};
@@ -550,12 +547,8 @@ static int bringup_cpu(unsigned int cpu)
static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state--; st->state > st->target; st->state--) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
- }
+ for (st->state--; st->state > st->target; st->state--)
+ cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
}
static int cpuhp_up_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,
@@ -644,12 +637,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
WARN_ON_ONCE(!cpuhp_is_ap_state(state));
- if (st->rollback) {
- struct cpuhp_step *step = cpuhp_get_step(state);
- if (step->skip_onerr)
- goto next;
- }
-
if (cpuhp_is_atomic_state(state)) {
local_irq_disable();
st->result = cpuhp_invoke_callback(cpu, state, bringup, st->node, &st->last);
@@ -673,7 +660,6 @@ static void cpuhp_thread_fun(unsigned int cpu)
st->should_run = false;
}
-next:
cpuhp_lock_release(bringup);
if (!st->should_run)
@@ -916,12 +902,8 @@ void cpuhp_report_idle_dead(void)
static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st)
{
- for (st->state++; st->state < st->target; st->state++) {
- struct cpuhp_step *step = cpuhp_get_step(st->state);
-
- if (!step->skip_onerr)
- cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
- }
+ for (st->state++; st->state < st->target; st->state++)
+ cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
}
static int cpuhp_down_callbacks(unsigned int cpu, struct cpuhp_cpu_state *st,