Skip to contentSkip to conversation

stop_machine: Make migration_cpu_stop() does useful works for CONFIG_PREEMPT_NONE

PATCH In review Cheng Chao 5 participants 25 messages Sep 9, 2016 – Sep 22, 2016 1 message not in the archive Scheduler

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): OF73BCE1E4.D2224FEC-ON48258025.0022CFA7-48258025.0022E3DC@kedacom.com

Cheng Chao

author
#1 lore raw

PATCH v2 1/1 stop_machine: Make migration_cpu_stop() does useful works for CONFIG_PREEMPT_NONE

For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

But in fact, all above works are almost useless(wasteful),the reason is migration_cpu_stop() can not migrate. why? migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

Signed-off-by Cheng Chao <[email protected]>
---
 kernel/stop_machine.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/stop_machine.c +11 -0
index 4a1ca5f..41aea5e 100644
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
Signature · 1 line
2.4.11
author
#2 lore raw

--in-reply-to doesn't work?

v1 is :
https://lkml.org/lkml/2016/9/7/819

Quoted 42 lines · Cheng Chao

For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

But in fact, all above works are almost useless(wasteful),the reason is migration_cpu_stop() can not migrate. why? migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

Signed-off-by Cheng Chao <[email protected]>
---
 kernel/stop_machine.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/stop_machine.c +11 -0
index 4a1ca5f..41aea5e 100644
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
#3 lore raw
In reply to Cheng Chao
Quoted 4 lines · Cheng Chao

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

and this finally migrates the target, ttwu() does another select_task_rq().

Quoted 2 lines

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

yes, this avoids the extra wakeup/select_task_rq. So this is the minor optimization.

Quoted 13 lines
Signed-off-by Cheng Chao <[email protected]>
---
 kernel/stop_machine.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/stop_machine.c +0 -0
index 4a1ca5f..41aea5e 100644
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 138

Quoted 10 lines
kernel/stop_machine.c +10 -0
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();

Well. This can help in general, wait_for_completion() won't block, but only if we queue the work on the same CPU. Otherwise this schedule() adds the unnecessary pessimization.

That is why I suggested to use _cond_resched() instead of schedule().

But let me repeat, I leave this to maintainers.

Oleg.

maintainer
#4 lore raw
Quoted 6 lines · Oleg Nesterov
Quoted 4 lines · Cheng Chao

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

Still not seeing any mail from Cheng Chao. I think he's mailing from a domain that's not accepting email and then infradead makes its go-away, why accept email from people who will not accept bounces.

Cheng, please fix your email setup.

#5 lore raw

PATCH v3 1/1 stop_machine: Make migration_cpu_stop() does useful works for CONFIG_PREEMPT_NONE

In reply to Cheng Chao

For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

But in fact, all above works are almost useless(wasteful),the reason is migration_cpu_stop() can not migrate. why? migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

Signed-off-by Cheng Chao <[email protected]>
---
 kernel/stop_machine.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/stop_machine.c +11 -0
index 4a1ca5f..41aea5e 100644
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
Signature · 1 line
2.4.11
#6 lore raw

hi Peter, I guess you can receive the mail from me now, I have changed the mailbox to gmail.

Oleg has already done much work for this patch, I am really obliged. please review this patch, thanks.

Quoted 42 lines · Cheng Chao

For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

But in fact, all above works are almost useless(wasteful),the reason is migration_cpu_stop() can not migrate. why? migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

Signed-off-by Cheng Chao <[email protected]>
---
 kernel/stop_machine.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/stop_machine.c +11 -0
index 4a1ca5f..41aea5e 100644
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
maintainer
#7 lore raw
Quoted 2 lines · Cheng Chao

hi Peter, I guess you can receive the mail from me now, I have changed the mailbox to gmail.

Indeed, I'll have a look on Monday.

#8 lore raw
In reply to Cheng Chao
Quoted 9 lines · Cheng Chao
kernel/stop_machine.c +4 -0
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 142

Quoted 10 lines
kernel/stop_machine.c +7 -0
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }

Cheng, I already tried twice to suggest to conditionalize this schedule, because it can only help if cpu == smp_processor_id, and you didn't reply. I still think _cond_resched() makes more sense.

I won't really argue if you prefer it this way. But did you see my emails?

Oleg.

#9 lore raw
Quoted 25 lines · Oleg Nesterov
Quoted 9 lines · Cheng Chao
kernel/stop_machine.c +4 -0
@@ -126,6 +126,17 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+	/*
+	 * Makes the stopper thread run as soon as possible.

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 142

Quoted 10 lines
kernel/stop_machine.c +7 -0
+	 * And if the caller is TASK_RUNNING, keeps the caller TASK_RUNNING.
+	 * It's special useful for some callers which are expected to be
+	 * TASK_ON_RQ_QUEUED.
+	 * sched_exec does benefit from this improvement.
+	 */
+	schedule();
+#endif
 	wait_for_completion(&done.completion);
 	return done.ret;
 }

Cheng, I already tried twice to suggest to conditionalize this schedule, because it can only help if cpu == smp_processor_id, and you didn't reply. I still think _cond_resched() makes more sense.

Quoted 2 lines

I won't really argue if you prefer it this way. But did you see my emails?

I read them, thanks. because Peter didn't receive my mails before, it took me much time to fix my mailbox, so I didn't reply on time.

Ok, even if cpu != smp_processor_id(), to call schedule() instead _cond_resched() can give the caller a chance not to sleep. when the caller runs on the cpu again, it may likely find the completion is already done.
then the stopper thread cpu_stop_signal_done() and the caller wait_for_completion() will actually run very soon.

I think it is trivial improvement. using cond_resched()/_cond_resched() is better for readability, I choose the cond_resched().

thanks again.

Quoted 2 lines

Oleg.

maintainer
#10 lore raw
In reply to Cheng Chao
Quoted 7 lines · Cheng Chao

For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).

If the migration_cpu_stop() can not migrate,why do we call stop_one_cpu()? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes migration_cpu_stop(), and the stopper thread wakes up the task.

Quoted 6 lines

But in fact, all above works are almost useless(wasteful),the reason is migration_cpu_stop() can not migrate. why? migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().

This patch keeps the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can do useful works.

OK, completely confused. What!?

/me ponders....

So what you're saying is that migration_stop_cpu() doesn't work because wait_for_completion() dequeues the task.

True I suppose. Not sure I like your solution, nor your implementation of the solution much though.

I would much prefer an unconditional cond_resched() there, but also, I think we should do what __migrate_swap_task() does, and set wake_cpu.

So something like so..

---
 kernel/sched/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
kernel/sched/core.c +6 -2
index ddd5f48551f1..ade772aa9610 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);
maintainer
#11 lore raw
Quoted 24 lines · Peter Zijlstra

So what you're saying is that migration_stop_cpu() doesn't work because wait_for_completion() dequeues the task.

True I suppose. Not sure I like your solution, nor your implementation of the solution much though.

I would much prefer an unconditional cond_resched() there, but also, I think we should do what __migrate_swap_task() does, and set wake_cpu.

So something like so..

---
 kernel/sched/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
kernel/sched/core.c +0 -1
index ddd5f48551f1..ade772aa9610 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))

kernel/sched/core.c static int migration_cpu_stop(void *data) · line 1074

Quoted 10 lines
kernel/sched/core.c +6 -1
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);

And this, too narrow a constraint do git diff made it go away.

---
 kernel/stop_machine.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/stop_machine.c +5 -0
index ae6f41fb9cba..637798d6b554 100644
@@ -121,6 +121,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
#12 lore raw

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 128

Quoted 12 lines · Peter Zijlstra
kernel/stop_machine.c +5 -0
@@ -121,6 +121,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();

Yes, this is what I tried to suggest too.

But this leads to the question which I wanted to ask many times.

Why cond_resched() is not NOP if CONFIG_PREEMPT=y ?

Perhaps we have some users like, just for example,

	preempt_enable_no_resched();
	cond_resched();

which actually want the should_resched() check even if CONFIG_PREEMPT, but most callers do not?

Oleg.

maintainer
#13 lore raw
Quoted 3 lines · Oleg Nesterov

But this leads to the question which I wanted to ask many times.

Why cond_resched() is not NOP if CONFIG_PREEMPT=y ?

Dunno, nobody bothered to do it? We should keep the might_sleep() of course, but the preemption check is pointless.

Quoted 7 lines

Perhaps we have some users like, just for example,

	preempt_enable_no_resched();
	cond_resched();

which actually want the should_resched() check even if CONFIG_PREEMPT, but most callers do not?

I would hope not, the few preempt_enable_no_resched() users _should_ have an actual schedule() call in the _very_ near future.

#14 lore raw
Quoted 9 lines · Peter Zijlstra
Quoted 3 lines · Oleg Nesterov

But this leads to the question which I wanted to ask many times.

Why cond_resched() is not NOP if CONFIG_PREEMPT=y ?

Dunno, nobody bothered to do it? We should keep the might_sleep() of course, but the preemption check is pointless.

Yes, agreed, I actually meant _cond_resched().

Quoted 10 lines
Quoted 7 lines

Perhaps we have some users like, just for example,

	preempt_enable_no_resched();
	cond_resched();

which actually want the should_resched() check even if CONFIG_PREEMPT, but most callers do not?

I would hope not, the few preempt_enable_no_resched() users _should_ have an actual schedule() call in the _very_ near future.

Me too, and I failed to find something which could be broken... So perhaps should make it nop and investigate the new bug reports after that.

Hmm. And preempt_enable_no_resched_notrace() under TASK_DEAD in __schedule() should be removed it seems, do_exit() can call __schedule() directly.

Oleg.

maintainer
#15 lore raw
Quoted 3 lines · Oleg Nesterov

Me too, and I failed to find something which could be broken... So perhaps should make it nop and investigate the new bug reports after that.

Works for me :-)

Quoted 4 lines

Hmm. And preempt_enable_no_resched_notrace() under TASK_DEAD in __schedule() should be removed it seems, do_exit() can call __schedule() directly.

something like so?

---

 include/linux/kernel.h |  2 +-
 include/linux/sched.h  |  2 ++
 kernel/exit.c          | 11 ++---------
 kernel/sched/core.c    | 23 ++++++++++++-----------
 4 files changed, 17 insertions(+), 21 deletions(-)
include/linux/kernel.h +1 -1
index d96a6118d26a..e5bd9cdd2e24 100644
@@ -266,7 +266,7 @@ extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
 extern int oops_may_print(void);
-void do_exit(long error_code)
+void __noreturn do_exit(long error_code)
 	__noreturn;
 void complete_and_exit(struct completion *, long)
 	__noreturn;
include/linux/sched.h +2 -0
index eb64fcd89e68..b0c818a05b2e 100644
@@ -448,6 +448,8 @@ static inline void io_schedule(void)
 	io_schedule_timeout(MAX_SCHEDULE_TIMEOUT);
 }
 
+void __noreturn do_task_dead(void);
+
 struct nsproxy;
 struct user_namespace;
kernel/exit.c +2 -9
index 091a78be3b09..d4c12692f766 100644
@@ -725,7 +725,7 @@ static void check_stack_usage(void)
 static inline void check_stack_usage(void) {}
 #endif
 
-void do_exit(long code)
+void __noreturn do_exit(long code)
 {
 	struct task_struct *tsk = current;
 	int group_dead;

@@ -897,14 +897,7 @@ void do_exit(long code)
 	smp_mb();
 	raw_spin_unlock_wait(&tsk->pi_lock);
 
-	/* causes final put_task_struct in finish_task_switch(). */
-	tsk->state = TASK_DEAD;
-	tsk->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
-	schedule();
-	BUG();
-	/* Avoid "noreturn function does return".  */
-	for (;;)
-		cpu_relax();	/* For when BUG is null */
+	do_task_dead();
 }
 EXPORT_SYMBOL_GPL(do_exit);
