[PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Dave Jiang]

The RESTORE_CONTEXT macro is missing the '\n' at the end. It was removed in the
previous patch that touched system.h. It causes compile failure if any
inline asm is added after the macro. Discovered this when playing with
kgdb.

Signed-off-by: Dave Jiang <djiang@mvista.com>

---

 include/asm-x86_64/system.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

---

diff --git a/include/asm-x86_64/system.h b/include/asm-x86_64/system.h
index bd376bc..cbb8579 100644
--- a/include/asm-x86_64/system.h
+++ b/include/asm-x86_64/system.h
@@ -15,7 +15,7 @@
 
 /* frame pointer must be last for get_wchan */
 #define SAVE_CONTEXT    "pushf ; pushq %%rbp ; movq %%rsi,%%rbp\n\t"
-#define RESTORE_CONTEXT "movq %%rbp,%%rsi ; popq %%rbp ; popf\t"
+#define RESTORE_CONTEXT "movq %%rbp,%%rsi ; popq %%rbp ; popf\n\t"
 
 #define __EXTRA_CLOBBER  \
 	,"rcx","rbx","rdx","r8","r9","r10","r11","r12","r13","r14","r15"

Re: [PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Andi Kleen]

On Wednesday 07 March 2007 21:45, Dave Jiang wrote:
> 
> The RESTORE_CONTEXT macro is missing the '\n' at the end. It was removed in the
> previous patch that touched system.h. It causes compile failure if any
> inline asm is added after the macro. Discovered this when playing with
> kgdb.

We went through this several times. kgdb has no business to poke 
into kernel private macros like this and it shouldn't do that. 
So fixing kgdb to not require touching the context switch is the right
change.

-Andi

Re: [PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Dave Jiang]

Andi Kleen wrote:
> On Wednesday 07 March 2007 21:45, Dave Jiang wrote:
>> The RESTORE_CONTEXT macro is missing the '\n' at the end. It was removed in the
>> previous patch that touched system.h. It causes compile failure if any
>> inline asm is added after the macro. Discovered this when playing with
>> kgdb.
> 
> We went through this several times. kgdb has no business to poke 
> into kernel private macros like this and it shouldn't do that. 
> So fixing kgdb to not require touching the context switch is the right
> change.
> 
> -Andi

In spite of kgdb, shouldn't it have that \n anyways in case some other code
gets added in the future after the macro? Or are you saying that there should
never be any code ever after that macro?

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Andi Kleen]

On Thursday 08 March 2007 18:44, Dave Jiang wrote:

> In spite of kgdb, shouldn't it have that \n anyways in case some other code
> gets added in the future after the macro? Or are you saying that there should
> never be any code ever after that macro?

Sure if there is mainline code added after that macro we add the \n.
But only if it makes sense to add code there, which it didn't in kgdb.

-Andi

Re: [PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Tom Rini]

On Thu, Mar 08, 2007 at 07:37:56PM +0100, Andi Kleen wrote:
> On Thursday 08 March 2007 18:44, Dave Jiang wrote:
> 
> > In spite of kgdb, shouldn't it have that \n anyways in case some other code
> > gets added in the future after the macro? Or are you saying that there should
> > never be any code ever after that macro?
> 
> Sure if there is mainline code added after that macro we add the \n.
> But only if it makes sense to add code there, which it didn't in kgdb.

Was that because with recent enough tools and config options there was
enough annotations so GDB could finally figure out where things had
stopped?  Thanks.

-- 
Tom Rini

Permanent Kgdb integration into the kernel - lets get with it.

[Piet Delaney]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

On Thu, 2007-03-08 at 11:49 -0700, Tom Rini wrote:
> On Thu, Mar 08, 2007 at 07:37:56PM +0100, Andi Kleen wrote:
> > On Thursday 08 March 2007 18:44, Dave Jiang wrote:
> > 
> > > In spite of kgdb, shouldn't it have that \n anyways in case some other code
> > > gets added in the future after the macro? Or are you saying that there should
> > > never be any code ever after that macro?
> > 
> > Sure if there is mainline code added after that macro we add the \n.
> > But only if it makes sense to add code there, which it didn't in kgdb.
> 
> Was that because with recent enough tools and config options there was
> enough annotations so GDB could finally figure out where things had
> stopped?  Thanks.

The reason Linus said he didn't allow George's kgdb mm patch to 
be integrating into the kernel a year or two ago was that Amit and
George had significantly different implementations. So Amit, Tom, 
George, and the rest of the kgdb development gang worked together 
and came up with a unified version that we now support on SourceForge. 

Tom rolled up a mm patch back in December for Andrew and then the
integration process stopped. I suggest we work together on getting
the kgdb patch back into the mm series and permanently into the kernel
like the kexec code and then we can avoid this kernel development
obfuscation.

-piet

 
> 
-- 
Piet Delaney                                    Phone: (408) 200-5256
Blue Lane Technologies                          Fax:   (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014                            Email: piet@bluelane.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Randy Dunlap]

On Thu, 08 Mar 2007 14:24:10 -0800 Piet Delaney wrote:

> On Thu, 2007-03-08 at 11:49 -0700, Tom Rini wrote:
> > On Thu, Mar 08, 2007 at 07:37:56PM +0100, Andi Kleen wrote:
> > > On Thursday 08 March 2007 18:44, Dave Jiang wrote:
> > > 
> > > > In spite of kgdb, shouldn't it have that \n anyways in case some other code
> > > > gets added in the future after the macro? Or are you saying that there should
> > > > never be any code ever after that macro?
> > > 
> > > Sure if there is mainline code added after that macro we add the \n.
> > > But only if it makes sense to add code there, which it didn't in kgdb.
> > 
> > Was that because with recent enough tools and config options there was
> > enough annotations so GDB could finally figure out where things had
> > stopped?  Thanks.
> 
> The reason Linus said he didn't allow George's kgdb mm patch to 
> be integrating into the kernel a year or two ago was that Amit and
> George had significantly different implementations. So Amit, Tom, 
> George, and the rest of the kgdb development gang worked together 
> and came up with a unified version that we now support on SourceForge. 
> 
> Tom rolled up a mm patch back in December for Andrew and then the
> integration process stopped. I suggest we work together on getting
> the kgdb patch back into the mm series and permanently into the kernel
> like the kexec code and then we can avoid this kernel development
> obfuscation.

Hi,
Is there any movement on this?

Thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Sergei Shtylyov]

Hello.

Randy Dunlap wrote:

>>>>>In spite of kgdb, shouldn't it have that \n anyways in case some other code
>>>>>gets added in the future after the macro? Or are you saying that there should
>>>>>never be any code ever after that macro?
>>>>
>>>>Sure if there is mainline code added after that macro we add the \n.
>>>>But only if it makes sense to add code there, which it didn't in kgdb.

>>>Was that because with recent enough tools and config options there was
>>>enough annotations so GDB could finally figure out where things had
>>>stopped?  Thanks.
>>
>>The reason Linus said he didn't allow George's kgdb mm patch to 
>>be integrating into the kernel a year or two ago was that Amit and
>>George had significantly different implementations. So Amit, Tom, 
>>George, and the rest of the kgdb development gang worked together 
>>and came up with a unified version that we now support on SourceForge. 

>>Tom rolled up a mm patch back in December for Andrew and then the
>>integration process stopped. I suggest we work together on getting
>>the kgdb patch back into the mm series and permanently into the kernel
>>like the kexec code and then we can avoid this kernel development
>>obfuscation.
 
> Hi,
> Is there any movement on this?

   Jason Wessel has taken up KGDB maintenance for upstream. We're now working on merging the several diverse trees together.

WBR, Sergei

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Randy Dunlap]

Sergei Shtylyov wrote:
> Hello.
> 
> Randy Dunlap wrote:
> 
>>>>>> In spite of kgdb, shouldn't it have that \n anyways in case some 
>>>>>> other code
>>>>>> gets added in the future after the macro? Or are you saying that 
>>>>>> there should
>>>>>> never be any code ever after that macro?
>>>>>
>>>>> Sure if there is mainline code added after that macro we add the \n.
>>>>> But only if it makes sense to add code there, which it didn't in kgdb.
> 
>>>> Was that because with recent enough tools and config options there was
>>>> enough annotations so GDB could finally figure out where things had
>>>> stopped?  Thanks.
>>>
>>> The reason Linus said he didn't allow George's kgdb mm patch to be 
>>> integrating into the kernel a year or two ago was that Amit and
>>> George had significantly different implementations. So Amit, Tom, 
>>> George, and the rest of the kgdb development gang worked together and 
>>> came up with a unified version that we now support on SourceForge. 
> 
>>> Tom rolled up a mm patch back in December for Andrew and then the
>>> integration process stopped. I suggest we work together on getting
>>> the kgdb patch back into the mm series and permanently into the kernel
>>> like the kexec code and then we can avoid this kernel development
>>> obfuscation.
> 
>> Hi,
>> Is there any movement on this?
> 
>   Jason Wessel has taken up KGDB maintenance for upstream. We're now 
> working on merging the several diverse trees together.
> 
> WBR, Sergei

at kgdb.sf.net or where is the work being done?

Thanks,
-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Sergei Shtylyov]

Randy Dunlap wrote:

>>>>>>> In spite of kgdb, shouldn't it have that \n anyways in case some 
>>>>>>> other code
>>>>>>> gets added in the future after the macro? Or are you saying that 
>>>>>>> there should
>>>>>>> never be any code ever after that macro?

>>>>>> Sure if there is mainline code added after that macro we add the \n.
>>>>>> But only if it makes sense to add code there, which it didn't in 
>>>>>> kgdb.

>>>>> Was that because with recent enough tools and config options there was
>>>>> enough annotations so GDB could finally figure out where things had
>>>>> stopped?  Thanks.

>>>> The reason Linus said he didn't allow George's kgdb mm patch to be 
>>>> integrating into the kernel a year or two ago was that Amit and
>>>> George had significantly different implementations. So Amit, Tom, 
>>>> George, and the rest of the kgdb development gang worked together 
>>>> and came up with a unified version that we now support on SourceForge. 

>>>> Tom rolled up a mm patch back in December for Andrew and then the
>>>> integration process stopped. I suggest we work together on getting
>>>> the kgdb patch back into the mm series and permanently into the kernel
>>>> like the kexec code and then we can avoid this kernel development
>>>> obfuscation.

>>> Hi,
>>> Is there any movement on this?

>>   Jason Wessel has taken up KGDB maintenance for upstream. We're now 
>> working on merging the several diverse trees together.

> at kgdb.sf.net or where is the work being done?

   Currently, 2.6.21 branch is in SF CVS but unfortunately it's not yet in sync with (abandoned) Tom Rini's trees.

> Thanks,

WBR, Sergei

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Andi Kleen]

> Is there any movement on this?

I'm open to reasonable patches for the hooks at least. If that is done
then the actual kgdb code can be reviewed and considered eventually too. 

But just having the hooks in would make it easy enough to use anyways 
(no patching, just dropping in of new files, or even loading of it as a
module into any kernel)

When I did the original x86-64 kgdb port this worked nicely --
kgdb could work with just the standard die notifiers and a simple
change in the serial console code.

The recent kgdb seems to need much more changes again though.

However every time when I suggested this (fixing the hooks first
and submitting the really needed changes piece by piece)
there didn't seem to be any interest from the various kgdb maintainers.

So my impression currently is that they're not interested in merging.

Another problem is that kgdb is moving more and more away from
mainline by adding various weird hacks and workarounds in random
code that just make  merging harder.

Before anything could be considered for merging that all would
need to be cleaned up.

-Andi

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Robin Holt]

On Tue, Apr 17, 2007 at 08:45:03PM +0200, Andi Kleen wrote:
> 
> > Is there any movement on this?
> 
> I'm open to reasonable patches for the hooks at least. If that is done
> then the actual kgdb code can be reviewed and considered eventually too. 

Would you be open to adding to that set of hooks the ones needed to get
KDB working as a loadable module?  Keith, would that be possible?

Thanks,
Robin

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Piet Delaney]

[-- Attachment #1: Type: text/plain, Size: 4213 bytes --]

On Tue, 2007-04-17 at 11:30 -0700, Randy Dunlap wrote:
> On Thu, 08 Mar 2007 14:24:10 -0800 Piet Delaney wrote:
> 
> > On Thu, 2007-03-08 at 11:49 -0700, Tom Rini wrote:
> > > On Thu, Mar 08, 2007 at 07:37:56PM +0100, Andi Kleen wrote:
> > > > On Thursday 08 March 2007 18:44, Dave Jiang wrote:
> > > > 
> > > > > In spite of kgdb, shouldn't it have that \n anyways in case some other code
> > > > > gets added in the future after the macro? Or are you saying that there should
> > > > > never be any code ever after that macro?
> > > > 
> > > > Sure if there is mainline code added after that macro we add the \n.
> > > > But only if it makes sense to add code there, which it didn't in kgdb.
> > > 
> > > Was that because with recent enough tools and config options there was
> > > enough annotations so GDB could finally figure out where things had
> > > stopped?  Thanks.
> > 
> > The reason Linus said he didn't allow George's kgdb mm patch to 
> > be integrating into the kernel a year or two ago was that Amit and
> > George had significantly different implementations. So Amit, Tom, 
> > George, and the rest of the kgdb development gang worked together 
> > and came up with a unified version that we now support on SourceForge. 
> > 
> > Tom rolled up a mm patch back in December for Andrew and then the
> > integration process stopped. I suggest we work together on getting
> > the kgdb patch back into the mm series and permanently into the kernel
> > like the kexec code and then we can avoid this kernel development
> > obfuscation.
> 
> Hi,
> Is there any movement on this?

Hi Randy:

Jason Wessel <jason.wessel@windriver.com> is currently leading yet
another attempt at getting kgdb permanently into the kernel. Jason
has a linux2_6_21 patch on SourceForge:
	
http://kgdb.cvs.sourceforge.net/kgdb/kgdb-2/8250.patch?view=log&pathrev=linux2_6_21_uprev

and has been working with Sergei Shtylyov <sshtylyov@ru.mvista.com>
recently on getting KGDBOE Netpoll patches that got lost around the 
time of Tom's attempt. Just on Monday there were a dozen posting to 
the source forge mailing list:

	

kgdb-bugreport@lists.sourceforge.net

on this effort.

I'd like to see a git repository on kernel.org that is used to update
the mainstream kernel. Unfortunately getting accounts on kernel.org is
next to impossible. If Jason doesn't have one yet it would be nice to
offer him one for the kgdb developers to use. 

I agree with Andi that the kgdb code seems to be getting more
complicated that needed thought I don't find the hooks offensive.
Here I keep my kgdb hooks completely under #ifdef KGDB, so there
is absolutely no difference to the kernel when KGDB isn't configured.
I also like having debug printks, similar to the SOCK_DEBUG() macros,
to make it easy to watch kgdb internals in action. Ya can't run kgdb
on itself. 

I find these blemish's a minor concern compared to the damage/lost
of not integrating kgdb into the kernel permanently. When developers
can't rely on using kgdb for easy development they tend to write code
without consideration for what it's like using their code with the
debugger. Linux is making a major headway into $100 embedded systems;
the recent use in the Linksys WRT54GL (DD-WRT) and Engenius EOC-3220
for example. Making kgdb easily accessible will make the viability of
using Linux for embedded system greatly increased, IMHO. 

Perhaps with a bit of support from the kernel.org folks we could get
the kgdb patch, with all of it's blemishes, into Andrews 2.6.21-rc7-mm1
patch. Accounts on kernel.org for kgdb developers would be a modest
effort. I find the CVS patch framework rather clumsy and would rather
follow the KISS principle and just use git repositories like the rest
of the kernel developers appear to be using.

-piet

> 
> Thanks,
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
-- 
Piet Delaney                                    Phone: (408) 200-5256
Blue Lane Technologies                          Fax:   (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014                            Email: piet@bluelane.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Andrew Morton]

On Fri, 20 Apr 2007 15:51:35 -0700
Piet Delaney <pdelaney@bluelane.com> wrote:

> > Is there any movement on this?
> 
> Hi Randy:
> 
> Jason Wessel <jason.wessel@windriver.com> is currently leading yet
> another attempt at getting kgdb permanently into the kernel. Jason
> has a linux2_6_21 patch on SourceForge:
> 	
> http://kgdb.cvs.sourceforge.net/kgdb/kgdb-2/8250.patch?view=log&pathrev=linux2_6_21_uprev
> 
> and has been working with Sergei Shtylyov <sshtylyov@ru.mvista.com>
> recently on getting KGDBOE Netpoll patches that got lost around the 
> time of Tom's attempt. Just on Monday there were a dozen posting to 
> the source forge mailing list:
> 
> 	
> 
> kgdb-bugreport@lists.sourceforge.net
> 
> on this effort.

Unfortunately there doesn't seem to be anything there which I can autopull
into the -mm tree.  I'm presently set up for quilt trees in open
directories and for git trees.  I could add plain-old-gzipped-diffs or
whatever.

> I'd like to see a git repository on kernel.org that is used to update
> the mainstream kernel. Unfortunately getting accounts on kernel.org is
> next to impossible. If Jason doesn't have one yet it would be nice to
> offer him one for the kgdb developers to use. 

This seems to be to do with some silly spamfilter issue or something.  I
sent an email off-list.

> I agree with Andi that the kgdb code seems to be getting more
> complicated that needed thought I don't find the hooks offensive.
> Here I keep my kgdb hooks completely under #ifdef KGDB, so there
> is absolutely no difference to the kernel when KGDB isn't configured.

We can address that sort of thing via review: you send send the diffs out,
we read them and comment on them.  We do this 100 times a day - it is
simply a non-issue, as long as there's actually someone who has the
time/effort/inclination to push this feature to completion, which is what
kgdb has sadly lacked for the past decade or so.

> I also like having debug printks, similar to the SOCK_DEBUG() macros,
> to make it easy to watch kgdb internals in action. Ya can't run kgdb
> on itself. 
> 
> I find these blemish's a minor concern compared to the damage/lost
> of not integrating kgdb into the kernel permanently. When developers
> can't rely on using kgdb for easy development they tend to write code
> without consideration for what it's like using their code with the
> debugger. Linux is making a major headway into $100 embedded systems;
> the recent use in the Linksys WRT54GL (DD-WRT) and Engenius EOC-3220
> for example. Making kgdb easily accessible will make the viability of
> using Linux for embedded system greatly increased, IMHO. 

Lots of people want kgdb.  One person is famously less keen on it, but
we'll be able to talk him around, as long as the patches aren't daft.

> Perhaps with a bit of support from the kernel.org folks we could get
> the kgdb patch, with all of it's blemishes, into Andrews 2.6.21-rc7-mm1
> patch. Accounts on kernel.org for kgdb developers would be a modest
> effort. I find the CVS patch framework rather clumsy and would rather
> follow the KISS principle and just use git repositories like the rest
> of the kernel developers appear to be using.

Yes, a git tree on k.org is appropriate - let's make that happen.

But beware that it will need to be updated pretty reguarly, and it'll need
to be against Linux-tree-of-the-day, please.  The x86 code continues to
change at a tremendous rate (I count 204 x86 patches queued for 2.6.22),
and kgdb supports lots of other architectures too.

So whoever is signed up to maintain this will have quite a bit of messy
maintaneance to do during the getting-it-ready phase.  This will of course
be minimised by being VERY careful to mimimise the impact of the patchset
on the existing code.

And if/when it is merged, there will be quite a bit of tricky maintenance
work to do, if my experience of the kgdb stub is anything to go by.

There inevitably will be long-term low-level impact upon the arch
maintainers too.  But that's OK, because the way to merge this feature is
to put the arch-neutral core into the tree first, and to then send each
per-arch patch to the relevant arch maintainer for merging.  That way, they
get to decide whether they wish to take on the burden of participating in
its long-term maintenance.

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Andi Kleen]

> Lots of people want kgdb.  One person is famously less keen on it, but
> we'll be able to talk him around, as long as the patches aren't daft.

The big question is if the kgdb developers seriously want mainline.
At least in the past this definitely wasn't the case.

If they're not open to change requests from mainline reviewers we don't
even need to bother to start the whole exercise.

Just putting their stuff onto korg isn't enough.

-Andi

Re: Permanent Kgdb integration into the kernel - lets get with it. (Dave: How do FreeBSD folks maintain the KGDB stub?)

[Piet Delaney]

[-- Attachment #1: Type: text/plain, Size: 3738 bytes --]

On Sat, 2007-04-21 at 11:48 +0200, Andi Kleen wrote:
> > Lots of people want kgdb.  One person is famously less keen on it, but
> > we'll be able to talk him around, as long as the patches aren't daft.
> 
> The big question is if the kgdb developers seriously want mainline.
> At least in the past this definitely wasn't the case.

I haven't seen any email from kgdb developers saying they didn't want
kgdb to be part of mainline. 

Happen to have any e-mail demonstrating that? 

It's appears to me that:

	1. Jason Wessel is putting a lot of effort at that right now.

        2. Tom Rini worked hard at this just a few months ago.

        3. George Anzinger was working hard at this a year or two
           with the mm series and as likely disappointed when it wasn't
           put into the mainline. As I recall the reason Linus gave
           was that there were two competing patches and he wanted that
           be resolved before integrating it into the mainline. So
           George worked with Amit at SourceForge over that past year
           or two and it's now integrated.

> 
> If they're not open to change requests from mainline reviewers we don't
> even need to bother to start the whole exercise.

What issue are there of have been that your referring to?

Once KGDB is part of KORG can't it's maintenance and support be
a kernel wide responsibility. If someone breaks kgdb shouldn't
that be backed out until the KORG developers fixes the problem?
Centralizing the responsibility for KGDB seems like mistake. I 
doubt the FreeBSD folks rip out the KGDB support of a kernel hacker
breaks KGDB and then leaves a group of KGDB developers to sort out
the problem. Seems it should be cough as a mm patch with Andrew tossing
out the patch if it breaks KGDB. Kgdb developers could try to give
Andrew a heads up if this occurs and he didn't notice it.

Once KGDB is integrated the maintenance should be minimal and changes
that break KGDB are likely best addressed by the developer that just
broke it. At least that what I'd think is an optimal approach. Perhaps
Dave O'Brien could tell us how the FreeBSD folks take care for KGDB.

> 
> Just putting their stuff onto korg isn't enough.

Yep, and once it's integrated into korg it should finally become
a permanent part of the kernel and I suspect maintained by all
kernel developers. New KGDB features could be developed at SourceForge
but maintaining kernel coherence seems like a global responsibility.
Like running fault injection on your code before checking it in.

Maybe I'm totally out to lunch on this; perhaps Dave O'Brien
can straighten me our if I'm wrong or the Linux kernel core
responsibility paradigm are incompatible with this.

I'd prefer Linux being just as good as NetBSD with Debugging support; 
current presentations like:

	http://foss.in/2005/slides/netbsd-linux.pdf

show our current support as being much worse. Let's fix it.

You developed a kgdb proxy for Keith Owens kdb and I suspect you
would like to have KGDB being part of the kernel mainline as
long as it's done well. I doubt anyone would argue with that.
 
Perhaps it's possible to eventually setup KGDB so it can be 
debugged with kdb. Once KGDB is mainline that are plenty of
issues that can be addressed; for example taking a kernel
core dump after dropping into kgdb and having the registers
show up correctly in Dave Anderson's crash utility.

-piet

> 
> -Andi
-- 
Piet Delaney                                    Phone: (408) 200-5256
Blue Lane Technologies                          Fax:   (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014                            Email: piet@bluelane.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] x86_64 RESTORE_CONTEXT missing '\n'

[Andi Kleen]

On Thursday 08 March 2007 19:49, Tom Rini wrote:
> On Thu, Mar 08, 2007 at 07:37:56PM +0100, Andi Kleen wrote:
> > On Thursday 08 March 2007 18:44, Dave Jiang wrote:
> > 
> > > In spite of kgdb, shouldn't it have that \n anyways in case some other code
> > > gets added in the future after the macro? Or are you saying that there should
> > > never be any code ever after that macro?
> > 
> > Sure if there is mainline code added after that macro we add the \n.
> > But only if it makes sense to add code there, which it didn't in kgdb.
> 
> Was that because with recent enough tools and config options there was
> enough annotations so GDB could finally figure out where things had
> stopped?  

The x86 kernels are fully dwarf2 annotiated yes.

-Andi

[PATCH 6/13] maps#2: Move the page walker code to lib/

[Matt Mackall]

Move the page walker code to lib/

This lets it get shared outside of proc/ and linked in only when
needed.

Signed-off-by: Matt Mackall <mpm@selenic.com>

Index: mm/fs/proc/task_mmu.c
===================================================================
--- mm.orig/fs/proc/task_mmu.c	2007-03-27 22:13:43.000000000 -0500
+++ mm/fs/proc/task_mmu.c	2007-03-27 22:13:51.000000000 -0500
@@ -280,123 +280,6 @@ static int clear_refs_pte_range(pmd_t *p
 	return 0;
 }
 
-struct mm_walk {
-	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
-	int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
-	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
-	int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
-};
-
-static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
-			  struct mm_walk *walk, void *private)
-{
-	pte_t *pte;
-	int err;
-
-	for (pte = pte_offset_map(pmd, addr); addr != end;
-	     addr += PAGE_SIZE, pte++) {
-		if (pte_none(*pte))
-			continue;
-		err = walk->pte_entry(pte, addr, addr, private);
-		if (err) {
-			pte_unmap(pte);
-			return err;
-		}
-	}
-	pte_unmap(pte);
-	return 0;
-}
-
-static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
-			  struct mm_walk *walk, void *private)
-{
-	pmd_t *pmd;
-	unsigned long next;
-	int err;
-
-	for (pmd = pmd_offset(pud, addr); addr != end;
-	     pmd++, addr = next) {
-		next = pmd_addr_end(addr, end);
-		if (pmd_none_or_clear_bad(pmd))
-			continue;
-		if (walk->pmd_entry) {
-			err = walk->pmd_entry(pmd, addr, next, private);
-			if (err)
-				return err;
-		}
-		if (walk->pte_entry) {
-			err = walk_pte_range(pmd, addr, next, walk, private);
-			if (err)
-				return err;
-		}
-	}
-	return 0;
-}
-
-static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
-			  struct mm_walk *walk, void *private)
-{
-	pud_t *pud;
-	unsigned long next;
-	int err;
-
-	for (pud = pud_offset(pgd, addr); addr != end;
-	     pud++, addr = next) {
-		next = pud_addr_end(addr, end);
-		if (pud_none_or_clear_bad(pud))
-			continue;
-		if (walk->pud_entry) {
-			err = walk->pud_entry(pud, addr, next, private);
-			if (err)
-				return err;
-		}
-		if (walk->pmd_entry || walk->pte_entry) {
-			err = walk_pmd_range(pud, addr, next, walk, private);
-			if (err)
-				return err;
-		}
-	}
-	return 0;
-}
-
-/*
- * walk_page_range - walk a memory map's page tables with a callback
- * @mm - memory map to walk
- * @addr - starting address
- * @end - ending address
- * @action - callback invoked for every bottom-level (PTE) page table
- * @private - private data passed to the callback function
- *
- * Recursively walk the page table for the memory area in a VMA, calling
- * a callback for every bottom-level (PTE) page table.
- */
-static int walk_page_range(struct mm_struct *mm,
-			   unsigned long addr, unsigned long end,
-			   struct mm_walk *walk, void *private)
-{
-	pgd_t *pgd;
-	unsigned long next;
-	int err;
-
-	for (pgd = pgd_offset(mm, addr); addr != end;
-	     pgd++, addr = next) {
-		next = pgd_addr_end(addr, end);
-		if (pgd_none_or_clear_bad(pgd))
-			continue;
-		if (walk->pgd_entry) {
-			err = walk->pgd_entry(pgd, addr, next, private);
-			if (err)
-				return err;
-		}
-		if (walk->pud_entry || walk->pmd_entry || walk->pte_entry) {
-			err = walk_pud_range(pgd, addr, next, walk, private);
-			if (err)
-				return err;
-		}
-	}
-	return 0;
-}
-
 static struct mm_walk smaps_walk = { .pmd_entry = smaps_pte_range };
 
 static int show_smap(struct seq_file *m, void *v)
Index: mm/include/linux/mm.h
===================================================================
--- mm.orig/include/linux/mm.h	2007-03-27 22:13:42.000000000 -0500
+++ mm/include/linux/mm.h	2007-03-27 22:13:51.000000000 -0500
@@ -747,6 +747,16 @@ unsigned long unmap_vmas(struct mmu_gath
 		struct vm_area_struct *start_vma, unsigned long start_addr,
 		unsigned long end_addr, unsigned long *nr_accounted,
 		struct zap_details *);
+
+struct mm_walk {
+	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
+	int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
+	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
+	int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
+};
+
+int walk_page_range(struct mm_struct *, unsigned long addr, unsigned long end,
+		    struct mm_walk *walk, void *private);
 void free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
 		unsigned long end, unsigned long floor, unsigned long ceiling);
 void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *start_vma,
Index: mm/lib/Makefile
===================================================================
--- mm.orig/lib/Makefile	2007-03-27 22:14:09.000000000 -0500
+++ mm/lib/Makefile	2007-03-27 22:16:49.000000000 -0500
@@ -7,7 +7,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
 	 idr.o div64.o int_sqrt.o bitmap.o extable.o prio_tree.o \
 	 sha1.o irq_regs.o reciprocal_div.o
 
-lib-$(CONFIG_MMU) += ioremap.o
+lib-$(CONFIG_MMU) += ioremap.o pagewalk.o
 lib-$(CONFIG_SMP) += cpumask.o
 
 lib-y	+= kobject.o kref.o kobject_uevent.o klist.o
