linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wessel <jason.wessel@windriver.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org,
	"Frank Ch. Eigler" <fche@redhat.com>,
	Roland McGrath <roland@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [git pull] kgdb-light -v10
Date: Tue, 12 Feb 2008 07:30:15 -0600	[thread overview]
Message-ID: <47B19F67.5050105@windriver.com> (raw)
In-Reply-To: <20080212123839.GA15360@elte.hu>

Ingo Molnar wrote:
> * Andi Kleen <andi@firstfloor.org> wrote:
>
>   
>>> i went for correctness and simplicity first. If a system is hung, 
>>> the debugging CPU might hang too at any time. A timeout on the other 
>>> hand introduces the possibility of a 'dead' CPU just coming back to 
>>> life after the 'timeout', corrupting debugger data. So for now the 
>>> rule is very simple.
>>>       
>> If all code is correct, it likely won't need a debugger. But if you 
>> write a debugger you can't assume that.
>>     
>
> i gave you very specific technological reasons for why we dont want to 
> do spinning for now: we dont _ever_ want to break a correctly working 
> system with kgdb.
>
> A valid counter-argument is _not_ to argue "but it would be nice to have 
> if the system is broken in X, Y and Z ways" (like you did), but to point 
> it out why the behavior we chose is wrong on a correctly working system.
>
> Yes, a buggy system might misbehave in various ways but my primary 
> interest is in keeping correctly working systems correct.
>
> And note that kgdb is not just a "debugger", it's a system inspection 
> tool. An intelligent, human-controlled printk. A kernel internals 
> learning tool. An extension to the kernel console concept. Yes, people 
> frequently use it for debugging too, but the other uses are actually 
> more important in the big picture than the debugging aspect.
>
>   

This is not a technical argument, but I am not a big fan of hard hanging
the system if you cannot sync all the CPUs. The original intent was to
at least provide a sync error message to the end user after some
reasonable time. Then allow someone to collect any data you can get and
you basically have to reboot. The reboot was never forced, but assumed
the end users of this knew what they were doing in the first place.

Certainly in a completely working system where you use kgdb only for
inspection this is not an issue, unless you use a breakpoint or single
step one of the smp_call functions. As we all know there are lots of
ways to crash a perfectly working system.

>   
>>> no, not all architectures have it. This is a weak alias that is 
>>> otherwise not linked into the kernel.
>>>       
>> Can't be very many because oprofile needs it and it works on most 
>> archs now. Anyways, the right thing is to just add it to the 
>> architectures that still miss it, not reimplement it in kgdb.
>>     
>
> it's not reimplemented - kgdb_arch_pc() does not directly map to 
> instruction_pointer().
>
>   

We might be best served to add a comment to explain the purpose of
kgdb_arch_pc() and put it in the optional implementation function
headers in include/linux/kgdb.h

On some archs certain exceptions do not report the address that the
exception occurred at when you call instruction_pointer(). This optional
function allows for an arch to perform a "fixup" to get the address the
exception actually occurred at.

Kgdb requires the actual exception address so a sanity check can be
performed to make sure kgdb did not hit an exception while in a chunk of
code kgdb requires for its functionality. If you hit one of these
conditions kgdb makes its best attempt to try to "patch the wound"
inflicted by shooting yourself but at least you get notified vs a silent
hang :-)

Jason.

  reply	other threads:[~2008-02-12 13:32 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-11  1:53 kgdb in git-x86#mm review Andi Kleen
2008-02-11 15:32 ` Frank Ch. Eigler
2008-02-11 16:11   ` Andi Kleen
2008-02-11 16:21   ` [git pull] kgdb-light -v8, (was: Re: kgdb in git-x86#mm review) Ingo Molnar
2008-02-11 16:41     ` [git pull] kgdb-light -v8, Jan Kiszka
2008-02-11 16:54       ` Ingo Molnar
2008-02-11 17:10     ` [git pull] kgdb-light -v8, (was: Re: kgdb in git-x86#mm review) Andi Kleen
2008-02-11 23:03       ` [git pull] kgdb-light -v9 Ingo Molnar
2008-02-12 10:03         ` Andi Kleen
2008-02-12  9:35           ` Sam Ravnborg
2008-02-12 10:26           ` Roland McGrath
2008-02-12 10:34             ` Ingo Molnar
2008-02-12 11:27           ` [git pull] kgdb-light -v10 Ingo Molnar
2008-02-12 12:19             ` Andi Kleen
2008-02-12 12:38               ` Ingo Molnar
2008-02-12 13:30                 ` Jason Wessel [this message]
2008-02-12 14:39                   ` Andi Kleen
2008-02-12 14:35                     ` Jason Wessel
2008-02-12 15:36                       ` Andi Kleen
2008-02-12 16:21                         ` Jason Wessel
2008-02-12 17:10                           ` Andi Kleen
2008-02-12 16:48                             ` Jason Wessel
2008-02-12 13:50                 ` Andi Kleen
2008-02-12 15:16                   ` Ingo Molnar
2008-02-12 15:28                     ` Andi Kleen
2008-02-12 15:28                   ` Ingo Molnar
2008-02-12 16:11                     ` Andi Kleen
2008-02-12 16:24                       ` Ingo Molnar
2008-02-12 17:01                         ` Andi Kleen
2008-02-12 16:25                       ` Linus Torvalds
2008-02-12 16:42                         ` Ingo Molnar
2008-02-12 17:07                         ` Andi Kleen
2008-02-15 12:35                           ` [RFC][PATCH] modular kgdb-light (was: Re: [git pull] kgdb-light -v10) Jan Kiszka
2008-02-15 13:32                             ` Andi Kleen
2008-02-15 20:24                               ` [RFC][PATCH] modular kgdb-light Jason Wessel
2008-02-15 20:36                         ` [git pull] kgdb-light -v10 Jason Wessel
2008-02-12 16:46                     ` Linus Torvalds
2008-02-12 17:01                       ` Ingo Molnar
2008-02-12 17:10                         ` Ingo Molnar
2008-02-12 18:20                       ` Andi Kleen
2008-02-12 18:11                         ` Linus Torvalds
2008-02-12 19:22                           ` Andi Kleen
2008-02-12 19:01                             ` Linus Torvalds
2008-02-12 18:20                         ` Andrew Morton
2008-02-12 19:16                           ` Andi Kleen
2008-02-12 21:01                             ` Ingo Molnar
2008-02-12 19:34                           ` Frank Ch. Eigler
2008-02-12 20:16                             ` Andi Kleen
2008-02-12 13:18             ` Domenico Andreoli
2008-02-12 13:59               ` Jason Wessel
2008-02-12 15:45                 ` Domenico Andreoli
2008-02-11 16:03 ` kgdb in git-x86#mm review Mark Lord

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=47B19F67.5050105@windriver.com \
    --to=jason.wessel@windriver.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=fche@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).