kernel/sched/core.c +12 -11
index a0086a5fc008..6034f269000f 100644
@@ -3327,17 +3327,6 @@ static void __sched notrace __schedule(bool preempt)
 	rq = cpu_rq(cpu);
 	prev = rq->curr;
 
-	/*
-	 * do_exit() calls schedule() with preemption disabled as an exception;
-	 * however we must fix that up, otherwise the next task will see an
-	 * inconsistent (higher) preempt count.
-	 *
-	 * It also avoids the below schedule_debug() test from complaining
-	 * about this.
-	 */
-	if (unlikely(prev->state == TASK_DEAD))
-		preempt_enable_no_resched_notrace();
-
 	schedule_debug(prev);
 
 	if (sched_feat(HRTICK))

@@ -3404,6 +3393,18 @@ static void __sched notrace __schedule(bool preempt)
 	balance_callback(rq);
 }
 
+void __noreturn do_task_dead(void)
+{
+	/* causes final put_task_struct in finish_task_switch(). */
+	__set_current_state(TASK_DEAD);
+	current->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
+	__schedule(false);
+	BUG();
+	/* Avoid "noreturn function does return".  */
+	for (;;)
+		cpu_relax();	/* For when BUG is null */
+}
+
 static inline void sched_submit_work(struct task_struct *tsk)
 {
 	if (!tsk->state || tsk_is_pi_blocked(tsk))
#16 lore raw

great, __schedule() doesn't need pay any attention to the TASK_DEAD now.

Quoted 121 lines · Peter Zijlstra
Quoted 3 lines · Oleg Nesterov

Me too, and I failed to find something which could be broken... So perhaps should make it nop and investigate the new bug reports after that.

Works for me :-)

Quoted 4 lines

Hmm. And preempt_enable_no_resched_notrace() under TASK_DEAD in __schedule() should be removed it seems, do_exit() can call __schedule() directly.

something like so?

---

 include/linux/kernel.h |  2 +-
 include/linux/sched.h  |  2 ++
 kernel/exit.c          | 11 ++---------
 kernel/sched/core.c    | 23 ++++++++++++-----------
 4 files changed, 17 insertions(+), 21 deletions(-)
include/linux/kernel.h +1 -1
index d96a6118d26a..e5bd9cdd2e24 100644
@@ -266,7 +266,7 @@ extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
 extern int oops_may_print(void);
-void do_exit(long error_code)
+void __noreturn do_exit(long error_code)
 	__noreturn;
 void complete_and_exit(struct completion *, long)
 	__noreturn;
include/linux/sched.h +2 -0
index eb64fcd89e68..b0c818a05b2e 100644
@@ -448,6 +448,8 @@ static inline void io_schedule(void)
 	io_schedule_timeout(MAX_SCHEDULE_TIMEOUT);
 }
 
+void __noreturn do_task_dead(void);
+
 struct nsproxy;
 struct user_namespace;
kernel/exit.c +2 -9
index 091a78be3b09..d4c12692f766 100644
@@ -725,7 +725,7 @@ static void check_stack_usage(void)
 static inline void check_stack_usage(void) {}
 #endif
 
-void do_exit(long code)
+void __noreturn do_exit(long code)
 {
 	struct task_struct *tsk = current;
 	int group_dead;

@@ -897,14 +897,7 @@ void do_exit(long code)
 	smp_mb();
 	raw_spin_unlock_wait(&tsk->pi_lock);
 
-	/* causes final put_task_struct in finish_task_switch(). */
-	tsk->state = TASK_DEAD;
-	tsk->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
-	schedule();
-	BUG();
-	/* Avoid "noreturn function does return".  */
-	for (;;)
-		cpu_relax();	/* For when BUG is null */
+	do_task_dead();
 }
 EXPORT_SYMBOL_GPL(do_exit);
kernel/sched/core.c +12 -11
index a0086a5fc008..6034f269000f 100644
@@ -3327,17 +3327,6 @@ static void __sched notrace __schedule(bool preempt)
 	rq = cpu_rq(cpu);
 	prev = rq->curr;
 
