Re: [RFC][PATCH 02/13] stop_machine: Fix stop_cpus_in_progress ordering

[Steven Rostedt]

On Fri, 26 Jul 2019 16:54:11 +0200
Peter Zijlstra  wrote:

> Make sure the entire for loop has stop_cpus_in_progress set.


>  kernel/stop_machine.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -383,6 +383,7 @@ static bool queue_stop_cpus_work(const s
>*/
>   preempt_disable();
>   stop_cpus_in_progress = true;
> + barrier();

Like smp_mb() shouldn't barrier() have a comment to what is being
ordered and why?

-- Steve

>   for_each_cpu(cpu, cpumask) {
>   work = _cpu(cpu_stopper.stop_work, cpu);
>   work->fn = fn;
> @@ -391,6 +392,7 @@ static bool queue_stop_cpus_work(const s
>   if (cpu_stop_queue_work(cpu, work))
>   queued = true;
>   }
> + barrier();
>   stop_cpus_in_progress = false;
>   preempt_enable();
>  
> 

Re: [RFC][PATCH 02/13] stop_machine: Fix stop_cpus_in_progress ordering

[Phil Auld]

On Fri, Jul 26, 2019 at 04:54:11PM +0200 Peter Zijlstra wrote:
> Make sure the entire for loop has stop_cpus_in_progress set.
> 
> Cc: Valentin Schneider 
> Cc: Aaron Lu 
> Cc: keesc...@chromium.org
> Cc: mi...@kernel.org
> Cc: Pawan Gupta 
> Cc: Phil Auld 
> Cc: torva...@linux-foundation.org
> Cc: Tim Chen 
> Cc: fweis...@gmail.com
> Cc: subhra.mazum...@oracle.com
> Cc: t...@linutronix.de
> Cc: Julien Desfossez 
> Cc: p...@google.com
> Cc: Nishanth Aravamudan 
> Cc: Aubrey Li 
> Cc: Mel Gorman 
> Cc: kerr...@google.com
> Cc: Paolo Bonzini 
> Signed-off-by: Peter Zijlstra (Intel) 
> Link: 
> https://lkml.kernel.org/r/0fd8fd4b99b9b9aa88d8b2dff897f7fd0d88f72c.1559129225.git.vpil...@digitalocean.com
> ---
>  kernel/stop_machine.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -383,6 +383,7 @@ static bool queue_stop_cpus_work(const s
>*/
>   preempt_disable();
>   stop_cpus_in_progress = true;
> + barrier();
>   for_each_cpu(cpu, cpumask) {
>   work = _cpu(cpu_stopper.stop_work, cpu);
>   work->fn = fn;
> @@ -391,6 +392,7 @@ static bool queue_stop_cpus_work(const s
>   if (cpu_stop_queue_work(cpu, work))
>   queued = true;
>   }
> + barrier();
>   stop_cpus_in_progress = false;
>   preempt_enable();
>  
> 
> 

This looks good.

Reviewed-by: Phil Auld 


-- 

[RFC][PATCH 02/13] stop_machine: Fix stop_cpus_in_progress ordering

[Peter Zijlstra]

Make sure the entire for loop has stop_cpus_in_progress set.

Cc: Valentin Schneider 
Cc: Aaron Lu 
Cc: keesc...@chromium.org
Cc: mi...@kernel.org
Cc: Pawan Gupta 
Cc: Phil Auld 
Cc: torva...@linux-foundation.org
Cc: Tim Chen 
Cc: fweis...@gmail.com
Cc: subhra.mazum...@oracle.com
Cc: t...@linutronix.de
Cc: Julien Desfossez 
Cc: p...@google.com
Cc: Nishanth Aravamudan 
Cc: Aubrey Li 
Cc: Mel Gorman 
Cc: kerr...@google.com
Cc: Paolo Bonzini 
Signed-off-by: Peter Zijlstra (Intel) 
Link: 
https://lkml.kernel.org/r/0fd8fd4b99b9b9aa88d8b2dff897f7fd0d88f72c.1559129225.git.vpil...@digitalocean.com
---
 kernel/stop_machine.c |2 ++
 1 file changed, 2 insertions(+)

--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -383,6 +383,7 @@ static bool queue_stop_cpus_work(const s
 */
preempt_disable();
stop_cpus_in_progress = true;
+   barrier();
for_each_cpu(cpu, cpumask) {
work = _cpu(cpu_stopper.stop_work, cpu);
work->fn = fn;
@@ -391,6 +392,7 @@ static bool queue_stop_cpus_work(const s
if (cpu_stop_queue_work(cpu, work))
queued = true;
}
+   barrier();
stop_cpus_in_progress = false;
preempt_enable();