| From foo@baz Sun Jun 17 12:07:34 CEST 2018 |
| From: Peter Zijlstra <peterz@infradead.org> |
| Date: Mon, 30 Apr 2018 14:50:22 +0200 |
| Subject: kthread, sched/wait: Fix kthread_parkme() wait-loop |
| |
| From: Peter Zijlstra <peterz@infradead.org> |
| |
| [ Upstream commit 741a76b350897604c48fb12beff1c9b77724dc96 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/kthread.c | 7 ++++--- |
| 1 file changed, 4 insertions(+), 3 deletions(-) |
| |
| --- a/kernel/kthread.c |
| +++ b/kernel/kthread.c |
| @@ -177,12 +177,13 @@ void *kthread_probe_data(struct task_str |
| |
| static void __kthread_parkme(struct kthread *self) |
| { |
| - __set_current_state(TASK_PARKED); |
| - while (test_bit(KTHREAD_SHOULD_PARK, &self->flags)) { |
| + for (;;) { |
| + set_current_state(TASK_PARKED); |
| + if (!test_bit(KTHREAD_SHOULD_PARK, &self->flags)) |
| + break; |
| if (!test_and_set_bit(KTHREAD_IS_PARKED, &self->flags)) |
| complete(&self->parked); |
| schedule(); |
| - __set_current_state(TASK_PARKED); |
| } |
| clear_bit(KTHREAD_IS_PARKED, &self->flags); |
| __set_current_state(TASK_RUNNING); |