-	/*
-	 * do_exit() calls schedule() with preemption disabled as an exception;
-	 * however we must fix that up, otherwise the next task will see an
-	 * inconsistent (higher) preempt count.
-	 *
-	 * It also avoids the below schedule_debug() test from complaining
-	 * about this.
-	 */
-	if (unlikely(prev->state == TASK_DEAD))
-		preempt_enable_no_resched_notrace();
-
 	schedule_debug(prev);
 
 	if (sched_feat(HRTICK))

@@ -3404,6 +3393,18 @@ static void __sched notrace __schedule(bool preempt)
 	balance_callback(rq);
 }
 
+void __noreturn do_task_dead(void)
+{
+	/* causes final put_task_struct in finish_task_switch(). */
+	__set_current_state(TASK_DEAD);
+	current->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
+	__schedule(false);
+	BUG();
+	/* Avoid "noreturn function does return".  */
+	for (;;)
+		cpu_relax();	/* For when BUG is null */
+}
+
 static inline void sched_submit_work(struct task_struct *tsk)
 {
 	if (!tsk->state || tsk_is_pi_blocked(tsk))
maintainer
#17 lore raw
Quoted 2 lines · Cheng Chao

great, __schedule() doesn't need pay any attention to the TASK_DEAD now.

There's still __schedule()->context_switch()->finish_task_switch().

#18 lore raw
In reply to Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 3 lines · Oleg Nesterov

Hmm. And preempt_enable_no_resched_notrace() under TASK_DEAD in __schedule() should be removed it seems, do_exit() can call __schedule() directly.

something like so?

Yes, exactly.

But I think that raw_spin_unlock_wait(&tsk->pi_lock) and its huge comment should be moved into the new helper too, this connects to set_current_state(TASK_DEAD).

Oleg.

merge bot
#19 lore raw

[tip:sched/core] sched/core: Optimize __schedule()

In reply to Peter Zijlstra
sched/core: Optimize __schedule()
Commit-ID:  9af6528ee9b682df7f29dbee86fbba0b67eab944
Gitweb:     http://git.kernel.org/tip/9af6528ee9b682df7f29dbee86fbba0b67eab944
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Tue, 13 Sep 2016 18:37:29 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 22 Sep 2016 14:53:45 +0200

sched/core: Optimize __schedule()

Oleg noted that by making do_exit() use __schedule() for the TASK_DEAD context switch, we can avoid the TASK_DEAD special case currently in __schedule() because that avoids the extra preempt_disable() from schedule().

In order to facilitate this, create a do_task_dead() helper which we place in the scheduler code, such that it can access __schedule().

Also add some __noreturn annotations to the functions, there's no coming back from do_exit().

Suggested-by Oleg Nesterov <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Cheng Chao <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 include/linux/kernel.h |  9 +++------
 include/linux/sched.h  |  2 ++
 kernel/exit.c          | 26 ++------------------------
 kernel/sched/core.c    | 38 +++++++++++++++++++++++++++-----------
 4 files changed, 34 insertions(+), 41 deletions(-)
include/linux/kernel.h +3 -6
index d96a611..74fd6f0 100644
@@ -259,17 +259,14 @@ static inline void might_fault(void) { }
 extern struct atomic_notifier_head panic_notifier_list;
 extern long (*panic_blink)(int state);
 __printf(1, 2)
-void panic(const char *fmt, ...)
-	__noreturn __cold;
+void panic(const char *fmt, ...) __noreturn __cold;
 void nmi_panic(struct pt_regs *regs, const char *msg);
 extern void oops_enter(void);
 extern void oops_exit(void);
 void print_oops_end_marker(void);
 extern int oops_may_print(void);
-void do_exit(long error_code)
-	__noreturn;
-void complete_and_exit(struct completion *, long)
-	__noreturn;
+void do_exit(long error_code) __noreturn;
+void complete_and_exit(struct completion *, long) __noreturn;
 
 /* Internal, do not use. */
 int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
include/linux/sched.h +2 -0
index d750240..f00ee8e 100644
@@ -448,6 +448,8 @@ static inline void io_schedule(void)
 	io_schedule_timeout(MAX_SCHEDULE_TIMEOUT);
 }
 
+void __noreturn do_task_dead(void);
+
 struct nsproxy;
 struct user_namespace;
kernel/exit.c +2 -24
index 091a78b..1e1d913 100644
@@ -725,7 +725,7 @@ static void check_stack_usage(void)
 static inline void check_stack_usage(void) {}
 #endif
 
-void do_exit(long code)
+void __noreturn do_exit(long code)
 {
 	struct task_struct *tsk = current;
 	int group_dead;

@@ -882,29 +882,7 @@ void do_exit(long code)
 	exit_rcu();
 	TASKS_RCU(__srcu_read_unlock(&tasks_rcu_exit_srcu, tasks_rcu_i));
 
-	/*
-	 * The setting of TASK_RUNNING by try_to_wake_up() may be delayed
-	 * when the following two conditions become true.
-	 *   - There is race condition of mmap_sem (It is acquired by
-	 *     exit_mm()), and
-	 *   - SMI occurs before setting TASK_RUNINNG.
-	 *     (or hypervisor of virtual machine switches to other guest)
-	 *  As a result, we may become TASK_RUNNING after becoming TASK_DEAD
-	 *
-	 * To avoid it, we have to wait for releasing tsk->pi_lock which
-	 * is held by try_to_wake_up()
-	 */
-	smp_mb();
-	raw_spin_unlock_wait(&tsk->pi_lock);
-
-	/* causes final put_task_struct in finish_task_switch(). */
-	tsk->state = TASK_DEAD;
-	tsk->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
-	schedule();
-	BUG();
-	/* Avoid "noreturn function does return".  */
-	for (;;)
-		cpu_relax();	/* For when BUG is null */
+	do_task_dead();
 }
 EXPORT_SYMBOL_GPL(do_exit);
kernel/sched/core.c +27 -11
index ff4e3c0..b2ec53c 100644
@@ -3331,17 +3331,6 @@ static void __sched notrace __schedule(bool preempt)
 	rq = cpu_rq(cpu);
 	prev = rq->curr;
 
-	/*
-	 * do_exit() calls schedule() with preemption disabled as an exception;
-	 * however we must fix that up, otherwise the next task will see an
-	 * inconsistent (higher) preempt count.
-	 *
-	 * It also avoids the below schedule_debug() test from complaining
-	 * about this.
-	 */
-	if (unlikely(prev->state == TASK_DEAD))
-		preempt_enable_no_resched_notrace();
-
 	schedule_debug(prev);
 
 	if (sched_feat(HRTICK))

@@ -3409,6 +3398,33 @@ static void __sched notrace __schedule(bool preempt)
 }
 STACK_FRAME_NON_STANDARD(__schedule); /* switch_to() */
 
+void __noreturn do_task_dead(void)
+{
+	/*
+	 * The setting of TASK_RUNNING by try_to_wake_up() may be delayed
+	 * when the following two conditions become true.
+	 *   - There is race condition of mmap_sem (It is acquired by
+	 *     exit_mm()), and
+	 *   - SMI occurs before setting TASK_RUNINNG.
+	 *     (or hypervisor of virtual machine switches to other guest)
+	 *  As a result, we may become TASK_RUNNING after becoming TASK_DEAD
+	 *
+	 * To avoid it, we have to wait for releasing tsk->pi_lock which
+	 * is held by try_to_wake_up()
+	 */
+	smp_mb();
+	raw_spin_unlock_wait(&current->pi_lock);
+
+	/* causes final put_task_struct in finish_task_switch(). */
+	__set_current_state(TASK_DEAD);
+	current->flags |= PF_NOFREEZE;	/* tell freezer to ignore us */
+	__schedule(false);
+	BUG();
+	/* Avoid "noreturn function does return".  */
+	for (;;)
+		cpu_relax();	/* For when BUG is null */
+}
+
 static inline void sched_submit_work(struct task_struct *tsk)
 {
 	if (!tsk->state || tsk_is_pi_blocked(tsk))
#20 lore raw
In reply to Peter Zijlstra

Peter, thank you.

Quoted 29 lines · Peter Zijlstra
Quoted 28 lines · Peter Zijlstra

So what you're saying is that migration_stop_cpu() doesn't work because wait_for_completion() dequeues the task.

True I suppose. Not sure I like your solution, nor your implementation of the solution much though.

I would much prefer an unconditional cond_resched() there, but also, I think we should do what __migrate_swap_task() does, and set wake_cpu.

So something like so..

---
 kernel/sched/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
kernel/sched/core.c +3 -2
index ddd5f48551f1..ade772aa9610 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
Quoted 9 lines

kernel/sched/core.c static int migration_cpu_stop(void *data) · line 1074

Quoted 6 lines
kernel/sched/core.c +3 -0
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);

And this, too narrow a constraint do git diff made it go away.

yes, set wake_cpu is better when try_to_wake_up(). Peter, Is it as a new patch?

Quoted 11 lines
---
 kernel/stop_machine.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/stop_machine.c +0 -0
index ae6f41fb9cba..637798d6b554 100644
@@ -121,6 +121,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 131

Quoted 10 lines
kernel/stop_machine.c +5 -0
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }

I agree to use cond_resched(). https://lkml.org/lkml/2016/9/12/1228

for CONFIG_PREEMPT=y and CONFIG_PREEMPT_VOLUNTARY=y, this patch seems unnecessary.
1. when CONFIG_PREEMPT=y
stop_one_cpu()
->cpu_stop_queue_work()

    ->spin_unlock_irqrestore() (preempt_enable() calls __preempt_schedule())

2. when CONFIG_PREEMPT_VOLUNTARY=y
stop_one_cpu()
->wait_for_completion()

    ->...
      ->might_sleep() (calls _cond_resched()

so we really don't need "if defined(CONFIG_PREEMPT_NONE)"?

I also think without the "if defined(CONFIG_PREEMPT_NONE)", the code is more clean,and the logic is still ok.

kernel/stop_machine.c +9 -0
index 4a1ca5f..87464a2 100644
@@ -126,6 +126,15 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
        cpu_stop_init_done(&done, 1);
        if (!cpu_stop_queue_work(cpu, &work))
                return -ENOENT;
+
+#if defined(CONFIG_PREEMPT_NONE)
+       /*
+        * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+        * by doing a preemption.
+        */
+       cond_resched();
+#endif
+
        wait_for_completion(&done.completion);
        return done.ret;
 }
maintainer
#21 lore raw
Quoted 2 lines · Cheng Chao

Peter, Is it as a new patch?

I wanted both changes in one pathc, but I fudged my git-diff.

Quoted 24 lines
Quoted 11 lines
---
 kernel/stop_machine.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/stop_machine.c +0 -0
index ae6f41fb9cba..637798d6b554 100644
@@ -121,6 +121,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);

kernel/stop_machine.c int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg) · line 131

Quoted 10 lines
kernel/stop_machine.c +5 -0
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }

I agree to use cond_resched(). https://lkml.org/lkml/2016/9/12/1228

Quoted 4 lines

so we really don't need "if defined(CONFIG_PREEMPT_NONE)"?

I also think without the "if defined(CONFIG_PREEMPT_NONE)", the code is more clean,and the logic is still ok.

You really don't need the #ifdef, look at the actual code, not the Kconfig language.

#22 lore raw

PATCH v4 1/1 stop_machine: Avoid a sleep and wakeup in the stop_one_cpu()

In reply to Cheng Chao

In case @cpu == smp_proccessor_id(), we can avoid a sleep+wakeup by doing a preemption.

the caller such as sched_exec can benefit from this change.

Signed-off-by Cheng Chao <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 kernel/sched/core.c   | 8 ++++++--
 kernel/stop_machine.c | 5 +++++
 2 files changed, 11 insertions(+), 2 deletions(-)
