All of lore.kernel.org
 help / color / mirror / Atom feed
From: "tip-bot for Isaac J. Manjarres" <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: psodagud@codeaurora.org, linux-kernel@vger.kernel.org,
	tglx@linutronix.de, hpa@zytor.com, mingo@kernel.org,
	isaacm@codeaurora.org, torvalds@linux-foundation.org,
	pkondeti@codeaurora.org, peterz@infradead.org
Subject: [tip:sched/core] stop_machine: Disable preemption after queueing stopper threads
Date: Wed, 25 Jul 2018 07:21:09 -0700	[thread overview]
Message-ID: <tip-2610e88946632afb78aa58e61f11368ac4c0af7b@git.kernel.org> (raw)
In-Reply-To: <1531856129-9871-1-git-send-email-isaacm@codeaurora.org>

Commit-ID:  2610e88946632afb78aa58e61f11368ac4c0af7b
Gitweb:     https://git.kernel.org/tip/2610e88946632afb78aa58e61f11368ac4c0af7b
Author:     Isaac J. Manjarres <isaacm@codeaurora.org>
AuthorDate: Tue, 17 Jul 2018 12:35:29 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 25 Jul 2018 11:25:08 +0200

stop_machine: Disable preemption after queueing stopper threads

This commit:

  9fb8d5dc4b64 ("stop_machine, Disable preemption when waking two stopper threads")

does not fully address the race condition that can occur
as follows:

On one CPU, call it CPU 3, thread 1 invokes
cpu_stop_queue_two_works(2, 3,...), and the execution is such
that thread 1 queues the works for migration/2 and migration/3,
and is preempted after releasing the locks for migration/2 and
migration/3, but before waking the threads.

Then, On CPU 2, a kworker, call it thread 2, is running,
and it invokes cpu_stop_queue_two_works(1, 2,...), such that
thread 2 queues the works for migration/1 and migration/2.
Meanwhile, on CPU 3, thread 1 resumes execution, and wakes
migration/2 and migration/3. This means that when CPU 2
releases the locks for migration/1 and migration/2, but before
it wakes those threads, it can be preempted by migration/2.

If thread 2 is preempted by migration/2, then migration/2 will
execute the first work item successfully, since migration/3
was woken up by CPU 3, but when it goes to execute the second
work item, it disables preemption, calls multi_cpu_stop(),
and thus, CPU 2 will wait forever for migration/1, which should
have been woken up by thread 2. However migration/1 cannot be
woken up by thread 2, since it is a kworker, so it is affine to
CPU 2, but CPU 2 is running migration/2 with preemption
disabled, so thread 2 will never run.

Disable preemption after queueing works for stopper threads
to ensure that the operation of queueing the works and waking
the stopper threads is atomic.

Co-Developed-by: Prasad Sodagudi <psodagud@codeaurora.org>
Co-Developed-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
Signed-off-by: Prasad Sodagudi <psodagud@codeaurora.org>
Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bigeasy@linutronix.de
Cc: gregkh@linuxfoundation.org
Cc: matt@codeblueprint.co.uk
Fixes: 9fb8d5dc4b64 ("stop_machine, Disable preemption when waking two stopper threads")
Link: http://lkml.kernel.org/r/1531856129-9871-1-git-send-email-isaacm@codeaurora.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/stop_machine.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 1ff523dae6e2..e190d1ef3a23 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -260,6 +260,15 @@ retry:
 	err = 0;
 	__cpu_stop_queue_work(stopper1, work1, &wakeq);
 	__cpu_stop_queue_work(stopper2, work2, &wakeq);
+	/*
+	 * The waking up of stopper threads has to happen
+	 * in the same scheduling context as the queueing.
+	 * Otherwise, there is a possibility of one of the
+	 * above stoppers being woken up by another CPU,
+	 * and preempting us. This will cause us to n ot
+	 * wake up the other stopper forever.
+	 */
+	preempt_disable();
 unlock:
 	raw_spin_unlock(&stopper2->lock);
 	raw_spin_unlock_irq(&stopper1->lock);
@@ -271,7 +280,6 @@ unlock:
 	}
 
 	if (!err) {
-		preempt_disable();
 		wake_up_q(&wakeq);
 		preempt_enable();
 	}

      parent reply	other threads:[~2018-07-25 14:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 19:35 [PATCH] stop_machine: Disable preemption after queueing stopper threads Isaac J. Manjarres
2018-07-24  1:13 ` isaacm
2018-07-24  6:23   ` Sebastian Andrzej Siewior
2018-07-25  4:15     ` isaacm
2018-07-30 10:20     ` Thomas Gleixner
2018-07-30 11:21       ` Peter Zijlstra
2018-07-30 12:41         ` Thomas Gleixner
2018-07-30 17:12           ` Sodagudi Prasad
2018-07-30 17:16             ` Thomas Gleixner
2018-07-30 21:07             ` Peter Zijlstra
2018-08-01  8:07               ` Sodagudi Prasad
2018-08-06  8:37                 ` Pavan Kondeti
2018-08-02 12:06         ` [tip:sched/core] stop_machine: Reflow cpu_stop_queue_two_works() tip-bot for Peter Zijlstra
2018-08-02 13:27         ` tip-bot for Peter Zijlstra
2018-07-25 14:21 ` tip-bot for Isaac J. Manjarres [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-2610e88946632afb78aa58e61f11368ac4c0af7b@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=hpa@zytor.com \
    --cc=isaacm@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pkondeti@codeaurora.org \
    --cc=psodagud@codeaurora.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.