All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
@ 2018-08-28  6:54 Mukesh Ojha
  2018-08-28  7:55 ` Peter Zijlstra
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Mukesh Ojha @ 2018-08-28  6:54 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: Mukesh Ojha, Peter Zijlstra, Ingo Molnar

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 <mojha@codeaurora.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>

---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
  2018-08-28  6:54 [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure Mukesh Ojha
@ 2018-08-28  7:55 ` Peter Zijlstra
  2018-08-30 11:03 ` [tip:smp/urgent] " tip-bot for Mukesh Ojha
  2018-08-31 12:15 ` tip-bot for Mukesh Ojha
  2 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2018-08-28  7:55 UTC (permalink / raw)
  To: Mukesh Ojha; +Cc: tglx, linux-kernel, Ingo Molnar

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) <peterz@infradead.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:smp/urgent] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
  2018-08-28  6:54 [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure Mukesh Ojha
  2018-08-28  7:55 ` Peter Zijlstra
@ 2018-08-30 11:03 ` tip-bot for Mukesh Ojha
  2018-08-31  8:33   ` Peter Zijlstra
  2018-08-31 12:15 ` tip-bot for Mukesh Ojha
  2 siblings, 1 reply; 6+ messages in thread
From: tip-bot for Mukesh Ojha @ 2018-08-30 11:03 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: mingo, peterz, hpa, linux-kernel, tglx, mojha

Commit-ID:  935aad3015fb4afc0b7ef6e6c18175b95461de47
Gitweb:     https://git.kernel.org/tip/935aad3015fb4afc0b7ef6e6c18175b95461de47
Author:     Mukesh Ojha <mojha@codeaurora.org>
AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <mojha@codeaurora.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/1535439294-31426-1-git-send-email-mojha@codeaurora.org


---
 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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tip:smp/urgent] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
  2018-08-30 11:03 ` [tip:smp/urgent] " tip-bot for Mukesh Ojha
@ 2018-08-31  8:33   ` Peter Zijlstra
  2018-08-31 12:14     ` Thomas Gleixner
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2018-08-31  8:33 UTC (permalink / raw)
  To: mojha, linux-kernel, tglx, hpa, mingo; +Cc: linux-tip-commits

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 <mojha@codeaurora.org>
> AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> 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)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tip:smp/urgent] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
  2018-08-31  8:33   ` Peter Zijlstra
@ 2018-08-31 12:14     ` Thomas Gleixner
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2018-08-31 12:14 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: mojha, linux-kernel, hpa, mingo, linux-tip-commits

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

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 <mojha@codeaurora.org>
> > AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
> > Committer:  Thomas Gleixner <tglx@linutronix.de>
> > 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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:smp/urgent] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure
  2018-08-28  6:54 [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure Mukesh Ojha
  2018-08-28  7:55 ` Peter Zijlstra
  2018-08-30 11:03 ` [tip:smp/urgent] " tip-bot for Mukesh Ojha
@ 2018-08-31 12:15 ` tip-bot for Mukesh Ojha
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Mukesh Ojha @ 2018-08-31 12:15 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: mingo, linux-kernel, peterz, tglx, hpa, mojha

Commit-ID:  6fb86d97207880c1286cd4cb3a7e6a598afbc727
Gitweb:     https://git.kernel.org/tip/6fb86d97207880c1286cd4cb3a7e6a598afbc727
Author:     Mukesh Ojha <mojha@codeaurora.org>
AuthorDate: Tue, 28 Aug 2018 12:24:54 +0530
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <mojha@codeaurora.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/1535439294-31426-1-git-send-email-mojha@codeaurora.org
---
 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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-31 12:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28  6:54 [PATCH] cpu/hotplug: Remove skip_onerr field from cpuhp_step structure Mukesh Ojha
2018-08-28  7:55 ` Peter Zijlstra
2018-08-30 11:03 ` [tip:smp/urgent] " tip-bot for Mukesh Ojha
2018-08-31  8:33   ` Peter Zijlstra
2018-08-31 12:14     ` Thomas Gleixner
2018-08-31 12:15 ` tip-bot for Mukesh Ojha

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.