kernel/sched/core.c +6 -2
index a0086a5..283b662 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);
kernel/stop_machine.c +5 -0
index 4a1ca5f..1a24890 100644
@@ -126,6 +126,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
Signature · 1 line
2.4.11
#23 lore raw

Re: [PATCH v4] stop_machine: Avoid a sleep and wakeup in the stop_one_cpu()

Quoted 5 lines · Cheng Chao
kernel/sched/core.c +0 -0
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because

kernel/sched/core.c static int migration_cpu_stop(void *data) · line 1071

Quoted 10 lines
kernel/sched/core.c +6 -2
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}

Cough ;) again, I leave this to Peter...

But imo this change should be documented or perhaps even separated. It looks fine to me, but this has nothing to do with "we can avoid a sleep+wakeup by doing a preemption" from the changelog.

This is another improvement, and a small note in the changelog can unconfuse the reader of git blame/log.

Oleg.

#24 lore raw

Re: [PATCH v4] stop_machine: Avoid a sleep and wakeup in the stop_one_cpu()

In reply to Cheng Chao

Hi Peter,

What should I do next? Thanks.

Cheng

Quoted 49 lines · Cheng Chao

In case @cpu == smp_proccessor_id(), we can avoid a sleep+wakeup by doing a preemption.

the caller such as sched_exec can benefit from this change.

Signed-off-by Cheng Chao <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 kernel/sched/core.c   | 8 ++++++--
 kernel/stop_machine.c | 5 +++++
 2 files changed, 11 insertions(+), 2 deletions(-)
kernel/sched/core.c +6 -2
index a0086a5..283b662 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);
kernel/stop_machine.c +5 -0
index 4a1ca5f..1a24890 100644
@@ -126,6 +126,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * by doing a preemption.
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }
merge bot
#25 lore raw

[tip:sched/core] stop_machine: Avoid a sleep and wakeup in stop_one_cpu()

In reply to Cheng Chao
stop_machine: Avoid a sleep and wakeup in stop_one_cpu()
Commit-ID:  bf89a304722f6904009499a31dc68ab9a5c9742e
Gitweb:     http://git.kernel.org/tip/bf89a304722f6904009499a31dc68ab9a5c9742e
Author:     Cheng Chao <[email protected]>
AuthorDate: Wed, 14 Sep 2016 10:01:50 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 22 Sep 2016 14:53:45 +0200

stop_machine: Avoid a sleep and wakeup in stop_one_cpu()

In case @cpu == smp_proccessor_id(), we can avoid a sleep+wakeup cycle by doing a preemption.

Callers such as sched_exec() can benefit from this change.

Signed-off-by Cheng Chao <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched/core.c   | 8 ++++++--
 kernel/stop_machine.c | 5 +++++
 2 files changed, 11 insertions(+), 2 deletions(-)
kernel/sched/core.c +6 -2
index c5f020c..ff4e3c0 100644
@@ -1063,8 +1063,12 @@ static int migration_cpu_stop(void *data)
 	 * holding rq->lock, if p->on_rq == 0 it cannot get enqueued because
 	 * we're holding p->pi_lock.
 	 */
-	if (task_rq(p) == rq && task_on_rq_queued(p))
-		rq = __migrate_task(rq, p, arg->dest_cpu);
+	if (task_rq(p) == rq) {
+		if (task_on_rq_queued(p))
+			rq = __migrate_task(rq, p, arg->dest_cpu);
+		else
+			p->wake_cpu = arg->dest_cpu;
+	}
 	raw_spin_unlock(&rq->lock);
 	raw_spin_unlock(&p->pi_lock);
kernel/stop_machine.c +5 -0
index 4a1ca5f..082e71f 100644
@@ -126,6 +126,11 @@ int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
 	cpu_stop_init_done(&done, 1);
 	if (!cpu_stop_queue_work(cpu, &work))
 		return -ENOENT;
+	/*
+	 * In case @cpu == smp_proccessor_id() we can avoid a sleep+wakeup
+	 * cycle by doing a preemption:
+	 */
+	cond_resched();
 	wait_for_completion(&done.completion);
 	return done.ret;
 }