Crash in schedule path after worker thread dies
After a worker thread died due to a bug in a work function, a NULL dereference was seen when schedule calls wq_worker_sleeping calls kthread_data:
return to_kthread(task)->data;
mm_release has apparently already set the task's vfork_done = NULL, causing to_kthread to return a bad address (on 3.0-rc7 on ARM).
I haven't tried a fix because I'm not sure if avoiding this case is enough to properly recover from death of a worker thread, or if this has already been discussed and rejected in the past. I searched around a little and found some mentions of problems in worker functions that were probably followed by the kthread_data crash, but didn't turn up any specific discussion of this crash. So I thought I'd start by mentioning this here, and can help fix or test if needed.
Todd
Hello, Todd.
Quoted 9 lines · Todd Poynor
After a worker thread died due to a bug in a work function, a NULL dereference was seen when schedule calls
wq_worker_sleepingcallskthread_data:return to_kthread(task)->data;
mm_releasehas apparently already set the task'svfork_done= NULL, causingto_kthreadto return a bad address (on 3.0-rc7 on ARM).
Quoted 8 lines
I haven't tried a fix because I'm not sure if avoiding this case is enough to properly recover from death of a worker thread, or if this has already been discussed and rejected in the past. I searched around a little and found some mentions of problems in worker functions that were probably followed by the
kthread_datacrash, but didn't turn up any specific discussion of this crash. So I thought I'd start by mentioning this here, and can help fix or test if needed.
Hmmm... the scheduler code is oopsing after the worker already oopsed, escalating the failure mode, right? Avoiding the scheduler oops can be done simply by clearing PF_WQ_WORKER in do_exit(); however, it would still leave workqueue code assuming the worker is busy running and may lead to workqueue stall.
Cleaning that up would require more code but at that point it isn't really clear what we would be buying with the added complexity. Hmmm... maybe it would still be worthwhile to at least keep scheduler working. Something like this?
kernel/exit.c +1 -0
@@ -915,6 +915,7 @@ NORET_TYPE void do_exit(long code)* kernel address. */ set_fs (USER_DS ); + tsk->flags &= ~PF_WQ_WORKER ;ptrace_event (PTRACE_EVENT_EXIT , code);
Signature · 1 line
tejun
Hello Tejun,
Quoted 11 lines · Tejun Heo
Quoted 9 lines · Todd Poynor
After a worker thread died due to a bug in a work function, a NULL dereference was seen when schedule calls
wq_worker_sleepingcallskthread_data:return to_kthread(task)->data;
mm_releasehas apparently already set the task'svfork_done= NULL, causingto_kthreadto return a bad address (on 3.0-rc7 on ARM). [...]
Quoted 5 lines
Hmmm... the scheduler code is oopsing after the worker already oopsed, escalating the failure mode, right? Avoiding the scheduler oops can be done simply by clearing
PF_WQ_WORKERindo_exit(); however, it would still leave workqueue code assuming the worker is busy running and may lead to workqueue stall.
Right, the worker oops took out that thread (don't know if workqueue processing would have gracefully recovered if not for the scheduler oops), but the scheduler oops was catastrophic.
Quoted 10 lines
Cleaning that up would require more code but at that point it isn't really clear what we would be buying with the added complexity. Hmmm... maybe it would still be worthwhile to at least keep scheduler working. Something like this?
kernel/exit.c +0
-0
kernel/exit.c NORET_TYPE void do_exit(long code) · line 920
Quoted 6 lines
kernel/exit.c +1
-0@@ -915,6 +915,7 @@ NORET_TYPE void do_exit(long code)* kernel address. */ set_fs (USER_DS ); + tsk->flags &= ~PF_WQ_WORKER ;ptrace_event (PTRACE_EVENT_EXIT , code);
Sounds good to me, thanks!
Todd
Quoted 3 lines
kernel/exit.c +0
-1--tejun