mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
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>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [git pull] kgdb-light -v10
Date: Tue, 12 Feb 2008 08:46:51 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.1.00.0802120829410.2920@woody.linux-foundation.org> (raw)
In-Reply-To: <20080212152846.GC3078@elte.hu>



On Tue, 12 Feb 2008, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> > 
> > Stopping all CPUs for indefinite time very much seems like "breaking a 
> > correctly working system" to me. [...]
> 
> well, this is a small detail, but still you are wrong, and on a 
> correctly working system this will not occur. (if yes, tell me how)

Quite frankly, I don't see why the kernel kgdb layer should have *any* 
code like this at all.

The one who is actually debugging is the one who should decide which CPU's 
get stopped, and which don't.

I realize that the gdb remote protocol is probably a piece of crap and 
cannot handle that, but hey, that's not my problem, and more importantly, 
I don't think it's even a *remotely* valid reason for making bad decisions 
in the kernel. gdb was still open source last time I saw, and I think it's 
reasonable to just say:

 - 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"

Yes, other CPU's will obviously often end up stopping due to waiting for 
some spinlock or other if we stop one, but that's a separate issue, and 
quite often it might be sufficient - and what we want.

And yes, you'd likely have to add some support to gdb to make this 
_usable_, but now all that usability crap, all those timeouts for "stop 
all CPU's" are now in user space on the _debugger_ side. That can be as 
fancy as it wants to be.

And maybe this isn't realistic. I'm not saying "we _must_ do it this way", 
I just want to say that the kernel kgdb layer should be as thin ass 
humanly possible, and maybe the right thing to do is to simply totally 
punt on the whole "stop other cpu's" issue and make it a debugger-side 
question.

In other words, is it perhaps possible to just *get*rid*of* that 
"kgdb_active" and "nmicallback" and the whole multi-CPU roundup? Just use 
a kgdb spinlock around the stuff that actually sends and receives 
individual packets, and expect the debugger side to sort them out (yeah, I 
suspect this involves having to add the CPU ID to each packet).

			Linus

  parent reply	other threads:[~2008-02-12 16:48 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 [this message]
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=alpine.LFD.1.00.0802120829410.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®