Index: mm/lib/pagewalk.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ mm/lib/pagewalk.c	2007-03-27 22:18:37.000000000 -0500
@@ -0,0 +1,111 @@
+#include <linux/mm.h>
+
+static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
+			  struct mm_walk *walk, void *private)
+{
+	pte_t *pte;
+	int err;
+
+	for (pte = pte_offset_map(pmd, addr); addr != end;
+	     addr += PAGE_SIZE, pte++) {
+		if (pte_none(*pte))
+			continue;
+		err = walk->pte_entry(pte, addr, addr, private);
+		if (err) {
+			pte_unmap(pte);
+			return err;
+		}
+	}
+	pte_unmap(pte);
+	return 0;
+}
+
+static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
+			  struct mm_walk *walk, void *private)
+{
+	pmd_t *pmd;
+	unsigned long next;
+	int err;
+
+	for (pmd = pmd_offset(pud, addr); addr != end;
+	     pmd++, addr = next) {
+		next = pmd_addr_end(addr, end);
+		if (pmd_none_or_clear_bad(pmd))
+			continue;
+		if (walk->pmd_entry) {
+			err = walk->pmd_entry(pmd, addr, next, private);
+			if (err)
+				return err;
+		}
+		if (walk->pte_entry) {
+			err = walk_pte_range(pmd, addr, next, walk, private);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
+static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
+			  struct mm_walk *walk, void *private)
+{
+	pud_t *pud;
+	unsigned long next;
+	int err;
+
+	for (pud = pud_offset(pgd, addr); addr != end;
+	     pud++, addr = next) {
+		next = pud_addr_end(addr, end);
+		if (pud_none_or_clear_bad(pud))
+			continue;
+		if (walk->pud_entry) {
+			err = walk->pud_entry(pud, addr, next, private);
+			if (err)
+				return err;
+		}
+		if (walk->pmd_entry || walk->pte_entry) {
+			err = walk_pmd_range(pud, addr, next, walk, private);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
+/*
+ * walk_page_range - walk a memory map's page tables with a callback
+ * @mm - memory map to walk
+ * @addr - starting address
+ * @end - ending address
+ * @walk - set of callbacks to invoke for each level of the tree
+ * @private - private data passed to the callback function
+ *
+ * Recursively walk the page table for the memory area in a VMA, calling
+ * a callback for every bottom-level (PTE) page table.
+ */
+int walk_page_range(struct mm_struct *mm,
+		    unsigned long addr, unsigned long end,
+		    struct mm_walk *walk, void *private)
+{
+	pgd_t *pgd;
+	unsigned long next;
+	int err;
+
+	for (pgd = pgd_offset(mm, addr); addr != end;
+	     pgd++, addr = next) {
+		next = pgd_addr_end(addr, end);
+		if (pgd_none_or_clear_bad(pgd))
+			continue;
+		if (walk->pgd_entry) {
+			err = walk->pgd_entry(pgd, addr, next, private);
+			if (err)
+				return err;
+		}
+		if (walk->pud_entry || walk->pmd_entry || walk->pte_entry) {
+			err = walk_pud_range(pgd, addr, next, walk, private);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}

Re: [PATCH 6/13] maps#2: Move the page walker code to lib/

[Nick Piggin]

Matt Mackall wrote:
> Move the page walker code to lib/
> 
> This lets it get shared outside of proc/ and linked in only when
> needed.

Still should go into mm/

If it had, you might have also noticed your pagetable walking code
is completely different from how everyone else does it, and fixed
that too.

BTW. Is it the case that unused and unexported symbols don't get
pruned by the linker except inside lib/?

> 
> Signed-off-by: Matt Mackall <mpm@selenic.com>
> 
> Index: mm/fs/proc/task_mmu.c
> ===================================================================
> --- mm.orig/fs/proc/task_mmu.c	2007-03-27 22:13:43.000000000 -0500
> +++ mm/fs/proc/task_mmu.c	2007-03-27 22:13:51.000000000 -0500
> @@ -280,123 +280,6 @@ static int clear_refs_pte_range(pmd_t *p
>  	return 0;
>  }
>  
> -struct mm_walk {
> -	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
> -	int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
> -	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
> -	int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
> -};
> -
> -static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> -			  struct mm_walk *walk, void *private)
> -{
> -	pte_t *pte;
> -	int err;
> -
> -	for (pte = pte_offset_map(pmd, addr); addr != end;
> -	     addr += PAGE_SIZE, pte++) {
> -		if (pte_none(*pte))
> -			continue;
> -		err = walk->pte_entry(pte, addr, addr, private);
> -		if (err) {
> -			pte_unmap(pte);
> -			return err;
> -		}
> -	}
> -	pte_unmap(pte);
> -	return 0;
> -}
> -
> -static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
> -			  struct mm_walk *walk, void *private)
> -{
> -	pmd_t *pmd;
> -	unsigned long next;
> -	int err;
> -
> -	for (pmd = pmd_offset(pud, addr); addr != end;
> -	     pmd++, addr = next) {
> -		next = pmd_addr_end(addr, end);
> -		if (pmd_none_or_clear_bad(pmd))
> -			continue;
> -		if (walk->pmd_entry) {
> -			err = walk->pmd_entry(pmd, addr, next, private);
> -			if (err)
> -				return err;
> -		}
> -		if (walk->pte_entry) {
> -			err = walk_pte_range(pmd, addr, next, walk, private);
> -			if (err)
> -				return err;
> -		}
> -	}
> -	return 0;
> -}
> -
> -static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
> -			  struct mm_walk *walk, void *private)
> -{
> -	pud_t *pud;
> -	unsigned long next;
> -	int err;
> -
> -	for (pud = pud_offset(pgd, addr); addr != end;
> -	     pud++, addr = next) {
> -		next = pud_addr_end(addr, end);
> -		if (pud_none_or_clear_bad(pud))
> -			continue;
> -		if (walk->pud_entry) {
> -			err = walk->pud_entry(pud, addr, next, private);
> -			if (err)
> -				return err;
> -		}
> -		if (walk->pmd_entry || walk->pte_entry) {
> -			err = walk_pmd_range(pud, addr, next, walk, private);
> -			if (err)
> -				return err;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * walk_page_range - walk a memory map's page tables with a callback
> - * @mm - memory map to walk
> - * @addr - starting address
> - * @end - ending address
> - * @action - callback invoked for every bottom-level (PTE) page table
> - * @private - private data passed to the callback function
> - *
> - * Recursively walk the page table for the memory area in a VMA, calling
> - * a callback for every bottom-level (PTE) page table.
> - */
> -static int walk_page_range(struct mm_struct *mm,
> -			   unsigned long addr, unsigned long end,
> -			   struct mm_walk *walk, void *private)
> -{
> -	pgd_t *pgd;
> -	unsigned long next;
> -	int err;
> -
> -	for (pgd = pgd_offset(mm, addr); addr != end;
> -	     pgd++, addr = next) {
> -		next = pgd_addr_end(addr, end);
> -		if (pgd_none_or_clear_bad(pgd))
> -			continue;
> -		if (walk->pgd_entry) {
> -			err = walk->pgd_entry(pgd, addr, next, private);
> -			if (err)
> -				return err;
> -		}
> -		if (walk->pud_entry || walk->pmd_entry || walk->pte_entry) {
> -			err = walk_pud_range(pgd, addr, next, walk, private);
> -			if (err)
> -				return err;
> -		}
> -	}
> -	return 0;
> -}
> -
>  static struct mm_walk smaps_walk = { .pmd_entry = smaps_pte_range };
>  
>  static int show_smap(struct seq_file *m, void *v)
> Index: mm/include/linux/mm.h
> ===================================================================
> --- mm.orig/include/linux/mm.h	2007-03-27 22:13:42.000000000 -0500
> +++ mm/include/linux/mm.h	2007-03-27 22:13:51.000000000 -0500
> @@ -747,6 +747,16 @@ unsigned long unmap_vmas(struct mmu_gath
>  		struct vm_area_struct *start_vma, unsigned long start_addr,
>  		unsigned long end_addr, unsigned long *nr_accounted,
>  		struct zap_details *);
> +
> +struct mm_walk {
> +	int (*pgd_entry)(pgd_t *, unsigned long, unsigned long, void *);
> +	int (*pud_entry)(pud_t *, unsigned long, unsigned long, void *);
> +	int (*pmd_entry)(pmd_t *, unsigned long, unsigned long, void *);
> +	int (*pte_entry)(pte_t *, unsigned long, unsigned long, void *);
> +};
> +
> +int walk_page_range(struct mm_struct *, unsigned long addr, unsigned long end,
> +		    struct mm_walk *walk, void *private);
>  void free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
>  		unsigned long end, unsigned long floor, unsigned long ceiling);
>  void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *start_vma,
> Index: mm/lib/Makefile
> ===================================================================
> --- mm.orig/lib/Makefile	2007-03-27 22:14:09.000000000 -0500
> +++ mm/lib/Makefile	2007-03-27 22:16:49.000000000 -0500
> @@ -7,7 +7,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
>  	 idr.o div64.o int_sqrt.o bitmap.o extable.o prio_tree.o \
>  	 sha1.o irq_regs.o reciprocal_div.o
>  
> -lib-$(CONFIG_MMU) += ioremap.o
> +lib-$(CONFIG_MMU) += ioremap.o pagewalk.o
>  lib-$(CONFIG_SMP) += cpumask.o
>  
>  lib-y	+= kobject.o kref.o kobject_uevent.o klist.o
> Index: mm/lib/pagewalk.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ mm/lib/pagewalk.c	2007-03-27 22:18:37.000000000 -0500
> @@ -0,0 +1,111 @@
> +#include <linux/mm.h>
> +
> +static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> +			  struct mm_walk *walk, void *private)
> +{
> +	pte_t *pte;
> +	int err;
> +
> +	for (pte = pte_offset_map(pmd, addr); addr != end;
> +	     addr += PAGE_SIZE, pte++) {
> +		if (pte_none(*pte))
> +			continue;
> +		err = walk->pte_entry(pte, addr, addr, private);
> +		if (err) {
> +			pte_unmap(pte);
> +			return err;
> +		}
> +	}
> +	pte_unmap(pte);
> +	return 0;
> +}
> +
> +static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
> +			  struct mm_walk *walk, void *private)
> +{
> +	pmd_t *pmd;
> +	unsigned long next;
> +	int err;
> +
> +	for (pmd = pmd_offset(pud, addr); addr != end;
> +	     pmd++, addr = next) {
> +		next = pmd_addr_end(addr, end);
> +		if (pmd_none_or_clear_bad(pmd))
> +			continue;
> +		if (walk->pmd_entry) {
> +			err = walk->pmd_entry(pmd, addr, next, private);
> +			if (err)
> +				return err;
> +		}
> +		if (walk->pte_entry) {
> +			err = walk_pte_range(pmd, addr, next, walk, private);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int walk_pud_range(pgd_t *pgd, unsigned long addr, unsigned long end,
> +			  struct mm_walk *walk, void *private)
> +{
> +	pud_t *pud;
> +	unsigned long next;
> +	int err;
> +
> +	for (pud = pud_offset(pgd, addr); addr != end;
> +	     pud++, addr = next) {
> +		next = pud_addr_end(addr, end);
> +		if (pud_none_or_clear_bad(pud))
> +			continue;
> +		if (walk->pud_entry) {
> +			err = walk->pud_entry(pud, addr, next, private);
> +			if (err)
> +				return err;
> +		}
> +		if (walk->pmd_entry || walk->pte_entry) {
> +			err = walk_pmd_range(pud, addr, next, walk, private);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/*
> + * walk_page_range - walk a memory map's page tables with a callback
> + * @mm - memory map to walk
> + * @addr - starting address
> + * @end - ending address
> + * @walk - set of callbacks to invoke for each level of the tree
> + * @private - private data passed to the callback function
> + *
> + * Recursively walk the page table for the memory area in a VMA, calling
> + * a callback for every bottom-level (PTE) page table.
> + */
> +int walk_page_range(struct mm_struct *mm,
> +		    unsigned long addr, unsigned long end,
> +		    struct mm_walk *walk, void *private)
> +{
> +	pgd_t *pgd;
> +	unsigned long next;
> +	int err;
> +
> +	for (pgd = pgd_offset(mm, addr); addr != end;
> +	     pgd++, addr = next) {
> +		next = pgd_addr_end(addr, end);
> +		if (pgd_none_or_clear_bad(pgd))
> +			continue;
> +		if (walk->pgd_entry) {
> +			err = walk->pgd_entry(pgd, addr, next, private);
> +			if (err)
> +				return err;
> +		}
> +		if (walk->pud_entry || walk->pmd_entry || walk->pte_entry) {
> +			err = walk_pud_range(pgd, addr, next, walk, private);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
SUSE Labs, Novell Inc.

Re: [PATCH 6/13] maps#2: Move the page walker code to lib/

[Matt Mackall]

On Wed, Apr 11, 2007 at 04:35:44PM +1000, Nick Piggin wrote:
> Matt Mackall wrote:
> >Move the page walker code to lib/
> >
> >This lets it get shared outside of proc/ and linked in only when
> >needed.
> 
> Still should go into mm/
> 
> If it had, you might have also noticed your pagetable walking code
> is completely different from how everyone else does it, and fixed
> that too.

I'll try to fix this up next week.

-- 
Mathematics is the supreme nostalgia of our time.

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Jason Wessel]

Andi Kleen wrote:
> > Is there any movement on this?
>
> I'm open to reasonable patches for the hooks at least. If that is done
> then the actual kgdb code can be reviewed and considered eventually too. 
>
> But just having the hooks in would make it easy enough to use anyways 
> (no patching, just dropping in of new files, or even loading of it as a
> module into any kernel)
>
> When I did the original x86-64 kgdb port this worked nicely --
> kgdb could work with just the standard die notifiers and a simple
> change in the serial console code.
>
> The recent kgdb seems to need much more changes again though.
>
> However every time when I suggested this (fixing the hooks first
> and submitting the really needed changes piece by piece)
> there didn't seem to be any interest from the various kgdb maintainers.
>
> So my impression currently is that they're not interested in merging.
>
> Another problem is that kgdb is moving more and more away from
> mainline by adding various weird hacks and workarounds in random
> code that just make  merging harder.
>
> Before anything could be considered for merging that all would
> need to be cleaned up.
>
> -Andi
>   
Andi,

I too am open to having a API, for KGDB, but it does need more than just 
the trap vectors and the serial driver as hook points. There are a 
number of patches to fix problems randing from the NET_POLL API, NMI 
handling, and saving a bit more information when loading kernel modules.

If you have an API, that you would like to contribute or suggest, I for 
one am interested. I have long thought it would be nice to be able to 
choose between kernel debug tools KDB, KGDB, KEXEC etc... much like you 
can dynamically load I/O modules in KGDB and choose either RS232 or 
Ethernet after the kernel has booted.

At the current time, I am most certainly trying to consolidate the 
source forge KGDB patches, Tom Rini's branch, as well as my own 
development branch and hopefully Sergei's development branch as well. 
Perhaps after the code stream is stable you we can take a further look 
at what it takes to abstract a generic kernel debug interface. In the 
mean time if you have code or skeleton API proposal, I am definitely 
listening. It would be nice to have an in kernel soft single step API as 
an example, which could be leveraged by KGDB and utrace/ptrace, but that 
is a bit forward looking at this point.

Jason.

Re: Permanent Kgdb integration into the kernel - lets get with it.

[Andi Kleen]

> I too am open to having a API, for KGDB, but it does need more than just 
> the trap vectors and the serial driver as hook points. There are a 
> number of patches to fix problems randing from the NET_POLL API, NMI 
> handling, 

You should submit these changes all as you fix them to the relevant
maintainers and mailing lists. There is only limited review capacity
and sending too many may tax it too much. So it's best used spaced
out -- as you do them. Also then you probably still remember why
you actually changed something. The few kgdb that were submitted
ever usually had the problem that they were submitted by someone
else who didn't do the work and couldn't quite remember why they
were actually needed.  Unsurprisingly they often didn't get
merged then.

> If you have an API, that you would like to contribute or suggest, I for 

The main interface are the (somewhat misnamed) die chains. More and more 
architectures are moving to them. 

There probably needs to be another one for serial intercept, but that 
should be very simple.  For network netpoll already exists.

> one am interested. I have long thought it would be nice to be able to 
> choose between kernel debug tools KDB, KGDB, KEXEC etc... much like you 
> can dynamically load I/O modules in KGDB and choose either RS232 or 
> Ethernet after the kernel has booted.

At some point we had a fully modular kdb (modprobe kdb worked) 
and kgdb was nearly there. Unfortunately the changes for kdb were
somewhat ugly. Unfortunately that work never hit the standard
patchkits for kgdb/kdb.

> At the current time, I am most certainly trying to consolidate the 
> source forge KGDB patches, Tom Rini's branch, as well as my own 
> development branch and hopefully Sergei's development branch as well. 
> Perhaps after the code stream is stable you we can take a further look 
> at what it takes to abstract a generic kernel debug interface. In the 

There already is one. I don't see the need for another one.

> mean time if you have code or skeleton API proposal, I am definitely 
> listening. It would be nice to have an in kernel soft single step API as 
> an example, which could be leveraged by KGDB and utrace/ptrace, but that 
> is a bit forward looking at this point.

kprobes already do that fine with the existing interfaces. Also
single stepping is not particularly complicated so i don't see why
you want to abstract it.

The other issue that needed some changes was stopping all CPUs for
debugging. Since some other subsystems (like kexec) have this problem
already i would suggest generalizing and exporting what they have already.

-Andi