kthread, sched/wait: Fix kthread_parkme() wait-loop
Gaurav reported a problem with __kthread_parkme() where a concurrent try_to_wake_up() could result in competing stores to ->state which, when the TASK_PARKED store got lost bad things would happen. The comment near set_current_state() actually mentions this competing store, but only mentions the case against TASK_RUNNING. This same store, with different timing, can happen against a subsequent !RUNNING store. This normally is not a problem, because as per that same comment, the !RUNNING state store is inside a condition based wait-loop: for (;;) { set_current_state(TASK_UNINTERRUPTIBLE); if (!need_sleep) break; schedule(); } __set_current_state(TASK_RUNNING); If we loose the (first) TASK_UNINTERRUPTIBLE store to a previous (concurrent) wakeup, the schedule() will NO-OP and we'll go around the loop once more. The problem here is that the TASK_PARKED store is not inside the KTHREAD_SHOULD_PARK condition wait-loop. There is a genuine issue with sleeps that do not have a condition; this is addressed in a subsequent patch. Reported-by: Gaurav Kohli <gkohli@codeaurora.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Oleg Nesterov <oleg@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Ingo Molnar <mingo@kernel.org>
This commit is contained in:
parent
457be908c8
commit
741a76b350
|
@ -177,12 +177,13 @@ void *kthread_probe_data(struct task_struct *task)
|
||||||
|
|
||||||
static void __kthread_parkme(struct kthread *self)
|
static void __kthread_parkme(struct kthread *self)
|
||||||
{
|
{
|
||||||
__set_current_state(TASK_PARKED);
|
for (;;) {
|
||||||
while (test_bit(KTHREAD_SHOULD_PARK, &self->flags)) {
|
set_current_state(TASK_PARKED);
|
||||||
|
if (!test_bit(KTHREAD_SHOULD_PARK, &self->flags))
|
||||||
|
break;
|
||||||
if (!test_and_set_bit(KTHREAD_IS_PARKED, &self->flags))
|
if (!test_and_set_bit(KTHREAD_IS_PARKED, &self->flags))
|
||||||
complete(&self->parked);
|
complete(&self->parked);
|
||||||
schedule();
|
schedule();
|
||||||
__set_current_state(TASK_PARKED);
|
|
||||||
}
|
}
|
||||||
clear_bit(KTHREAD_IS_PARKED, &self->flags);
|
clear_bit(KTHREAD_IS_PARKED, &self->flags);
|
||||||
__set_current_state(TASK_RUNNING);
|
__set_current_state(TASK_RUNNING);
|
||||||
|
|
Loading…
Reference in New Issue