Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works() to rely on stopper->enabled

[Oleg Nesterov]

On 10/09, Peter Zijlstra wrote:
>
> I stuck that on top..

Yes, exactly! Plus I'll send another (minor) cleanup on top of
this if everything goes right.

> I'll have a closer look at the 7 patches later
> when I might be more coherent (mad head-ache atm.)

Thanks!

Hmm... but please please see another email I'll send in a minute
in reply to todays 3/3...

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works() to rely on stopper->enabled

[Peter Zijlstra]

On Thu, Oct 08, 2015 at 07:01:41PM +0200, Oleg Nesterov wrote:
> @@ -261,12 +276,8 @@ int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, 
> cpu_stop_fn_t fn, void *
>   set_state(, MULTI_STOP_PREPARE);
>  
>   /*
> -  * If we observe both CPUs active we know _cpu_down() cannot yet have
> -  * queued its stop_machine works and therefore ours will get executed
> -  * first. Or its not either one of our CPUs that's getting unplugged,
> -  * in which case we don't care.
> -  *
> -  * This relies on the stopper workqueues to be FIFO.
> +  * We do not want to migrate to inactive CPU. FIXME: move this
> +  * into migrate_swap_stop() callback.
>*/
>   if (!cpu_active(cpu1) || !cpu_active(cpu2)) {
>   preempt_enable();


I stuck that on top.. I'll have a closer look at the 7 patches later
when I might be more coherent (mad head-ache atm.)

---
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1335,12 +1335,16 @@ static int migrate_swap_stop(void *data)
struct rq *src_rq, *dst_rq;
int ret = -EAGAIN;
 
+   if (!cpu_active(arg->src_cpu) || !cpu_active(arg->dst_cpu))
+   return -EAGAIN;
+
src_rq = cpu_rq(arg->src_cpu);
dst_rq = cpu_rq(arg->dst_cpu);
 
double_raw_lock(>src_task->pi_lock,
>dst_task->pi_lock);
double_rq_lock(src_rq, dst_rq);
+
if (task_cpu(arg->dst_task) != arg->dst_cpu)
goto unlock;
 
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -275,15 +275,6 @@ int stop_two_cpus(unsigned int cpu1, uns
cpu_stop_init_done(, 2);
set_state(, MULTI_STOP_PREPARE);
 
-   /*
-* We do not want to migrate to inactive CPU. FIXME: move this
-* into migrate_swap_stop() callback.
-*/
-   if (!cpu_active(cpu1) || !cpu_active(cpu2)) {
-   preempt_enable();
-   return -ENOENT;
-   }
-
if (cpu1 > cpu2)
swap(cpu1, cpu2);
if (cpu_stop_queue_two_works(cpu1, , cpu2, )) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works() to rely on stopper->enabled

[Peter Zijlstra]

On Thu, Oct 08, 2015 at 07:01:41PM +0200, Oleg Nesterov wrote:
> @@ -261,12 +276,8 @@ int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, 
> cpu_stop_fn_t fn, void *
>   set_state(, MULTI_STOP_PREPARE);
>  
>   /*
> -  * If we observe both CPUs active we know _cpu_down() cannot yet have
> -  * queued its stop_machine works and therefore ours will get executed
> -  * first. Or its not either one of our CPUs that's getting unplugged,
> -  * in which case we don't care.
> -  *
> -  * This relies on the stopper workqueues to be FIFO.
> +  * We do not want to migrate to inactive CPU. FIXME: move this
> +  * into migrate_swap_stop() callback.
>*/
>   if (!cpu_active(cpu1) || !cpu_active(cpu2)) {
>   preempt_enable();


I stuck that on top.. I'll have a closer look at the 7 patches later
when I might be more coherent (mad head-ache atm.)

---
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1335,12 +1335,16 @@ static int migrate_swap_stop(void *data)
struct rq *src_rq, *dst_rq;
int ret = -EAGAIN;
 
+   if (!cpu_active(arg->src_cpu) || !cpu_active(arg->dst_cpu))
+   return -EAGAIN;
+
src_rq = cpu_rq(arg->src_cpu);
dst_rq = cpu_rq(arg->dst_cpu);
 
double_raw_lock(>src_task->pi_lock,
>dst_task->pi_lock);
double_rq_lock(src_rq, dst_rq);
+
if (task_cpu(arg->dst_task) != arg->dst_cpu)
goto unlock;
 
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -275,15 +275,6 @@ int stop_two_cpus(unsigned int cpu1, uns
cpu_stop_init_done(, 2);
set_state(, MULTI_STOP_PREPARE);
 
-   /*
-* We do not want to migrate to inactive CPU. FIXME: move this
-* into migrate_swap_stop() callback.
-*/
-   if (!cpu_active(cpu1) || !cpu_active(cpu2)) {
-   preempt_enable();
-   return -ENOENT;
-   }
-
if (cpu1 > cpu2)
swap(cpu1, cpu2);
if (cpu_stop_queue_two_works(cpu1, , cpu2, )) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/3] stop_machine: change cpu_stop_queue_two_works() to rely on stopper->enabled

[Oleg Nesterov]

On 10/09, Peter Zijlstra wrote:
>
> I stuck that on top..

Yes, exactly! Plus I'll send another (minor) cleanup on top of
this if everything goes right.

> I'll have a closer look at the 7 patches later
> when I might be more coherent (mad head-ache atm.)

Thanks!

Hmm... but please please see another email I'll send in a minute
in reply to todays 3/3...

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/