mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>,
	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>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [git pull] kgdb-light -v10
Date: Tue, 12 Feb 2008 10:11:13 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.1.00.0802121001180.2920@woody.linux-foundation.org> (raw)
In-Reply-To: <20080212182024.GA4940@one.firstfloor.org>



On Tue, 12 Feb 2008, Andi Kleen wrote:
>
> >  - the kgdb commands should always act on the *current* CPU only
> >  - add one command that says "switch over to CPU #n" which just releases 
> >    the current CPU and sends an IPI to that CPU #n (no timeouts, no 
> >    synchronous waiting, no nothing - it's like a "continue", but with a 
> >    "try to get the other CPU to stop"
> 
> The problem I see here is that the kernel tends to get badly confused
> if one CPU just stops responding. At some point someone does an global
> IPI and that then hangs.  You would need to hotunplug the CPU which
> is theoretically possible, but quite intrusive.

You're thinking about this totally *wrong*.

You definitely do not want to hot-unplug or isolate anything at all. 
That's explicitly against the whole point of kgdb not changing what it is 
trying to measure.

Just let the other CPU's hang naturally if they need to wait for IPI's 
etc. What's the downside? That's what you were trying to do in the first 
place by havign the kgdb callback!

So you can't have it both ways. Either serializing other cpu's with kgdb 
is good (the whole "kgdb_nmicallback" thing or whatever it was called), in 
which case it's also perfectly ok to just let them stop when waiting for 
IPI's.

My point was *not* that kgdb should take control of one CPU, and the other 
CPU's should continue to work as if nothing happened. That is insane and 
impossible (since you may be stopping a CPU while it holds central 
spinlocks etc). No, my point was that I think kgdb should be as light and 
non-intrusive as possible, and that any "higher level behaviour" (like the 
decision of whether to try to synchronize other CPU's or not) should be 
left to the debugger.

But only if that makes kgdb patches less intrusive!

In other words, I'm not at all trying to push any particular solution 
here, except for the "keep it simple, and anything even remotely debatable 
or intrusive to the system should be excised". And I wanted to point out 
that maybe all these timeout etc decisions can be pushed to the debugger.

So I think we can either:

 - have no timeouts or other fancy crap _at_all_, with very simple locking 
   (ie looks what v10 mostly seems to do)

 - or you do the fancy dance entirely in the remote debugger.

I don't care. The only thing I care about is that kgdb support never 
_ever_ shows up in any interesting code, and that it remains totally 
invisible to essentially all of the kernel except the place that would 
otherwise print out an oops.

And I absolutely don't want it to be fancy, I want it to be so simple that 
even _I_ can look at it and say "I think this is crap, but it's _trivial_ 
crap".

IOW: as long as people keep arguing about it, I sure as hell won't ever 
merge it. It needs to be so _obvious_ and so _minimal_ that I can feel 
that I finally don't need to care.

			Linus

  reply	other threads:[~2008-02-12 18:13 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
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 [this message]
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=alpine.LFD.1.00.0802121001180.2920@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --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 \
    /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

all inboxes | Powered by JetHome®