From: Peter Zijlstra <[email protected]>
Make sure the entire for loop has stop_cpus_in_progress set.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
kernel/stop_machine.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 067cb83f37ea..583119e0c51c 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -375,6 +375,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
*/
preempt_disable();
stop_cpus_in_progress = true;
+ barrier();
for_each_cpu(cpu, cpumask) {
work = &per_cpu(cpu_stopper.stop_work, cpu);
work->fn = fn;
@@ -383,6 +384,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
if (cpu_stop_queue_work(cpu, work))
queued = true;
}
+ barrier();
stop_cpus_in_progress = false;
preempt_enable();
--
2.17.1
Commit-ID: 99d84bf8c65a7a0dbc9e166ca0a58ed949ac4f37
Gitweb: https://git.kernel.org/tip/99d84bf8c65a7a0dbc9e166ca0a58ed949ac4f37
Author: Peter Zijlstra <[email protected]>
AuthorDate: Wed, 29 May 2019 20:36:37 +0000
Committer: Peter Zijlstra <[email protected]>
CommitDate: Thu, 8 Aug 2019 09:09:30 +0200
stop_machine: Fix stop_cpus_in_progress ordering
Make sure the entire for loop has stop_cpus_in_progress set.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Aaron Lu <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: [email protected]
Cc: Phil Auld <[email protected]>
Cc: Julien Desfossez <[email protected]>
Cc: Nishanth Aravamudan <[email protected]>
Link: https://lkml.kernel.org/r/0fd8fd4b99b9b9aa88d8b2dff897f7fd0d88f72c.1559129225.git.vpillai@digitalocean.com
---
kernel/stop_machine.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index b4f83f7bdf86..c7031a22aa7b 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -383,6 +383,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
*/
preempt_disable();
stop_cpus_in_progress = true;
+ barrier();
for_each_cpu(cpu, cpumask) {
work = &per_cpu(cpu_stopper.stop_work, cpu);
work->fn = fn;
@@ -391,6 +392,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
if (cpu_stop_queue_work(cpu, work))
queued = true;
}
+ barrier();
stop_cpus_in_progress = false;
preempt_enable();
On Mon, Aug 26, 2019 at 09:19:31AM -0700, mark gross wrote:
> On Wed, May 29, 2019 at 08:36:37PM +0000, Vineeth Remanan Pillai wrote:
> > From: Peter Zijlstra <[email protected]>
> >
> > Make sure the entire for loop has stop_cpus_in_progress set.
> It is not clear how this commit comment matches the change. Please explain
> how adding 2 barrier's makes sure stop_cpus_in_progress is set for the entier
> for loop.
Without the barrier the compiler is free to move the stores around. It
probably doesn't do anything bad, but this makes sure it cannot.
On Wed, May 29, 2019 at 08:36:37PM +0000, Vineeth Remanan Pillai wrote:
> From: Peter Zijlstra <[email protected]>
>
> Make sure the entire for loop has stop_cpus_in_progress set.
It is not clear how this commit comment matches the change. Please explain
how adding 2 barrier's makes sure stop_cpus_in_progress is set for the entier
for loop.
--mark
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> kernel/stop_machine.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index 067cb83f37ea..583119e0c51c 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -375,6 +375,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
> */
> preempt_disable();
> stop_cpus_in_progress = true;
> + barrier();
> for_each_cpu(cpu, cpumask) {
> work = &per_cpu(cpu_stopper.stop_work, cpu);
> work->fn = fn;
> @@ -383,6 +384,7 @@ static bool queue_stop_cpus_work(const struct cpumask *cpumask,
> if (cpu_stop_queue_work(cpu, work))
> queued = true;
> }
> + barrier();
> stop_cpus_in_progress = false;
> preempt_enable();
>
> --
> 2.17.1
>