mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: Re: v2.6.31-rc6: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
Date: Mon, 24 Aug 2009 21:30:16 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0908242113540.3218@localhost.localdomain> (raw)
In-Reply-To: <alpine.LFD.2.01.0908242043280.3218@localhost.localdomain>



On Mon, 24 Aug 2009, Linus Torvalds wrote:
> 
> Anyway, I'll happily be shown wrong. I think the (second) patch I sent out 
> is an acceptable hack in the presense of the current locking, but as I 
> said, I'm not exactly happy about it, because I do think the locking is 
> broken.

Btw, another solution to all this would be to just not have that 
ldisc_mutex deadlock due to do_tty_hangup -> tty_ldisc_hangup at all.

The actual _flushing_ doesn't need the mutex - it's just that both 
flushing and hangup is done with workqueues.

If we can avoid the deadlock by not having the (artificial) workqueue 
dependency, it would allow everybody to just hold on to the mutex over the 
whole sequence - and would obviate the need for that hacky 
TTY_LDISC_CHANGING bit thing in tty_set_ldisc.

In other words, the whole problem really comes in from the fact that 
do_tty_hangup() is called from "hangup_work", and the workqueues can get 
hung to the point where you can't then do the (totally _unrelated_) queue 
flushing.

Because flush_to_ldisc() itself - which is what we want to do - doesn't 
need that mutex or the workqueue at all. It could run from any context, 
afaik.

So if we were to turn it into just a timer (rather than a "delayed work"), 
then we'd not need to do that "flush_scheduled_work()" thing at all, and 
we wouldn't have that interaction with do_tty_hangup(). At which point we 
could again hold on to locks, because we wouldn't need to worry about the 
workqueues getting stuck on the mutex (that isn't even needed for the 
actual flushing part that we want to do!).

So don't get me wrong - there are _multiple_ ways to solve this. But they 
are all pretty major surgery, changing "big" semantics. We could fix the 
locking, we could change how we flush, we could do all of those things. 
And I'd love to. But I think the almost-oneliner is the safest approach 
right now. It's certainly not perfect, but it's fairly minimal impact.

		Linus

  reply	other threads:[~2009-08-25  4:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-20  5:46 Eric W. Biederman
2009-08-20  6:07 ` Eric W. Biederman
     [not found]   ` <7b6bb4a50908200010h1c60d007p4fa017fd97c87c19@mail.gmail.com>
2009-08-20  7:33     ` Eric W. Biederman
2009-08-20  9:23       ` Xiaotian Feng
2009-08-21  2:09         ` Zhang, Yanmin
2009-08-21 18:23           ` Eric W. Biederman
2009-08-20  7:54   ` Dave Young
2009-08-20  8:00     ` Eric W. Biederman
2009-08-20  8:19       ` Dave Young
2009-08-24 22:34   ` Linus Torvalds
2009-08-24 23:51     ` Linus Torvalds
2009-08-25  0:09       ` Linus Torvalds
2009-08-25  1:41         ` Eric W. Biederman
2009-08-25  2:48         ` Dave Young
2009-08-25  3:08         ` Xiaotian Feng
2009-08-25  6:16           ` Zhang, Yanmin
2009-08-25  3:39         ` Frederic Weisbecker
2009-08-25  4:10           ` Linus Torvalds
2009-08-25  4:30             ` Linus Torvalds [this message]
2009-08-25 15:05               ` Frederic Weisbecker
2009-08-25 14:24             ` Frederic Weisbecker
2009-08-27  9:15         ` Zhang, Yanmin

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=alpine.LFD.2.01.0908242113540.3218@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=ebiederm@xmission.com \
    --cc=fweisbec@gmail.com \
    --cc=gregkh@suse.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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

Powered by JetHome