public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Todd Poynor <toddpoynor@google.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: Crash in schedule path after worker thread dies
Date: Wed, 20 Jul 2011 19:07:17 +0200	[thread overview]
Message-ID: <20110720170717.GF9924@mtj.dyndns.org> (raw)
In-Reply-To: <20110720000738.GA18774@google.com>

Hello, Todd.

On Tue, Jul 19, 2011 at 05:07:38PM -0700, Todd Poynor wrote:
> 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.

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?

diff --git a/kernel/exit.c b/kernel/exit.c
index 6c7fbbe..412ead0 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -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);
 
-- 
tejun

  reply	other threads:[~2011-07-20 17:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-20  0:07 Crash in schedule path after worker thread dies Todd Poynor
2011-07-20 17:07 ` Tejun Heo [this message]
2011-07-20 18:06   ` Todd Poynor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110720170717.GF9924@mtj.dyndns.org \
    --to=tj@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=toddpoynor@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox