Message-ID: <20080212135027.GA1343@one.firstfloor.org>
Date:	Tue, 12 Feb 2008 14:50:27 +0100
From:	Andi Kleen <andi@...stfloor.org>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	"Frank Ch. Eigler" <fche@...hat.com>,
	Roland McGrath <roland@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [git pull] kgdb-light -v10

On Tue, Feb 12, 2008 at 01:38:39PM +0100, Ingo Molnar wrote:
> So unless i forgot about something (please yell if so), it seems to me 
> kgdb is now pretty ready for an upstream merge.

I don't know -- I have not reread everything. Please don't consider
my comments as approval of the code base. I still think it does quite
a lot of dubious and ugly things overall and should get far more clean up
and get more testing too.

> do spinning for now: we dont _ever_ want to break a correctly working 
> system with kgdb.

Stopping all CPUs for indefinite time very much seems like 
"breaking a correctly working system" to me.  In a correctly working system 
kgdb is never entered.

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

The only way I know of to do that is gdb vmlinux /proc/kcore 
kgdb certainly isn't it.

> And note that kgdb is not just a "debugger", it's a system inspection 
> tool. An intelligent, human-controlled printk.

For that gdb vmlinux /proc/kcore already works fine. Or fireproxy.
If that was the only goal we wouldn't need all that stub code.


> > > just introduce unnecessary complexity.
> > 
> > The question is less about actually having it as a module, but just if 
> > the interfaces are clean enough to allow it as a module. If not you 
> > should probably clean them up.
> 
> but your contention is simply wrong. Most of our debugging 
> infrastructure is non-modular for a good reason. Modularization 
> increases complexity and that's exactly the wrong direction for 

The main complexity in module handling is handling (or rather preventing)
module unload.  I explicitely excluded that in my earlier mail. 
Module loading on the other hand tends to be relatively easy.

I did a modular kernel debugger on my own some time ago and once
the interfaces were clean it was very simple. I think the reverse
is true too -- if having it as a module is easy then the interfaces
are clean too. That is why I asked for it. It's a good basic
sanity check on the design.

> 
> > > 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().

If that is true then it is definitely misnamed and likely 
incorrectly implemented on the architecture in question.

> > [...] If kgdb is active it should have priority over crash dumps.
> 
> that's the approach we are taking: be as unintrusive as possible. This 
> means that the notifier here is registered at the lowest priority. You 
> might disagree with it but it's a completely sensible and consistent 
> approach.

Yeah, it is consistently wrong agreed.

-Andi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists