On 06/25/14 17:43, Thomas Gleixner wrote:
The kthread park/unpark logic has the following issue:
Task CPU 0 CPU 1
T1 unplug cpu1
kthread_park(T2)
set_bit(KTHREAD_SHOULD_PARK);
wait_for_completion()
T2 parkme(X)
__set_current_state(TASK_PARKED);
while (test_bit(KTHREAD_SHOULD_PARK))
{
if
(!test_and_set_bit(KTHREAD_IS_PARKED))
complete();
schedule();
T1 plug cpu1
--> premature wakeup of T2, i.e. before unpark, so T2 gets scheduled on
CPU 0
I understood the explanation above. But still I don't understand how
this premature wakeup of T2 is happening/possible? Also, what will
happen if the task state is not in TASK_PARKED when __kthread_unpark is
called? __kthread_bind will fail silently causing the same problem.
Reorder the logic so that the unplug code binds the thread to the
target cpu before clearing the KTHREAD_SHOULD_PARK bit.
Reported-by: Subbaraman Narayanamurthy<subba...@codeaurora.org>
Signed-off-by: Thomas Gleixner<t...@linutronix.de>
Cc:sta...@vger.kernel.org
---
kernel/kthread.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
Index: linux/kernel/kthread.c
===================================================================
--- linux.orig/kernel/kthread.c
+++ linux/kernel/kthread.c
@@ -382,6 +382,15 @@ struct task_struct *kthread_create_on_cp
static void __kthread_unpark(struct task_struct *k, struct kthread *kthread)
{
+ /*
+ * Rebind the thread to the target cpu first if it is a per
+ * cpu thread unconditionally because it must be bound to the
+ * target cpu before it can observe the KTHREAD_SHOULD_PARK
+ * bit cleared.
+ */
+ if (test_bit(KTHREAD_IS_PER_CPU, &kthread->flags))
+ __kthread_bind(k, kthread->cpu, TASK_PARKED);
+
clear_bit(KTHREAD_SHOULD_PARK, &kthread->flags);
/*
* We clear the IS_PARKED bit here as we don't wait
@@ -389,11 +398,8 @@ static void __kthread_unpark(struct task
* park before that happens we'd see the IS_PARKED bit
* which might be about to be cleared.
*/
- if (test_and_clear_bit(KTHREAD_IS_PARKED, &kthread->flags)) {
- if (test_bit(KTHREAD_IS_PER_CPU, &kthread->flags))
- __kthread_bind(k, kthread->cpu, TASK_PARKED);
+ if (test_and_clear_bit(KTHREAD_IS_PARKED, &kthread->flags))
wake_up_state(k, TASK_PARKED);
- }
}
/**
Thanks for the patch. I've tested (running hotplug tests) it for
sometime and looks good so far. Can you please submit it?
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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/
