Commit b80a2bfc authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Thomas Gleixner
Browse files

stop_machine: Reflow cpu_stop_queue_two_works() 



The code flow in cpu_stop_queue_two_works() is a little arcane; fix this by
lifting the preempt_disable() to the top to create more natural nesting wrt
the spinlocks and make the wake_up_q() and preempt_enable() unconditional
at the end.

Furthermore, enable preemption in the -EDEADLK case, such that we spin-wait
with preemption enabled.

Suggested-by: default avatarThomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: isaacm@codeaurora.org
Cc: matt@codeblueprint.co.uk
Cc: psodagud@codeaurora.org
Cc: gregkh@linuxfoundation.org
Cc: pkondeti@codeaurora.org
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20180730112140.GH2494@hirez.programming.kicks-ass.net
parent b6a60cf3

kernel/stop_machine.c

+23 −18
Original line number Original line Diff line number Diff line
@@ -236,13 +236,24 @@ static int cpu_stop_queue_two_works(int cpu1, struct cpu_stop_work *work1, 
	struct cpu_stopper *stopper2 = per_cpu_ptr(&cpu_stopper, cpu2);

	struct cpu_stopper *stopper2 = per_cpu_ptr(&cpu_stopper, cpu2);

	DEFINE_WAKE_Q(wakeq);

	DEFINE_WAKE_Q(wakeq);

	int err;

	int err;



retry:

retry:

	/*

	 * 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 not wake up the other

	 * stopper forever.

	 */

	preempt_disable();

	raw_spin_lock_irq(&stopper1->lock);

	raw_spin_lock_irq(&stopper1->lock);

	raw_spin_lock_nested(&stopper2->lock, SINGLE_DEPTH_NESTING);

	raw_spin_lock_nested(&stopper2->lock, SINGLE_DEPTH_NESTING);





	if (!stopper1->enabled || !stopper2->enabled) {

		err = -ENOENT;

		err = -ENOENT;

	if (!stopper1->enabled || !stopper2->enabled)

		goto unlock;

		goto unlock;

	}



	/*

	/*

	 * Ensure that if we race with __stop_cpus() the stoppers won't get

	 * Ensure that if we race with __stop_cpus() the stoppers won't get

	 * queued up in reverse order leading to system deadlock.

	 * queued up in reverse order leading to system deadlock.
@@ -253,36 +264,30 @@ retry: 
	 * It can be falsely true but it is safe to spin until it is cleared,

	 * It can be falsely true but it is safe to spin until it is cleared,

	 * queue_stop_cpus_work() does everything under preempt_disable().

	 * queue_stop_cpus_work() does everything under preempt_disable().

	 */

	 */

	if (unlikely(stop_cpus_in_progress)) {

		err = -EDEADLK;

		err = -EDEADLK;

	if (unlikely(stop_cpus_in_progress))

		goto unlock;

		goto unlock;

	}





	err = 0;

	err = 0;

	__cpu_stop_queue_work(stopper1, work1, &wakeq);

	__cpu_stop_queue_work(stopper1, work1, &wakeq);

	__cpu_stop_queue_work(stopper2, work2, &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:

unlock:

	raw_spin_unlock(&stopper2->lock);

	raw_spin_unlock(&stopper2->lock);

	raw_spin_unlock_irq(&stopper1->lock);

	raw_spin_unlock_irq(&stopper1->lock);





	if (unlikely(err == -EDEADLK)) {

	if (unlikely(err == -EDEADLK)) {

		preempt_enable();



		while (stop_cpus_in_progress)

		while (stop_cpus_in_progress)

			cpu_relax();

			cpu_relax();



		goto retry;

		goto retry;

	}

	}





	if (!err) {

	wake_up_q(&wakeq);

	wake_up_q(&wakeq);

	preempt_enable();

	preempt_enable();

	}





	return err;

	return err;

}

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE