Re: kgdb support in vanilla 2.6.2

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

Andrew Morton <akpm@osdl.org> writes:

> need to take a look at such things and really convice ourselves that
> they're worthwhile.  Personally, I'd only be interested in the basic stub.

What I found always extremly ugly in the i386 stub was that it uses
magic globals to talk to the page fault handler. For the x86-64
version I replaced that by just using __get/__put_user in the memory
accesses, which is much cleaner. I would suggest doing that for i386
too.

Also what's also ugly in i386 is that it uses ugly hooks in traps.c/fault.c.
On x86-64 I instead added generic notifiers (see include/asm-x86_64/die.h
and notify_die in arch/x86_64/kernel/traps.c) 
where both kdb and kgdb and possibly dprobes and other debuggers can hook
in without conflicting patches for the same files from everybody.
I would strongly suggest to adopt such a generic framework for i386 too
to clean up the core kernel <-> debugger interaction. As soon as this
frame work is in just dropping the stub is is very clean.

The x86-64 version should be pretty simple to port to i386 if someone
is interested ...

Another issue is that for modern gdb and frame pointer less debugging
with dwarf2 we really need dwarf2 cfi annotation on i386 too. It is
not as ugly as it used to be because newer binutils have much nicer to
use .cfi_* mnemonics to generate the dwarf2 unwind table. x86-64 uses
that now thanks to Jim Houston. It only works with uptodate binutils,
but I guess that's a reasonable requirement. It's a bit intrusive in
entry.S, but not too bad compared to the old way (take a look at
arch/i386/kernel/vsyscall-sysenter.S to see how the old way looks
like) Having the dwarf2 unwind information in the kernel vmlinux is
useful even independent of kgdb for other tools that look at crash
dumps.

-Andi

Re: kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

> > need to take a look at such things and really convice ourselves that
> > they're worthwhile.  Personally, I'd only be interested in the basic stub.
> 
> What I found always extremly ugly in the i386 stub was that it uses
> magic globals to talk to the page fault handler. For the x86-64
> version I replaced that by just using __get/__put_user in the memory
> accesses, which is much cleaner. I would suggest doing that for i386
> too.
> 
> Also what's also ugly in i386 is that it uses ugly hooks in traps.c/fault.c.
> On x86-64 I instead added generic notifiers (see include/asm-x86_64/die.h
> and notify_die in arch/x86_64/kernel/traps.c) 
> where both kdb and kgdb and possibly dprobes and other debuggers can hook
> in without conflicting patches for the same files from everybody.
> I would strongly suggest to adopt such a generic framework for i386 too
> to clean up the core kernel <-> debugger interaction. As soon as this
> frame work is in just dropping the stub is is very clean.
> 
> The x86-64 version should be pretty simple to port to i386 if someone
> is interested ...

Hmm, this tells me that perhaps it is reasonable to get amd64 version
in first, since it received lot of cleaning already...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> Andrew Morton <akpm@osdl.org> writes:
> > need to take a look at such things and really convice ourselves that
> > they're worthwhile.  Personally, I'd only be interested in the basic
> > stub.
>
> What I found always extremly ugly in the i386 stub was that it uses
> magic globals to talk to the page fault handler. For the x86-64
> version I replaced that by just using __get/__put_user in the memory
> accesses, which is much cleaner. I would suggest doing that for i386
> too.

May be I am missing something obvious. When debugging a page fault handler if 
kgdb accesses an swapped-out user page doesn't it deadlock when trying to 
hold mm semaphore?

I use the global to let page fault handler that this access is from kgdb so 
don't try to load any pages, report an error instead.

>
> Also what's also ugly in i386 is that it uses ugly hooks in
> traps.c/fault.c. On x86-64 I instead added generic notifiers (see
> include/asm-x86_64/die.h and notify_die in arch/x86_64/kernel/traps.c)
> where both kdb and kgdb and possibly dprobes and other debuggers can hook
> in without conflicting patches for the same files from everybody.
> I would strongly suggest to adopt such a generic framework for i386 too
> to clean up the core kernel <-> debugger interaction. As soon as this
> frame work is in just dropping the stub is is very clean.

I liked these.  Never got time to integrate them.

>
> The x86-64 version should be pretty simple to port to i386 if someone
> is interested ...
>
> Another issue is that for modern gdb and frame pointer less debugging
> with dwarf2 we really need dwarf2 cfi annotation on i386 too. It is
> not as ugly as it used to be because newer binutils have much nicer to
> use .cfi_* mnemonics to generate the dwarf2 unwind table. x86-64 uses
> that now thanks to Jim Houston. It only works with uptodate binutils,
> but I guess that's a reasonable requirement. It's a bit intrusive in
> entry.S, but not too bad compared to the old way (take a look at
> arch/i386/kernel/vsyscall-sysenter.S to see how the old way looks
> like) Having the dwarf2 unwind information in the kernel vmlinux is
> useful even independent of kgdb for other tools that look at crash
> dumps.

George has coded cfi directives i386 too. He can use them to backtrace past 
irqs stack.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Thu, 5 Feb 2004 23:20:04 +0530
"Amit S. Kale" <amitkale@emsyssoft.com> wrote:

> On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > Andrew Morton <akpm@osdl.org> writes:
> > > need to take a look at such things and really convice ourselves that
> > > they're worthwhile.  Personally, I'd only be interested in the basic
> > > stub.
> >
> > What I found always extremly ugly in the i386 stub was that it uses
> > magic globals to talk to the page fault handler. For the x86-64
> > version I replaced that by just using __get/__put_user in the memory
> > accesses, which is much cleaner. I would suggest doing that for i386
> > too.
> 
> May be I am missing something obvious. When debugging a page fault handler if 
> kgdb accesses an swapped-out user page doesn't it deadlock when trying to 
> hold mm semaphore?

Modern i386 kernels don't grab the mm semaphore when the access is >= TASK_SIZE
and the access came from kernel space (actually I see x86-64 still does, but that's 
a bug, will fix). You could only see a deadlock when using user addresses
and you already hold the mm semaphore for writing (normal read lock is ok). 
Just don't do that. 


> George has coded cfi directives i386 too. He can use them to backtrace past 
> irqs stack.

Problem is that he did it without binutils support. I don't think that's a good
idea because it makes the code basically unmaintainable for normal souls
(it's like writing assembly code directly in hex) 

-Andi

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> On Thu, 5 Feb 2004 23:20:04 +0530
>
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > > Andrew Morton <akpm@osdl.org> writes:
> > > > need to take a look at such things and really convice ourselves that
> > > > they're worthwhile.  Personally, I'd only be interested in the basic
> > > > stub.
> > >
> > > What I found always extremly ugly in the i386 stub was that it uses
> > > magic globals to talk to the page fault handler. For the x86-64
> > > version I replaced that by just using __get/__put_user in the memory
> > > accesses, which is much cleaner. I would suggest doing that for i386
> > > too.
> >
> > May be I am missing something obvious. When debugging a page fault
> > handler if kgdb accesses an swapped-out user page doesn't it deadlock
> > when trying to hold mm semaphore?
>
> Modern i386 kernels don't grab the mm semaphore when the access is >=
> TASK_SIZE and the access came from kernel space (actually I see x86-64
> still does, but that's a bug, will fix). You could only see a deadlock when
> using user addresses and you already hold the mm semaphore for writing
> (normal read lock is ok). Just don't do that.

OK. It don't deadlock when kgdb accesses kernel addresses.

When a user space address is accessed through kgdb, won't the kernel attempt 
to fault in the user page? We don't want that to happen inside kgdb.

-Amit


>
> > George has coded cfi directives i386 too. He can use them to backtrace
> > past irqs stack.
>
> Problem is that he did it without binutils support. I don't think that's a
> good idea because it makes the code basically unmaintainable for normal
> souls (it's like writing assembly code directly in hex)
>
> -Andi

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Fri, 6 Feb 2004 17:28:36 +0530
"Amit S. Kale" <amitkale@emsyssoft.com> wrote:

> On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> > On Thu, 5 Feb 2004 23:20:04 +0530
> >
> > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > > > Andrew Morton <akpm@osdl.org> writes:
> > > > > need to take a look at such things and really convice ourselves that
> > > > > they're worthwhile.  Personally, I'd only be interested in the basic
> > > > > stub.
> > > >
> > > > What I found always extremly ugly in the i386 stub was that it uses
> > > > magic globals to talk to the page fault handler. For the x86-64
> > > > version I replaced that by just using __get/__put_user in the memory
> > > > accesses, which is much cleaner. I would suggest doing that for i386
> > > > too.
> > >
> > > May be I am missing something obvious. When debugging a page fault
> > > handler if kgdb accesses an swapped-out user page doesn't it deadlock
> > > when trying to hold mm semaphore?
> >
> > Modern i386 kernels don't grab the mm semaphore when the access is >=
> > TASK_SIZE and the access came from kernel space (actually I see x86-64
> > still does, but that's a bug, will fix). You could only see a deadlock when
> > using user addresses and you already hold the mm semaphore for writing
> > (normal read lock is ok). Just don't do that.
> 
> OK. It don't deadlock when kgdb accesses kernel addresses.
> 
> When a user space address is accessed through kgdb, won't the kernel attempt 
> to fault in the user page? We don't want that to happen inside kgdb.

Yes, it will. But I don't think it's a bad thing. If the users doesn't want
that they should not follow user addresses. After all kgdb is for people
who know what they are doing.

-Andi

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
> On Fri, 6 Feb 2004 17:28:36 +0530
>
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> > > On Thu, 5 Feb 2004 23:20:04 +0530
> > >
> > > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > > On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > > > > Andrew Morton <akpm@osdl.org> writes:
> > > > > > need to take a look at such things and really convice ourselves
> > > > > > that they're worthwhile.  Personally, I'd only be interested in
> > > > > > the basic stub.
> > > > >
> > > > > What I found always extremly ugly in the i386 stub was that it uses
> > > > > magic globals to talk to the page fault handler. For the x86-64
> > > > > version I replaced that by just using __get/__put_user in the
> > > > > memory accesses, which is much cleaner. I would suggest doing that
> > > > > for i386 too.
> > > >
> > > > May be I am missing something obvious. When debugging a page fault
> > > > handler if kgdb accesses an swapped-out user page doesn't it deadlock
> > > > when trying to hold mm semaphore?
> > >
> > > Modern i386 kernels don't grab the mm semaphore when the access is >=
> > > TASK_SIZE and the access came from kernel space (actually I see x86-64
> > > still does, but that's a bug, will fix). You could only see a deadlock
> > > when using user addresses and you already hold the mm semaphore for
> > > writing (normal read lock is ok). Just don't do that.
> >
> > OK. It don't deadlock when kgdb accesses kernel addresses.
> >
> > When a user space address is accessed through kgdb, won't the kernel
> > attempt to fault in the user page? We don't want that to happen inside
> > kgdb.
>
> Yes, it will. But I don't think it's a bad thing. If the users doesn't want
> that they should not follow user addresses. After all kgdb is for people
> who know what they are doing.

Let kgdb refuse to access any addresses below TASK_SIZE. That's better than 
accidentally typing something and getting lost.

I guess preventing user address accesses is lesser evil compared to using a 
debugger global.

gdb itself may access user addresses in following scenarios. Not allowing user 
accesses doesn't hurt as gdb can accept that and recover gracefully.

1. When a function parameter contains a char pointer.
GDB prints the string pointed to by a function parameter when printing frame 
for the function. 

2. If a kernel variable points to a user string, print command will show its 
contents, unless user explicitely says print/x.

3. When a stack trace goes beyond an interrupt or an exception entrypoint:
The gdb at kgdb.sourceforge.net has some patchup for this problem, though it 
doesn't work all the time. Sometimes we do endup seeing one invalid stack 
frame for which gdb has accessed a user address.

-Amit

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Fri, 6 Feb 2004 18:35:16 +0530
"Amit S. Kale" <amitkale@emsyssoft.com> wrote:

> On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
> > On Fri, 6 Feb 2004 17:28:36 +0530
> >
> > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> > > > On Thu, 5 Feb 2004 23:20:04 +0530
> > > >
> > > > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > > > On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > > > > > Andrew Morton <akpm@osdl.org> writes:
> > > > > > > need to take a look at such things and really convice ourselves
> > > > > > > that they're worthwhile.  Personally, I'd only be interested in
> > > > > > > the basic stub.
> > > > > >
> > > > > > What I found always extremly ugly in the i386 stub was that it uses
> > > > > > magic globals to talk to the page fault handler. For the x86-64
> > > > > > version I replaced that by just using __get/__put_user in the
> > > > > > memory accesses, which is much cleaner. I would suggest doing that
> > > > > > for i386 too.
> > > > >
> > > > > May be I am missing something obvious. When debugging a page fault
> > > > > handler if kgdb accesses an swapped-out user page doesn't it deadlock
> > > > > when trying to hold mm semaphore?
> > > >
> > > > Modern i386 kernels don't grab the mm semaphore when the access is >=
> > > > TASK_SIZE and the access came from kernel space (actually I see x86-64
> > > > still does, but that's a bug, will fix). You could only see a deadlock
> > > > when using user addresses and you already hold the mm semaphore for
> > > > writing (normal read lock is ok). Just don't do that.
> > >
> > > OK. It don't deadlock when kgdb accesses kernel addresses.
> > >
> > > When a user space address is accessed through kgdb, won't the kernel
> > > attempt to fault in the user page? We don't want that to happen inside
> > > kgdb.
> >
> > Yes, it will. But I don't think it's a bad thing. If the users doesn't want
> > that they should not follow user addresses. After all kgdb is for people
> > who know what they are doing.
> 
> Let kgdb refuse to access any addresses below TASK_SIZE. That's better than 
> accidentally typing something and getting lost.

That's fine. But can you perhaps add a magic command that enables it again? 
I could imagine in a few cases it will be useful because there may be pages
mapped into the user address space that are not in the direct mapping
(that's mainly on 32bit of course, on amd64 you can always access all 
memory using mappings >TASK_SIZE)

-Andi

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Friday 06 Feb 2004 6:54 pm, Andi Kleen wrote:
> On Fri, 6 Feb 2004 18:35:16 +0530
>
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
> > > On Fri, 6 Feb 2004 17:28:36 +0530
> > >
> > > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > > On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> > > > > On Thu, 5 Feb 2004 23:20:04 +0530
> > > > >
> > > > > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > > > > On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> > > > > > > Andrew Morton <akpm@osdl.org> writes:
> > > > > > > > need to take a look at such things and really convice
> > > > > > > > ourselves that they're worthwhile.  Personally, I'd only be
> > > > > > > > interested in the basic stub.
> > > > > > >
> > > > > > > What I found always extremly ugly in the i386 stub was that it
> > > > > > > uses magic globals to talk to the page fault handler. For the
> > > > > > > x86-64 version I replaced that by just using __get/__put_user
> > > > > > > in the memory accesses, which is much cleaner. I would suggest
> > > > > > > doing that for i386 too.
> > > > > >
> > > > > > May be I am missing something obvious. When debugging a page
> > > > > > fault handler if kgdb accesses an swapped-out user page doesn't
> > > > > > it deadlock when trying to hold mm semaphore?
> > > > >
> > > > > Modern i386 kernels don't grab the mm semaphore when the access is
> > > > > >= TASK_SIZE and the access came from kernel space (actually I see
> > > > > x86-64 still does, but that's a bug, will fix). You could only see
> > > > > a deadlock when using user addresses and you already hold the mm
> > > > > semaphore for writing (normal read lock is ok). Just don't do that.
> > > >
> > > > OK. It don't deadlock when kgdb accesses kernel addresses.
> > > >
> > > > When a user space address is accessed through kgdb, won't the kernel
> > > > attempt to fault in the user page? We don't want that to happen
> > > > inside kgdb.
> > >
> > > Yes, it will. But I don't think it's a bad thing. If the users doesn't
> > > want that they should not follow user addresses. After all kgdb is for
> > > people who know what they are doing.
> >
> > Let kgdb refuse to access any addresses below TASK_SIZE. That's better
> > than accidentally typing something and getting lost.
>
> That's fine. But can you perhaps add a magic command that enables it again?

Yes. This sounds good.

-Amit

> I could imagine in a few cases it will be useful because there may be pages
> mapped into the user address space that are not in the direct mapping
> (that's mainly on 32bit of course, on amd64 you can always access all
> memory using mappings >TASK_SIZE)
>
> -Andi

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Amit S. Kale wrote:
> On Friday 06 Feb 2004 6:54 pm, Andi Kleen wrote:
> 
>>On Fri, 6 Feb 2004 18:35:16 +0530
>>
>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>
>>>On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
>>>
>>>>On Fri, 6 Feb 2004 17:28:36 +0530
>>>>
>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>
>>>>>On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
>>>>>
>>>>>>On Thu, 5 Feb 2004 23:20:04 +0530
>>>>>>
>>>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>>>
>>>>>>>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
>>>>>>>
>>>>>>>>Andrew Morton <akpm@osdl.org> writes:
>>>>>>>>
>>>>>>>>>need to take a look at such things and really convice
>>>>>>>>>ourselves that they're worthwhile.  Personally, I'd only be
>>>>>>>>>interested in the basic stub.
>>>>>>>>
>>>>>>>>What I found always extremly ugly in the i386 stub was that it
>>>>>>>>uses magic globals to talk to the page fault handler. For the
>>>>>>>>x86-64 version I replaced that by just using __get/__put_user
>>>>>>>>in the memory accesses, which is much cleaner. I would suggest
>>>>>>>>doing that for i386 too.
>>>>>>>
>>>>>>>May be I am missing something obvious. When debugging a page
>>>>>>>fault handler if kgdb accesses an swapped-out user page doesn't
>>>>>>>it deadlock when trying to hold mm semaphore?
>>>>>>
>>>>>>Modern i386 kernels don't grab the mm semaphore when the access is
>>>>>>
>>>>>>>= TASK_SIZE and the access came from kernel space (actually I see
>>>>>>
>>>>>>x86-64 still does, but that's a bug, will fix). You could only see
>>>>>>a deadlock when using user addresses and you already hold the mm
>>>>>>semaphore for writing (normal read lock is ok). Just don't do that.
>>>>>
>>>>>OK. It don't deadlock when kgdb accesses kernel addresses.
>>>>>
>>>>>When a user space address is accessed through kgdb, won't the kernel
>>>>>attempt to fault in the user page? We don't want that to happen
>>>>>inside kgdb.
>>>>
>>>>Yes, it will. But I don't think it's a bad thing. If the users doesn't
>>>>want that they should not follow user addresses. After all kgdb is for
>>>>people who know what they are doing.
>>>
>>>Let kgdb refuse to access any addresses below TASK_SIZE. That's better
>>>than accidentally typing something and getting lost.
>>
>>That's fine. But can you perhaps add a magic command that enables it again?
> 
> 
> Yes. This sounds good.

This could be a flag in the kgdb_info structure.  See -mm kgdb.  Does not 
require any new commands as it is just a global the user can change.


-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Saturday 28 Feb 2004 5:35 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > On Friday 06 Feb 2004 6:54 pm, Andi Kleen wrote:
> >>On Fri, 6 Feb 2004 18:35:16 +0530
> >>
> >>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> >>>On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
> >>>>On Fri, 6 Feb 2004 17:28:36 +0530
> >>>>
> >>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> >>>>>On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
> >>>>>>On Thu, 5 Feb 2004 23:20:04 +0530
> >>>>>>
> >>>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> >>>>>>>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> >>>>>>>>Andrew Morton <akpm@osdl.org> writes:
> >>>>>>>>>need to take a look at such things and really convice
> >>>>>>>>>ourselves that they're worthwhile.  Personally, I'd only be
> >>>>>>>>>interested in the basic stub.
> >>>>>>>>
> >>>>>>>>What I found always extremly ugly in the i386 stub was that it
> >>>>>>>>uses magic globals to talk to the page fault handler. For the
> >>>>>>>>x86-64 version I replaced that by just using __get/__put_user
> >>>>>>>>in the memory accesses, which is much cleaner. I would suggest
> >>>>>>>>doing that for i386 too.
> >>>>>>>
> >>>>>>>May be I am missing something obvious. When debugging a page
> >>>>>>>fault handler if kgdb accesses an swapped-out user page doesn't
> >>>>>>>it deadlock when trying to hold mm semaphore?
> >>>>>>
> >>>>>>Modern i386 kernels don't grab the mm semaphore when the access is
> >>>>>>
> >>>>>>>= TASK_SIZE and the access came from kernel space (actually I see
> >>>>>>
> >>>>>>x86-64 still does, but that's a bug, will fix). You could only see
> >>>>>>a deadlock when using user addresses and you already hold the mm
> >>>>>>semaphore for writing (normal read lock is ok). Just don't do that.
> >>>>>
> >>>>>OK. It don't deadlock when kgdb accesses kernel addresses.
> >>>>>
> >>>>>When a user space address is accessed through kgdb, won't the kernel
> >>>>>attempt to fault in the user page? We don't want that to happen
> >>>>>inside kgdb.
> >>>>
> >>>>Yes, it will. But I don't think it's a bad thing. If the users doesn't
> >>>>want that they should not follow user addresses. After all kgdb is for
> >>>>people who know what they are doing.
> >>>
> >>>Let kgdb refuse to access any addresses below TASK_SIZE. That's better
> >>>than accidentally typing something and getting lost.
> >>
> >>That's fine. But can you perhaps add a magic command that enables it
> >> again?
> >
> > Yes. This sounds good.
>
> This could be a flag in the kgdb_info structure.  See -mm kgdb.  Does not
> require any new commands as it is just a global the user can change.

Having all user modifiable variables in one place is definitely a good idea.  
Need to do this sometime.
-Amit

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Amit S. Kale wrote:
> On Saturday 28 Feb 2004 5:35 am, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>On Friday 06 Feb 2004 6:54 pm, Andi Kleen wrote:
>>>
>>>>On Fri, 6 Feb 2004 18:35:16 +0530
>>>>
>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>
>>>>>On Friday 06 Feb 2004 5:46 pm, Andi Kleen wrote:
>>>>>
>>>>>>On Fri, 6 Feb 2004 17:28:36 +0530
>>>>>>
>>>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>>>
>>>>>>>On Friday 06 Feb 2004 7:50 am, Andi Kleen wrote:
>>>>>>>
>>>>>>>>On Thu, 5 Feb 2004 23:20:04 +0530
>>>>>>>>
>>>>>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>>>>>
>>>>>>>>>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
>>>>>>>>>
>>>>>>>>>>Andrew Morton <akpm@osdl.org> writes:
>>>>>>>>>>
>>>>>>>>>>>need to take a look at such things and really convice
>>>>>>>>>>>ourselves that they're worthwhile.  Personally, I'd only be
>>>>>>>>>>>interested in the basic stub.
>>>>>>>>>>
>>>>>>>>>>What I found always extremly ugly in the i386 stub was that it
>>>>>>>>>>uses magic globals to talk to the page fault handler. For the
>>>>>>>>>>x86-64 version I replaced that by just using __get/__put_user
>>>>>>>>>>in the memory accesses, which is much cleaner. I would suggest
>>>>>>>>>>doing that for i386 too.
>>>>>>>>>
>>>>>>>>>May be I am missing something obvious. When debugging a page
>>>>>>>>>fault handler if kgdb accesses an swapped-out user page doesn't
>>>>>>>>>it deadlock when trying to hold mm semaphore?
>>>>>>>>
>>>>>>>>Modern i386 kernels don't grab the mm semaphore when the access is
>>>>>>>>
>>>>>>>>
>>>>>>>>>= TASK_SIZE and the access came from kernel space (actually I see
>>>>>>>>
>>>>>>>>x86-64 still does, but that's a bug, will fix). You could only see
>>>>>>>>a deadlock when using user addresses and you already hold the mm
>>>>>>>>semaphore for writing (normal read lock is ok). Just don't do that.
>>>>>>>
>>>>>>>OK. It don't deadlock when kgdb accesses kernel addresses.
>>>>>>>
>>>>>>>When a user space address is accessed through kgdb, won't the kernel
>>>>>>>attempt to fault in the user page? We don't want that to happen
>>>>>>>inside kgdb.
>>>>>>
>>>>>>Yes, it will. But I don't think it's a bad thing. If the users doesn't
>>>>>>want that they should not follow user addresses. After all kgdb is for
>>>>>>people who know what they are doing.
>>>>>
>>>>>Let kgdb refuse to access any addresses below TASK_SIZE. That's better
>>>>>than accidentally typing something and getting lost.
>>>>
>>>>That's fine. But can you perhaps add a magic command that enables it
>>>>again?
>>>
>>>Yes. This sounds good.
>>
>>This could be a flag in the kgdb_info structure.  See -mm kgdb.  Does not
>>require any new commands as it is just a global the user can change.
> 
> 
> Having all user modifiable variables in one place is definitely a good idea.  
> Need to do this sometime.

I also put the discovered status of each cpu other than the current one here. 
The structure is explained in the documentation part of the -mm patch.  Another 
thing I put here and have found to be very useful is the address that the stub 
was called from.  Often it is not clear just why we are in the stub, given that 
we trap such things as kernel page faults, NMI watchdog, BUG macros and such.  I 
have found, on occasion, that knowing this info has been key to understanding 
what was wrong.

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

George Anzinger <george@mvista.com> wrote:
>
>  Often it is not clear just why we are in the stub, given that 
> we trap such things as kernel page faults, NMI watchdog, BUG macros and such.

Yes, that can be confusing.  A little printk on the console prior to
entering the debugger would be nice.

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andrew Morton wrote:
> George Anzinger <george@mvista.com> wrote:
> 
>> Often it is not clear just why we are in the stub, given that 
>>we trap such things as kernel page faults, NMI watchdog, BUG macros and such.
> 
> 
> Yes, that can be confusing.  A little printk on the console prior to
> entering the debugger would be nice.

That assumes that one can do a printk and not run into a lock.  Far better 
IMNSHO is to provide a simple way to get it from gdb.  One can then even provide 
a gdb macro to print the relevant source line and its surrounds.  I my lighter 
moments I call this the comefrom macro :)  In my kgdb it would look like:

l * kgdb_info.called_from


-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Wednesday 03 Mar 2004 5:22 am, George Anzinger wrote:
> Andrew Morton wrote:
> > George Anzinger <george@mvista.com> wrote:
> >> Often it is not clear just why we are in the stub, given that
> >>we trap such things as kernel page faults, NMI watchdog, BUG macros and
> >> such.
> >
> > Yes, that can be confusing.  A little printk on the console prior to
> > entering the debugger would be nice.
>
> That assumes that one can do a printk and not run into a lock.  Far better
> IMNSHO is to provide a simple way to get it from gdb.  One can then even
> provide a gdb macro to print the relevant source line and its surrounds.  I
> my lighter moments I call this the comefrom macro :)  In my kgdb it would
> look like:
>
> l * kgdb_info.called_from

How about echoing "Waiting for gdb connection" stright into the serial line 
without any encoding? Since gdb won't be connected to the other end, and many 
a times a minicom could be running at the other end, it'll give a user an 
indication of kgdb being ready.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Wed, Mar 03, 2004 at 10:38:39AM +0530, Amit S. Kale wrote:
> On Wednesday 03 Mar 2004 5:22 am, George Anzinger wrote:
> > Andrew Morton wrote:
> > > George Anzinger <george@mvista.com> wrote:
> > >> Often it is not clear just why we are in the stub, given that
> > >>we trap such things as kernel page faults, NMI watchdog, BUG macros and
> > >> such.
> > >
> > > Yes, that can be confusing.  A little printk on the console prior to
> > > entering the debugger would be nice.
> >
> > That assumes that one can do a printk and not run into a lock.  Far better
> > IMNSHO is to provide a simple way to get it from gdb.  One can then even
> > provide a gdb macro to print the relevant source line and its surrounds.  I
> > my lighter moments I call this the comefrom macro :)  In my kgdb it would
> > look like:
> >
> > l * kgdb_info.called_from
> 
> How about echoing "Waiting for gdb connection" stright into the serial line 
> without any encoding? Since gdb won't be connected to the other end, and many 
> a times a minicom could be running at the other end, it'll give a user an 
> indication of kgdb being ready.

It's not "GDB is ready" it's "GDB is ready now because ..."

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Amit S. Kale wrote:
> On Wednesday 03 Mar 2004 5:22 am, George Anzinger wrote:
> 
>>Andrew Morton wrote:
>>
>>>George Anzinger <george@mvista.com> wrote:
>>>
>>>>Often it is not clear just why we are in the stub, given that
>>>>we trap such things as kernel page faults, NMI watchdog, BUG macros and
>>>>such.
>>>
>>>Yes, that can be confusing.  A little printk on the console prior to
>>>entering the debugger would be nice.
>>
>>That assumes that one can do a printk and not run into a lock.  Far better
>>IMNSHO is to provide a simple way to get it from gdb.  One can then even
>>provide a gdb macro to print the relevant source line and its surrounds.  I
>>my lighter moments I call this the comefrom macro :)  In my kgdb it would
>>look like:
>>
>>l * kgdb_info.called_from
> 
> 
> How about echoing "Waiting for gdb connection" stright into the serial line 
> without any encoding? Since gdb won't be connected to the other end, and many 
> a times a minicom could be running at the other end, it'll give a user an 
> indication of kgdb being ready.

Uh, different solution for a different problem.  The above command to gdb causes 
the source code around the location "kgdb_info.called_from" to be displayed.  In 
the -mm version, this is location is filled in by kgdb with the return address 
for the "kgdb_handle_exception()".  This allows you to see just why you are in kgdb.



-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Tue, Mar 02, 2004 at 01:27:51PM -0800, Andrew Morton wrote:
> George Anzinger <george@mvista.com> wrote:
> >
> >  Often it is not clear just why we are in the stub, given that 
> > we trap such things as kernel page faults, NMI watchdog, BUG macros and such.
> 
> Yes, that can be confusing.  A little printk on the console prior to
> entering the debugger would be nice.

What I did for kdb and panic some time ago was to flash the keyboard
lights. If you use a unique frequency (different from kdb 
and from panic) it works quite nicely.

-Andi

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andi Kleen wrote:
> On Tue, Mar 02, 2004 at 01:27:51PM -0800, Andrew Morton wrote:
> 
>>George Anzinger <george@mvista.com> wrote:
>>
>>> Often it is not clear just why we are in the stub, given that 
>>>we trap such things as kernel page faults, NMI watchdog, BUG macros and such.
>>
>>Yes, that can be confusing.  A little printk on the console prior to
>>entering the debugger would be nice.
> 
> 
> What I did for kdb and panic some time ago was to flash the keyboard
> lights. If you use a unique frequency (different from kdb 
> and from panic) it works quite nicely.

Assuming a key board and a clear (no spin locks) path to it.  Still it only says 
we are in kgdb, now why.


-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Wed, 03 Mar 2004 16:43:47 -0800
George Anzinger <george@mvista.com> wrote:

> Andi Kleen wrote:
> > On Tue, Mar 02, 2004 at 01:27:51PM -0800, Andrew Morton wrote:
> > 
> >>George Anzinger <george@mvista.com> wrote:
> >>
> >>> Often it is not clear just why we are in the stub, given that 
> >>>we trap such things as kernel page faults, NMI watchdog, BUG macros and such.
> >>
> >>Yes, that can be confusing.  A little printk on the console prior to
> >>entering the debugger would be nice.
> > 
> > 
> > What I did for kdb and panic some time ago was to flash the keyboard
> > lights. If you use a unique frequency (different from kdb 
> > and from panic) it works quite nicely.
> 
> Assuming a key board and a clear (no spin locks) path to it.  Still it only says 

I think it's reasonable to just write to the keyboard without any locking.
The keyboard driver will recover.

> we are in kgdb, now why.

The big advantage is that it works even when you are in X (like most people) 
printks are often not visible.

-Andi
> 

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Thursday 04 Mar 2004 6:20 am, Andi Kleen wrote:
> On Wed, 03 Mar 2004 16:43:47 -0800
>
> George Anzinger <george@mvista.com> wrote:
> > Andi Kleen wrote:
> > > On Tue, Mar 02, 2004 at 01:27:51PM -0800, Andrew Morton wrote:
> > >>George Anzinger <george@mvista.com> wrote:
> > >>> Often it is not clear just why we are in the stub, given that
> > >>>we trap such things as kernel page faults, NMI watchdog, BUG macros
> > >>> and such.
> > >>
> > >>Yes, that can be confusing.  A little printk on the console prior to
> > >>entering the debugger would be nice.
> > >
> > > What I did for kdb and panic some time ago was to flash the keyboard
> > > lights. If you use a unique frequency (different from kdb
> > > and from panic) it works quite nicely.

Flashing keyboard lights is far simpler compared to a printk. Printk is too 
heavy. Once a system is unstable, it's more important to run into kgdb code 
asap. Calling printk and co may be too much.

> >
> > Assuming a key board and a clear (no spin locks) path to it.  Still it
> > only says
>
> I think it's reasonable to just write to the keyboard without any locking.
> The keyboard driver will recover.

Flashing keyboard lights is easy on x86 and x86_64 platforms. Is that easy on 
ppc workstations/servers? Embedded boards don't have keyboards.

>
> > we are in kgdb, now why.
>
> The big advantage is that it works even when you are in X (like most
> people) printks are often not visible.

Yep.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>
> Flashing keyboard lights is easy on x86 and x86_64 platforms. 

Please, no keyboards.  Some people want to be able to use kgdboe
to find out why machine number 324 down the corridor just died.

How about just doing


char *why_i_crashed;


{
	...
	if (expr1)
		why_i_crashed = "hit a BUG";
	else if (expr2)
		why_i_crashed = "divide by zero";
	else ...
}

then provide a gdb macro which prints out the string at *why_i_crashed?

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Thursday 04 Mar 2004 10:48 am, Andrew Morton wrote:
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > Flashing keyboard lights is easy on x86 and x86_64 platforms.
>
> Please, no keyboards.  Some people want to be able to use kgdboe
> to find out why machine number 324 down the corridor just died.
>
> How about just doing
>
>
> char *why_i_crashed;
>
>
> {
> 	...
> 	if (expr1)
> 		why_i_crashed = "hit a BUG";
> 	else if (expr2)
> 		why_i_crashed = "divide by zero";
> 	else ...
> }
>
> then provide a gdb macro which prints out the string at *why_i_crashed?

If we can afford to do this (in terms of actions that can be done with the 
machine being unstable) we can certainly print a console message through gdb.

A stub is free to send console messages to gdb at any time. We can send a 
"'O'hex(Page fault at 0x1234)" packet to gdb regardless of whether 
CONFIG_KGDB_CONSOLE is configured in. This way kgdb will send this packet to 
gdb and then immediately report a segfault/trap. To a user it'll appear as a 
message printed from gdb "Page fault at 0x1234" followed by gdb showing a 
SIGSEGV etc. The gdb console message should print information other than a 
signal number.

-Amit

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>
> To a user it'll appear as a 
>  message printed from gdb "Page fault at 0x1234" followed by gdb showing a 
>  SIGSEGV etc.

Well that would be nice.  Bear in mind that one usage scenario is to say
"hey, machine 342 has stopped responding" and to then fire up gdb and
connect to that machine with kgdboe.

In other words: if we rely on a gdb instance being connected at the time of
the exception, that message will be lost.  There needs to be a way of
retrieving the message post-facto.

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Amit S. Kale wrote:
> On Thursday 04 Mar 2004 10:48 am, Andrew Morton wrote:
> 
>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>
>>>Flashing keyboard lights is easy on x86 and x86_64 platforms.
>>
>>Please, no keyboards.  Some people want to be able to use kgdboe
>>to find out why machine number 324 down the corridor just died.
>>
>>How about just doing
>>
>>
>>char *why_i_crashed;
>>
>>
>>{
>>	...
>>	if (expr1)
>>		why_i_crashed = "hit a BUG";
>>	else if (expr2)
>>		why_i_crashed = "divide by zero";
>>	else ...
>>}
>>
>>then provide a gdb macro which prints out the string at *why_i_crashed?
> 
> 
> If we can afford to do this (in terms of actions that can be done with the 
> machine being unstable) we can certainly print a console message through gdb.

Not once you are connected to gdb.  The "O" packet can only be sent if the 
program (i.e. kernel) is running as far as gdb knows.  So you could preceed a 
connection with this, but could not used it after gdb knows the kernel is stopped.
> 
> A stub is free to send console messages to gdb at any time. We can send a 
> "'O'hex(Page fault at 0x1234)" packet to gdb regardless of whether 
> CONFIG_KGDB_CONSOLE is configured in. This way kgdb will send this packet to 
> gdb and then immediately report a segfault/trap. To a user it'll appear as a 
> message printed from gdb "Page fault at 0x1234" followed by gdb showing a 
> SIGSEGV etc. The gdb console message should print information other than a 
> signal number.
> 
> -Amit
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Thu, Mar 04, 2004 at 12:54:12PM -0800, George Anzinger wrote:
> Amit S. Kale wrote:
> >On Thursday 04 Mar 2004 10:48 am, Andrew Morton wrote:
> >
> >>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> >>
> >>>Flashing keyboard lights is easy on x86 and x86_64 platforms.
> >>
> >>Please, no keyboards.  Some people want to be able to use kgdboe
> >>to find out why machine number 324 down the corridor just died.
> >>
> >>How about just doing
> >>
> >>
> >>char *why_i_crashed;
> >>
> >>
> >>{
> >>	...
> >>	if (expr1)
> >>		why_i_crashed = "hit a BUG";
> >>	else if (expr2)
> >>		why_i_crashed = "divide by zero";
> >>	else ...
> >>}
> >>
> >>then provide a gdb macro which prints out the string at *why_i_crashed?
> >
> >
> >If we can afford to do this (in terms of actions that can be done with the 
> >machine being unstable) we can certainly print a console message through 
> >gdb.
> 
> Not once you are connected to gdb.  The "O" packet can only be sent if the 
> program (i.e. kernel) is running as far as gdb knows.  So you could preceed 
> a connection with this, but could not used it after gdb knows the kernel is 
> stopped.

If GDB is already connected and sitting by waiting, you can send the O
packet.  If it is not, you could delay sending the O packet until you
know that GDB has now connected.

This isn't an unworkable idea, but it's probably better to just set
*why_i_crashed (think work work work, oh wait, what caused this again?)
and provide some handy macros (which we should be in the docs anyhow).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Tom Rini wrote:
> On Thu, Mar 04, 2004 at 12:54:12PM -0800, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>On Thursday 04 Mar 2004 10:48 am, Andrew Morton wrote:
>>>
>>>
>>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>>
>>>>
>>>>>Flashing keyboard lights is easy on x86 and x86_64 platforms.
>>>>
>>>>Please, no keyboards.  Some people want to be able to use kgdboe
>>>>to find out why machine number 324 down the corridor just died.
>>>>
>>>>How about just doing
>>>>
>>>>
>>>>char *why_i_crashed;
>>>>
>>>>
>>>>{
>>>>	...
>>>>	if (expr1)
>>>>		why_i_crashed = "hit a BUG";
>>>>	else if (expr2)
>>>>		why_i_crashed = "divide by zero";
>>>>	else ...
>>>>}
>>>>
>>>>then provide a gdb macro which prints out the string at *why_i_crashed?
>>>
>>>
>>>If we can afford to do this (in terms of actions that can be done with the 
>>>machine being unstable) we can certainly print a console message through 
>>>gdb.
>>
>>Not once you are connected to gdb.  The "O" packet can only be sent if the 
>>program (i.e. kernel) is running as far as gdb knows.  So you could preceed 
>>a connection with this, but could not used it after gdb knows the kernel is 
>>stopped.
> 
> 
> If GDB is already connected and sitting by waiting, you can send the O
> packet.  If it is not, you could delay sending the O packet until you
> know that GDB has now connected.
> 
> This isn't an unworkable idea, but it's probably better to just set
> *why_i_crashed (think work work work, oh wait, what caused this again?)
> and provide some handy macros (which we should be in the docs anyhow).

Well, I did provide a "come_from" macro, but more definition could be had...

On the subject of macros, I am being convinced by the gdb folks that the way to 
do the info threads thing is with macros.  We get almost all we want this way 
without messing with gdb or lying to it the way the -mm kgdb does.
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Wed, 3 Mar 2004 21:18:50 -0800
Andrew Morton <akpm@osdl.org> wrote:

> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> >
> > Flashing keyboard lights is easy on x86 and x86_64 platforms. 
> 
> Please, no keyboards.  Some people want to be able to use kgdboe
> to find out why machine number 324 down the corridor just died.

Not as the only indication I agree. But for machines running X that
are actually used by people I think it's important to always give some kind 
of visual feedback when the X server freezes. And kgdb will make the X server
freeze. You could actually make it a notifier list to register severals
ways to do this, e.g. the cluster people could add something that makes
it flash a warning light. For a standard box I think flashing the keyboard
is a good default for now

(ok there are USB keyboards too, for those there will need to be a different
solution)

> 
> char *why_i_crashed;
> 
> 
> {
> 	...
> 	if (expr1)
> 		why_i_crashed = "hit a BUG";
> 	else if (expr2)
> 		why_i_crashed = "divide by zero";
> 	else ...
> }
> 
> then provide a gdb macro which prints out the string at *why_i_crashed?

That doesn't tell the user at all why his X server just froze.
But it may be a good addition.

-Andi

Re: kgdb support in vanilla 2.6.2

[Amit S. Kale]

On Friday 06 Feb 2004 6:54 pm, Andi Kleen wrote:
> > > > > > > What I found always extremly ugly in the i386 stub was that it
> > > > > > > uses magic globals to talk to the page fault handler. For the
> > > > > > > x86-64 version I replaced that by just using __get/__put_user
> > > > > > > in the memory accesses, which is much cleaner. I would suggest
> > > > > > > doing that for i386 too.

Done in this version of kgdb.

-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andi Kleen wrote:
> On Thu, 5 Feb 2004 23:20:04 +0530
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> 
> 
>>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
>>
>>>Andrew Morton <akpm@osdl.org> writes:
>>>
>>>>need to take a look at such things and really convice ourselves that
>>>>they're worthwhile.  Personally, I'd only be interested in the basic
>>>>stub.
>>>
>>>What I found always extremly ugly in the i386 stub was that it uses
>>>magic globals to talk to the page fault handler. For the x86-64
>>>version I replaced that by just using __get/__put_user in the memory
>>>accesses, which is much cleaner. I would suggest doing that for i386
>>>too.
>>
>>May be I am missing something obvious. When debugging a page fault handler if 
>>kgdb accesses an swapped-out user page doesn't it deadlock when trying to 
>>hold mm semaphore?
> 
> 
> Modern i386 kernels don't grab the mm semaphore when the access is >= TASK_SIZE
> and the access came from kernel space (actually I see x86-64 still does, but that's 
> a bug, will fix). You could only see a deadlock when using user addresses
> and you already hold the mm semaphore for writing (normal read lock is ok). 
> Just don't do that. 
> 
> 
> 
>>George has coded cfi directives i386 too. He can use them to backtrace past 
>>irqs stack.
> 
> 
> Problem is that he did it without binutils support. I don't think that's a good
> idea because it makes the code basically unmaintainable for normal souls
> (it's like writing assembly code directly in hex) 

Well, bin utils, at this time, makes it even worse in that it does not support 
the expression syntax.  Also, it is not asm but dwarf2 and it is written in, 
IMHO, useful macros (not hex :)



-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Tue, 10 Feb 2004 13:56:24 -0800
George Anzinger <george@mvista.com> wrote:

> > Problem is that he did it without binutils support. I don't think that's a good
> > idea because it makes the code basically unmaintainable for normal souls
> > (it's like writing assembly code directly in hex) 
> 
> Well, bin utils, at this time, makes it even worse in that it does not support 
> the expression syntax.  Also, it is not asm but dwarf2 and it is written in, 
> IMHO, useful macros (not hex :)

The latest binutils should support .cfi_* for i386 too. I don't see much sense
in making the code more ugly just for staying backwards compatible with older versions for the 
debug case (without CONFIG_DEBUG_INFO it should be compatible though).
You need a fairly new gdb too anyways for it.

-Andi

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andi Kleen wrote:
> On Tue, 10 Feb 2004 13:56:24 -0800
> George Anzinger <george@mvista.com> wrote:
> 
> 
>>>Problem is that he did it without binutils support. I don't think that's a good
>>>idea because it makes the code basically unmaintainable for normal souls
>>>(it's like writing assembly code directly in hex) 
>>
>>Well, bin utils, at this time, makes it even worse in that it does not support 
>>the expression syntax.  Also, it is not asm but dwarf2 and it is written in, 
>>IMHO, useful macros (not hex :)
> 
> 
> The latest binutils should support .cfi_* for i386 too. I don't see much sense
> in making the code more ugly just for staying backwards compatible with older versions for the 
> debug case (without CONFIG_DEBUG_INFO it should be compatible though).
> You need a fairly new gdb too anyways for it.

Well, yes, the CVS version I mentioned in my patch is needed as I found a bug in 
the expression analizer.  I am NOT trying to say compatable with old tools.  I 
AM trying to do something the CURRENT tools make hard to impossible.

The problem with the gas CFI support is that it does not provide a way to define 
CFI expressions which are needed to determine if the CFI address should be zero 
(i.e. the return is to user space) or the current adjusted stack address.

I suppose the open ended .cfi_ thing could be used but it requires that you 
compute your own sleb128 and uleb128 values.  It is also not clear how you tell 
this thing if you want a word or a half word as the dwarf2 spec requires.  More 
info on this would be very "nice".  I really would like to do this with out the 
dwarf2 macros, but, please understand, one of the main reasons for the effort 
was to tie off the bottom of the stack and that seems to require an expression 
capability for the asm code in entry.S.

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Andi Kleen]

On Wed, 11 Feb 2004 17:34:13 -0800
George Anzinger <george@mvista.com> wrote:


> > The latest binutils should support .cfi_* for i386 too. I don't see much sense
> > in making the code more ugly just for staying backwards compatible with older versions for the 
> > debug case (without CONFIG_DEBUG_INFO it should be compatible though).
> > You need a fairly new gdb too anyways for it.
> 
> Well, yes, the CVS version I mentioned in my patch is needed as I found a bug in 
> the expression analizer.  I am NOT trying to say compatable with old tools.  I 
> AM trying to do something the CURRENT tools make hard to impossible.
> 
> The problem with the gas CFI support is that it does not provide a way to define 
> CFI expressions which are needed to determine if the CFI address should be zero 
> (i.e. the return is to user space) or the current adjusted stack address.

Michal, can you comment? 
 
> I suppose the open ended .cfi_ thing could be used but it requires that you 
> compute your own sleb128 and uleb128 values.  It is also not clear how you tell 
> this thing if you want a word or a half word as the dwarf2 spec requires.  More 
> info on this would be very "nice".  I really would like to do this with out the 
> dwarf2 macros, but, please understand, one of the main reasons for the effort 
> was to tie off the bottom of the stack and that seems to require an expression 
> capability for the asm code in entry.S.

The one issue that required expression support on x86-64 (switching between the
interrupt stack and the process stack) was handled by a dummy base register
with a single ifdef. This turned out to be relatively clean.

-Andi

Re: kgdb support in vanilla 2.6.2

[Piet Delaney]

On Tue, 2004-02-10 at 13:56, George Anzinger wrote:

I thought I'd poke around an AMD64 with etherbased kgdb on 2.6.*.
I just picked up a used AMD64 K8D Master-F MS-9131 and thought I'd
install Fedora Core 1 test1 and the latest kernel from kernel.org.

It Might be interesting to try out a SuSE release also, I was wondering
if 9.0 from linuxiso.org might be best.

Last I worked with your kgdb patch for 2.6 Andrew's mm patch had the
latest code; Is that still the case?

-piet

> Andi Kleen wrote:
> > On Thu, 5 Feb 2004 23:20:04 +0530
> > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > 
> > 
> >>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
> >>
> >>>Andrew Morton <akpm@osdl.org> writes:
> >>>
> >>>>need to take a look at such things and really convice ourselves that
> >>>>they're worthwhile.  Personally, I'd only be interested in the basic
> >>>>stub.
> >>>
> >>>What I found always extremly ugly in the i386 stub was that it uses
> >>>magic globals to talk to the page fault handler. For the x86-64
> >>>version I replaced that by just using __get/__put_user in the memory
> >>>accesses, which is much cleaner. I would suggest doing that for i386
> >>>too.
> >>
> >>May be I am missing something obvious. When debugging a page fault handler if 
> >>kgdb accesses an swapped-out user page doesn't it deadlock when trying to 
> >>hold mm semaphore?
> > 
> > 
> > Modern i386 kernels don't grab the mm semaphore when the access is >= TASK_SIZE
> > and the access came from kernel space (actually I see x86-64 still does, but that's 
> > a bug, will fix). You could only see a deadlock when using user addresses
> > and you already hold the mm semaphore for writing (normal read lock is ok). 
> > Just don't do that. 
> > 
> > 
> > 
> >>George has coded cfi directives i386 too. He can use them to backtrace past 
> >>irqs stack.
> > 
> > 
> > Problem is that he did it without binutils support. I don't think that's a good
> > idea because it makes the code basically unmaintainable for normal souls
> > (it's like writing assembly code directly in hex) 
> 
> Well, bin utils, at this time, makes it even worse in that it does not support 
> the expression syntax.  Also, it is not asm but dwarf2 and it is written in, 
> IMHO, useful macros (not hex :)
> 
> 
> 
> -- 
> George Anzinger   george@mvista.com
> High-res-timers:  http://sourceforge.net/projects/high-res-timers/
> Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
> 
> -
> 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/
-- 
piet@www.piet.net

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Piet Delaney wrote:
> On Tue, 2004-02-10 at 13:56, George Anzinger wrote:
> 
> I thought I'd poke around an AMD64 with etherbased kgdb on 2.6.*.
> I just picked up a used AMD64 K8D Master-F MS-9131 and thought I'd
> install Fedora Core 1 test1 and the latest kernel from kernel.org.
> 
> It Might be interesting to try out a SuSE release also, I was wondering
> if 9.0 from linuxiso.org might be best.
> 
> Last I worked with your kgdb patch for 2.6 Andrew's mm patch had the
> latest code; Is that still the case?

I think so.  I haven't look lately.  I did send out an additional patch that 
only put in dwarft2 records in some of the asm code to allow better "bt" through 
the interrupt/exception frames.  I don't know if he picked this up or not, but 
it does require using the cvs version of gdb.

-g
> 
> -piet
> 
> 
>>Andi Kleen wrote:
>>
>>>On Thu, 5 Feb 2004 23:20:04 +0530
>>>"Amit S. Kale" <amitkale@emsyssoft.com> wrote:
>>>
>>>
>>>
>>>>On Thursday 05 Feb 2004 8:41 am, Andi Kleen wrote:
>>>>
>>>>
>>>>>Andrew Morton <akpm@osdl.org> writes:
>>>>>
>>>>>
>>>>>>need to take a look at such things and really convice ourselves that
>>>>>>they're worthwhile.  Personally, I'd only be interested in the basic
>>>>>>stub.
>>>>>
>>>>>What I found always extremly ugly in the i386 stub was that it uses
>>>>>magic globals to talk to the page fault handler. For the x86-64
>>>>>version I replaced that by just using __get/__put_user in the memory
>>>>>accesses, which is much cleaner. I would suggest doing that for i386
>>>>>too.
>>>>
>>>>May be I am missing something obvious. When debugging a page fault handler if 
>>>>kgdb accesses an swapped-out user page doesn't it deadlock when trying to 
>>>>hold mm semaphore?
>>>
>>>
>>>Modern i386 kernels don't grab the mm semaphore when the access is >= TASK_SIZE
>>>and the access came from kernel space (actually I see x86-64 still does, but that's 
>>>a bug, will fix). You could only see a deadlock when using user addresses
>>>and you already hold the mm semaphore for writing (normal read lock is ok). 
>>>Just don't do that. 
>>>
>>>
>>>
>>>
>>>>George has coded cfi directives i386 too. He can use them to backtrace past 
>>>>irqs stack.
>>>
>>>
>>>Problem is that he did it without binutils support. I don't think that's a good
>>>idea because it makes the code basically unmaintainable for normal souls
>>>(it's like writing assembly code directly in hex) 
>>
>>Well, bin utils, at this time, makes it even worse in that it does not support 
>>the expression syntax.  Also, it is not asm but dwarf2 and it is written in, 
>>IMHO, useful macros (not hex :)
>>
>>
>>
>>-- 
>>George Anzinger   george@mvista.com
>>High-res-timers:  http://sourceforge.net/projects/high-res-timers/
>>Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
>>
>>-
>>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/

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

> I thought I'd poke around an AMD64 with etherbased kgdb on 2.6.*.
> I just picked up a used AMD64 K8D Master-F MS-9131 and thought I'd
> install Fedora Core 1 test1 and the latest kernel from kernel.org.
> 
> It Might be interesting to try out a SuSE release also, I was wondering
> if 9.0 from linuxiso.org might be best.
> 
> Last I worked with your kgdb patch for 2.6 Andrew's mm patch had the
> latest code; Is that still the case?

I think that latest is the code in kgdb.sf.net cvs. Code in -mm tree
is "known good".
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

It seems that some kgdb support is in 2.6.2-linus:

+++ b/Documentation/sh/kgdb.txt Tue Feb  3 19:45:43 2004
@@ -0,0 +1,179 @@
+
+This file describes the configuration and behavior of KGDB for the SH
+kernel. Based on a description from Henry Bell <henry.bell@st.com>, it
+has been modified to account for quirks in the current implementation.
+

That's great, can we get i386 kgdb, too? Or at least amd64 kgdb
;-). [Or was it a mistake? It seems unlikely that kgdb could enter
Linus tree without major flamewar...]

								Pavel 
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> It seems that some kgdb support is in 2.6.2-linus:

Lots of architectures have had in-kernel kgdb support for a long time. 
Just none of the three which I use :(

I wouldn't support inclusion of i386 kgdb until it has had a lot of
cleanup, possible de-featuritisification and some thought has been applied
to splitting it into arch and generic bits.  It's quite a lot of work.

Re: kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

> > It seems that some kgdb support is in 2.6.2-linus:
> 
> Lots of architectures have had in-kernel kgdb support for a long time. 
> Just none of the three which I use :(
> 
> I wouldn't support inclusion of i386 kgdb until it has had a lot of
> cleanup, possible de-featuritisification and some thought has been applied
> to splitting it into arch and generic bits.  It's quite a lot of work.

What about Amit's kgdb?

It's a *lot* cleaner. It does not have all the features (kgdb-eth is
not yet ready for prime time). Would you accept that?

Oh and it is already split into arch-dependend and arch-independend
parts, plus it has cleanly separated i/o methods...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> > > It seems that some kgdb support is in 2.6.2-linus:
> > 
> > Lots of architectures have had in-kernel kgdb support for a long time. 
> > Just none of the three which I use :(
> > 
> > I wouldn't support inclusion of i386 kgdb until it has had a lot of
> > cleanup, possible de-featuritisification and some thought has been applied
> > to splitting it into arch and generic bits.  It's quite a lot of work.
> 
> What about Amit's kgdb?
> 
> It's a *lot* cleaner. It does not have all the features (kgdb-eth is
> not yet ready for prime time). Would you accept that?
> 
> Oh and it is already split into arch-dependend and arch-independend
> parts, plus it has cleanly separated i/o methods...

That all sounds positive.  I was waiting until that effort settles down a
bit before taking it for a ride.  I have a bad habit of finding bugs in the
gdb stub for some reason.

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Thu, Feb 05, 2004 at 12:24:47AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > It seems that some kgdb support is in 2.6.2-linus:
> > 
> > Lots of architectures have had in-kernel kgdb support for a long time. 
> > Just none of the three which I use :(
> > 
> > I wouldn't support inclusion of i386 kgdb until it has had a lot of
> > cleanup, possible de-featuritisification and some thought has been applied
> > to splitting it into arch and generic bits.  It's quite a lot of work.
> 
> What about Amit's kgdb?
> 
> It's a *lot* cleaner. It does not have all the features (kgdb-eth is
> not yet ready for prime time). Would you accept that?
> 
> Oh and it is already split into arch-dependend and arch-independend
> parts, plus it has cleanly separated i/o methods...

.. and it's supported on i386, x86_64 and PPC32 right now.

Andrew, what features of George's version don't you like?  Right now
I'm working on moving the kgdb-eth driver that uses netpoll over
into Amit's version, and thinking of a cleaner away to allow for both
early debugging and multiple drivers (eth or serial A or serial B).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

Tom Rini <trini@kernel.crashing.org> wrote:
>
> Andrew, what features of George's version don't you like?

This is bad:

akpm:/usr/src/25> grep '^+#ifdef' patches/kgdb-ga.patch | wc -l 
     83

and the fact that it touches 36 different files.

Any time I've had to do any maintenance work against that stub I get lost
in a twisty maze and just whine at George about it.  It's just all over the
place.  Yes, this is partly the nature of the beast, but I don't see that a
ton of effort has been put into reducing the straggliness.

> Right now
> I'm working on moving the kgdb-eth driver that uses netpoll over
> into Amit's version, and thinking of a cleaner away to allow for both
> early debugging and multiple drivers (eth or serial A or serial B).

Sounds good.

Look, there's a lot of interest in this and I of course am fully
supportive.  If someone could send me Amit's patchset when they think I
should test it, I could then talk about it more usefully.

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Wed, Feb 04, 2004 at 04:16:26PM -0800, Andrew Morton wrote:
> Tom Rini <trini@kernel.crashing.org> wrote:
> >
> > Andrew, what features of George's version don't you like?
> 
> This is bad:
> 
> akpm:/usr/src/25> grep '^+#ifdef' patches/kgdb-ga.patch | wc -l 
>      83
> 
> and the fact that it touches 36 different files.
> 
> Any time I've had to do any maintenance work against that stub I get lost
> in a twisty maze and just whine at George about it.  It's just all over the
> place.  Yes, this is partly the nature of the beast, but I don't see that a
> ton of effort has been put into reducing the straggliness.
> 
> > Right now
> > I'm working on moving the kgdb-eth driver that uses netpoll over
> > into Amit's version, and thinking of a cleaner away to allow for both
> > early debugging and multiple drivers (eth or serial A or serial B).
> 
> Sounds good.
> 
> Look, there's a lot of interest in this and I of course am fully
> supportive.  If someone could send me Amit's patchset when they think I
> should test it, I could then talk about it more usefully.

Alright.  I hope to soon have netpoll'ed kgdb-over-ethernet happy.  From
there, I'll send you a patch that's Amit's work + cleanups / fixes, and
better PPC support.  Then we can see which features are in George's
version become a must-have.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andrew Morton wrote:
> Tom Rini <trini@kernel.crashing.org> wrote:
> 
>>Andrew, what features of George's version don't you like?
> 
> 
> This is bad:
> 
> akpm:/usr/src/25> grep '^+#ifdef' patches/kgdb-ga.patch | wc -l 
>      83
> 
> and the fact that it touches 36 different files.

Been away from this for a while, but I do think this needs a comment.  The fact 
that it touches 36 files is tilted rather strongly in that a good number of 
those files are in the Document/* tree.  I.e. there is a rather larger amount of 
documentation.

As to the #ifdefs, I once worked on a kernel (HPUX if you must know) where you 
could NOT remove the debug stub and its bits.  Turns out the kernel began to 
depend on the code that was supposed to be debugging it.  I rather strongly try 
to avoid Heisenberg and the nasty thinks that arise from this sort of thing.  I 
think you will find that most of those #ifdefs are "#ifdef CONFIG_KGDB" so that 
it you turn it off it is just as if the patch was not done (save the configure 
script, of course).

There is also the attempt to make one patch cover several kernels (such as in 
the 2.4 case where we may have O(1) or not) and also the preempt or not AND at 
the same time, want to debug the preempt code.
> 
> Any time I've had to do any maintenance work against that stub I get lost
> in a twisty maze and just whine at George about it.  It's just all over the
> place.  Yes, this is partly the nature of the beast, but I don't see that a
> ton of effort has been put into reducing the straggliness.

Yes, I agree.  Some of this is caused by the need to work with a rather fixed 
interface to gdb.  We would, for example, like to tell gdb to flush its cache 
from time to time.  It would also be nice if gdb were to hint to us about what 
it was trying to do.  The single step over a break point comes to mind here, as 
does the function call set up.  Still, improvements can be made.
> 


-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[La Monte H.P. Yarroll]

Andrew Morton wrote:

>Pavel Machek <pavel@ucw.cz> wrote:
>  
>
>>It seems that some kgdb support is in 2.6.2-linus:
>>    
>>
>
>Lots of architectures have had in-kernel kgdb support for a long time. 
>Just none of the three which I use :(
>
>I wouldn't support inclusion of i386 kgdb until it has had a lot of
>cleanup, possible de-featuritisification and some thought has been applied
>to splitting it into arch and generic bits.  It's quite a lot of work.
>  
>

Amit has started at least the third activity--he has split much of kgdb
into arch and generic bits.

Could you elaborate a little on the first two?

What major kinds of cleanup are we talking about?  Style issues?

What features (or classes of features) do you find excessive?  Would
it be sufficient to add a few config items to control subfeatures
of kgdb?

These are not idle questions.  If the amount of work to get it ready
for acceptance is tractable, I know a company that may be willing to
pay to have the work done.

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

Re: kgdb support in vanilla 2.6.2

[Andrew Morton]

"La Monte H.P. Yarroll" <piggy@timesys.com> wrote:
>
> Andrew Morton wrote:
> 
> >Pavel Machek <pavel@ucw.cz> wrote:
> >  
> >
> >>It seems that some kgdb support is in 2.6.2-linus:
> >>    
> >>
> >
> >Lots of architectures have had in-kernel kgdb support for a long time. 
> >Just none of the three which I use :(
> >
> >I wouldn't support inclusion of i386 kgdb until it has had a lot of
> >cleanup, possible de-featuritisification and some thought has been applied
> >to splitting it into arch and generic bits.  It's quite a lot of work.
> >  
> >
> 
> Amit has started at least the third activity--he has split much of kgdb
> into arch and generic bits.

Yes.

> Could you elaborate a little on the first two?
> 
> What major kinds of cleanup are we talking about?  Style issues?

Coding style compliance, reduction of ifdefs, etc.  Reduction of patch
footprint.  There are a few features in the patch in -mm which I am not
aware of anyone having used.

> What features (or classes of features) do you find excessive?  Would
> it be sufficient to add a few config items to control subfeatures
> of kgdb?
> 

People have added timestamping infrastructure, stack overflow testing and
inbuilt assertion frameworks to various gdb stubs at various times.  We
need to take a look at such things and really convice ourselves that
they're worthwhile.  Personally, I'd only be interested in the basic stub.

I need to take a look at Amit's current patch - it sounds good.

Re: kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

> > Could you elaborate a little on the first two?
> > 
> > What major kinds of cleanup are we talking about?  Style issues?
> 
> Coding style compliance, reduction of ifdefs, etc.  Reduction of patch
> footprint.  There are a few features in the patch in -mm which I am not
> aware of anyone having used.
> 
> > What features (or classes of features) do you find excessive?  Would
> > it be sufficient to add a few config items to control subfeatures
> > of kgdb?
> > 
> 
> People have added timestamping infrastructure, stack overflow testing and
> inbuilt assertion frameworks to various gdb stubs at various times.  We
> need to take a look at such things and really convice ourselves that
> they're worthwhile.  Personally, I'd only be interested in the basic stub.
> 
> I need to take a look at Amit's current patch - it sounds good.

Amit's version does not contain neither timestamping, nor assertions,
nor overflows.

It has config option that allows it to hook into scheduling
(CONFIG_KGDB_THREAD) that wraps schedules() with some code. Without
that, patch is not intrusive at all. It would look like (untested, but
you should get the idea):

[I combined i386, core and serial patch; that's minimum usefull
configuration, and killed code inside CONFIG_KGDB_THREAD, as its a
little intrusive. It does not look bad at all.]

								Pavel


--- linux-2.6.1/include/linux/debugger.h	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/include/linux/debugger.h	2004-01-13 14:21:47.000000000 +0530
@@ -0,0 +1,67 @@
....definition of gdb_debug_hook and some functions to keep ifdef
noise down...

--- linux-2.6.1/include/linux/kgdb.h	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/include/linux/kgdb.h	2004-01-21 21:53:49.000000000 +0530
@@ -0,0 +1,113 @@
...definition of kgdb_breakpoint, kgdb_arch and kgdb_serial...

--- linux-2.6.1/init/main.c	2004-01-10 11:01:50.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/init/main.c	2004-01-12 19:11:04.000000000 +0530
@@ -39,6 +39,7 @@
 #include <linux/writeback.h>
 #include <linux/cpu.h>
 #include <linux/efi.h>
+#include <linux/debugger.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -578,6 +579,7 @@ static int init(void * unused)
 	do_pre_smp_initcalls();
 
 	smp_init();
+	debugger_entry();
 	do_basic_setup();
 
 	prepare_namespace();

--- linux-2.6.1/kernel/kgdbstub.c	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/kernel/kgdbstub.c	2004-01-20 11:44:09.000000000 +0530
@@ -0,0 +1,1236 @@
...okay, here lies the debugger. It still could take some cleanups.

--- linux-2.6.1/kernel/Makefile	2003-11-24 07:01:15.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/kernel/Makefile	2004-01-12 19:11:04.000000000 +0530
@@ -19,6 +19,7 @@ obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_COMPAT) += compat.o
 obj-$(CONFIG_IKCONFIG) += configs.o
 obj-$(CONFIG_IKCONFIG_PROC) += configs.o
+obj-$(CONFIG_KGDB) += kgdbstub.o
 
 ifneq ($(CONFIG_IA64),y)
 # According to Alan Modra <alan@linuxcare.com.au>, the -fno-omit-frame-pointer is

--- linux-2.6.1/kernel/module.c	2004-01-10 11:01:50.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/kernel/module.c	2004-01-12 19:11:04.000000000 +0530
@@ -727,6 +727,11 @@ sys_delete_module(const char __user *nam
 	mod->state = MODULE_STATE_GOING;
 	restart_refcounts();
 
+	down(&notify_mutex);
+	notifier_call_chain(&module_notify_list, MODULE_STATE_GOING,
+				mod);
+	up(&notify_mutex);
+
 	/* Never wait if forced. */
 	if (!forced && module_refcount(mod) != 0)
 		wait_for_zero_refcount(mod);
@@ -1744,7 +1749,12 @@ sys_init_module(void __user *umod,
 	if (ret < 0) {
 		/* Init routine failed: abort.  Try to protect us from
                    buggy refcounters. */
+
 		mod->state = MODULE_STATE_GOING;
+		down(&notify_mutex);
+		notifier_call_chain(&module_notify_list, MODULE_STATE_GOING,
+					mod);
+		up(&notify_mutex);
 		synchronize_kernel();
 		if (mod->unsafe)
 			printk(KERN_ERR "%s: module is now stuck!\n",

--- linux-2.6.1/kernel/sched.c	2004-01-10 11:01:50.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/kernel/sched.c	2004-01-14 17:05:11.000000000 +0530
@@ -37,6 +37,7 @@
 #include <linux/rcupdate.h>
 #include <linux/cpu.h>
 #include <linux/percpu.h>
+#include <linux/debugger.h>
 
 #ifdef CONFIG_NUMA
 #define cpu_to_node_mask(cpu) node_to_cpumask(cpu_to_node(cpu))
@@ -2851,6 +2869,8 @@ void __might_sleep(char *file, int line)
 #if defined(in_atomic)
 	static unsigned long prev_jiffy;	/* ratelimiting */
 
+	if (atomic_read(&debugger_active))
+		return;
 	if ((in_atomic() || irqs_disabled()) && system_running) {
 		if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
 			return;

--- linux-2.6.1/Makefile	2004-01-10 11:01:35.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-core/Makefile	2004-01-12 19:11:05.000000000 +0530
@@ -439,6 +439,8 @@ endif
 
 ifndef CONFIG_FRAME_POINTER
 CFLAGS		+= -fomit-frame-pointer
+else
+CFLAGS		+= -fno-omit-frame-pointer
 endif
 
 ifdef CONFIG_DEBUG_INFO

--- linux-2.6.1/arch/i386/Kconfig	2004-01-10 11:01:35.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/Kconfig	2004-01-12 19:11:34.000000000 +0530
@@ -1252,6 +1252,37 @@ config DEBUG_SPINLOCK_SLEEP
 	  If you say Y here, various routines which may sleep will become very
 	  noisy if they are called with a spinlock held.	
 
+config KGDB
+	bool "KGDB: kernel debugging with remote gdb"
+	depends on DEBUG_KERNEL
+	select DEBUG_INFO
+	select FRAME_POINTER
+	help
+	  If you say Y here, it will be possible to remotely debug the
+	  kernel using gdb. This enlarges your kernel image disk size by
+	  several megabytes and requires a machine with more than 128 MB
+	  RAM to avoid excessive linking time. 
+	  Documentation of kernel debugger available at
+	  http://kgdb.sourceforge.net
+	  This is only useful for kernel hackers. If unsure, say N.
+
+config KGDB_THREAD
+	bool "KGDB: Thread analysis"
+	depends on KGDB
+	help
+	  With thread analysis enabled, gdb can talk to kgdb stub to list
+	  threads and to get stack trace for a thread. This option also enables
+	  some code which helps gdb get exact status of thread. Thread analysis
+	  adds some overhead to schedule and down functions. You can disable
+	  this option if you do not want to compromise on speed.
+
+config KGDB_CONSOLE
+	bool "KGDB: Console messages through gdb"
+	depends on KGDB
+	help
+	  If you say Y here, console messages will appear through gdb.
+	  Other consoles such as tty or ttyS will continue to work as usual.
+
 config FRAME_POINTER
 	bool "Compile the kernel with frame pointers"
 	help

--- linux-2.6.1/arch/i386/kernel/entry.S	2003-11-24 07:01:26.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/entry.S	2004-01-20 20:03:48.000000000 +0530
@@ -399,7 +399,17 @@ vector=vector+1
 	ALIGN
 common_interrupt:
 	SAVE_ALL
+	movl %esp, %eax
+/* Create a fake function call followed by a fake function prologue to fool
+ * gdb into believing that this is a normal function call. */
+	pushl EIP(%eax)
+
+common_interrupt_1:
+	pushl %ebp
+	movl %esp, %ebp
+	pushl %eax
 	call do_IRQ
+	addl $12, %esp
 	jmp ret_from_intr
 
 #define BUILD_INTERRUPT(name, nr)	\

--- linux-2.6.1/arch/i386/kernel/i386-stub.c	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/i386-stub.c	2004-01-12 19:11:34.000000000 +0530
@@ -0,0 +1,457 @@
...and i386 dependend parts go here...

--- linux-2.6.1/arch/i386/kernel/Makefile	2004-01-10 11:01:35.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/Makefile	2004-01-12 19:11:34.000000000 +0530
@@ -31,6 +31,7 @@ obj-y				+= sysenter.o vsyscall.o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat.o
 obj-$(CONFIG_HPET_TIMER) 	+= time_hpet.o
 obj-$(CONFIG_EFI) 		+= efi.o efi_stub.o
+obj-$(CONFIG_KGDB)		+= i386-stub.o
 
 EXTRA_AFLAGS   := -traditional
 

--- linux-2.6.1/arch/i386/kernel/nmi.c	2003-11-24 07:02:02.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/nmi.c	2004-01-12 19:11:34.000000000 +0530
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/nmi.h>
 #include <linux/sysdev.h>
+#include <linux/debugger.h>
 
 #include <asm/smp.h>
 #include <asm/mtrr.h>
@@ -420,14 +421,25 @@ void nmi_watchdog_tick (struct pt_regs *
 	int sum, cpu = smp_processor_id();
 
 	sum = irq_stat[cpu].apic_timer_irqs;
+	if (atomic_read(&debugger_active)) {
 
-	if (last_irq_sums[cpu] == sum) {
+		/*
+		 * The machine is in debugger, hold this cpu if already
+		 * not held.
+		 */
+		debugger_nmihook(cpu, regs);
+		alert_counter[cpu] = 0;
+
+	} else if (last_irq_sums[cpu] == sum) {
 		/*
 		 * Ayiee, looks like this CPU is stuck ...
 		 * wait a few IRQs (5 seconds) before doing the oops ...
 		 */
 		alert_counter[cpu]++;
 		if (alert_counter[cpu] == 5*nmi_hz) {
+
+			CHK_DEBUGGER(2,SIGSEGV,0,regs,)
+
 			spin_lock(&nmi_print_lock);
 			/*
 			 * We are in trouble anyway, lets at least try

--- linux-2.6.1/arch/i386/kernel/signal.c	2003-11-24 07:01:52.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/signal.c	2004-01-12 19:11:34.000000000 +0530
@@ -580,7 +580,8 @@ int do_signal(struct pt_regs *regs, sigs
 		 * have been cleared if the watchpoint triggered
 		 * inside the kernel.
 		 */
-		__asm__("movl %0,%%db7"	: : "r" (current->thread.debugreg[7]));
+		if (current->thread.debugreg[7])
+			__asm__("movl %0,%%db7"	: : "r" (current->thread.debugreg[7]));
 
 		/* Whee!  Actually deliver the signal.  */
 		handle_signal(signr, &info, oldset, regs);

--- linux-2.6.1/arch/i386/kernel/traps.c	2003-11-24 07:01:14.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/kernel/traps.c	2004-01-12 19:11:34.000000000 +0530
@@ -51,6 +51,7 @@
 
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/debugger.h>
 
 #include "mach_traps.h"
 
@@ -256,6 +257,7 @@ void die(const char * str, struct pt_reg
 {
 	static int die_counter;
 
+	CHK_DEBUGGER(1,SIGTRAP,err,regs,)
 	console_verbose();
 	spin_lock_irq(&die_lock);
 	bust_spinlocks(1);
@@ -330,6 +332,7 @@ static inline void do_trap(int trapnr, i
 #define DO_ERROR(trapnr, signr, str, name) \
 asmlinkage void do_##name(struct pt_regs * regs, long error_code) \
 { \
+	CHK_DEBUGGER(trapnr,signr,error_code,regs,)\
 	do_trap(trapnr, signr, str, 0, regs, error_code, NULL); \
 }
 
@@ -347,7 +350,10 @@ asmlinkage void do_##name(struct pt_regs
 #define DO_VM86_ERROR(trapnr, signr, str, name) \
 asmlinkage void do_##name(struct pt_regs * regs, long error_code) \
 { \
+	CHK_DEBUGGER(trapnr,signr,error_code,regs,goto skip_trap)\
 	do_trap(trapnr, signr, str, 1, regs, error_code, NULL); \
+skip_trap: \
+	return; \
 }
 
 #define DO_VM86_ERROR_INFO(trapnr, signr, str, name, sicode, siaddr) \
@@ -547,7 +553,7 @@ asmlinkage void do_debug(struct pt_regs 
 	tsk->thread.debugreg[6] = condition;
 
 	/* Mask out spurious TF errors due to lazy TF clearing */
-	if (condition & DR_STEP) {
+	if (condition & DR_STEP && !debugger_step) {
 		/*
 		 * The TF error should be masked out only if the current
 		 * process is not traced and if the TRAP flag has been set
@@ -570,11 +576,13 @@ asmlinkage void do_debug(struct pt_regs 
 	info.si_errno = 0;
 	info.si_code = TRAP_BRKPT;
 	
-	/* If this is a kernel mode trap, save the user PC on entry to 
-	 * the kernel, that's what the debugger can make sense of.
-	 */
-	info.si_addr = ((regs->xcs & 3) == 0) ? (void *)tsk->thread.eip : 
-	                                        (void *)regs->eip;
+
+	/* If this is a kernel mode trap, we need to reset db7 to allow us
+	 * to continue sanely */
+	if ((regs->xcs & 3) == 0)
+		goto clear_dr7;
+
+	info.si_addr = (void *)regs->eip;
 	force_sig_info(SIGTRAP, &info, tsk);
 
 	/* Disable additional traps. They'll be re-enabled when
@@ -584,6 +592,7 @@ clear_dr7:
 	__asm__("movl %0,%%db7"
 		: /* no output */
 		: "r" (0));
+	CHK_DEBUGGER(1,SIGTRAP,error_code,regs,)
 	return;
 
 debug_vm86:

--- linux-2.6.1/arch/i386/mm/fault.c	2003-12-26 18:33:55.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/arch/i386/mm/fault.c	2004-01-12 19:11:35.000000000 +0530
@@ -2,6 +2,11 @@
  *  linux/arch/i386/mm/fault.c
  *
  *  Copyright (C) 1995  Linus Torvalds
+ *
+ *  Change History
+ *
+ *	Tigran Aivazian <tigran@sco.com>	Remote debugging support.
+ *
  */
 
 #include <linux/signal.h>
@@ -21,6 +26,7 @@
 #include <linux/vt_kern.h>		/* For unblank_screen() */
 #include <linux/highmem.h>
 #include <linux/module.h>
+#include <linux/debugger.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -262,6 +268,12 @@ asmlinkage void do_page_fault(struct pt_
 	if (in_atomic() || !mm)
 		goto bad_area_nosemaphore;
 
+	if (debugger_memerr_expected) {
+		/* This fault was caused by memory access through a debugger.
+		 * Don't handle it like user accesses */
+		goto no_context;
+	}
+
 	down_read(&mm->mmap_sem);
 
 	vma = find_vma(mm, address);
@@ -399,6 +411,8 @@ no_context:
  	if (is_prefetch(regs, address))
  		return;
 
+	CHK_DEBUGGER(14, SIGSEGV,error_code, regs,)
+
 /*
  * Oops. The kernel tried to access some bad page. We'll have to
  * terminate things with extreme prejudice.

--- linux-2.6.1/include/asm-i386/kgdb.h	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/include/asm-i386/kgdb.h	2004-01-12 19:11:35.000000000 +0530
@@ -0,0 +1,49 @@
....i386-specific kgdb stuff....

--- linux-2.6.1/include/asm-i386/processor.h	2004-01-10 11:01:47.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-i386/include/asm-i386/processor.h	2004-01-12 19:11:35.000000000 +0530
@@ -425,6 +426,8 @@ struct thread_struct {
 	unsigned int		saved_fs, saved_gs;
 /* IO permissions */
 	unsigned long	*io_bitmap_ptr;
+	void *		debuggerinfo;
+
 };
 
 #define INIT_THREAD  {							\

--- linux-2.6.1/drivers/serial/8250.c	2004-01-10 11:01:45.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/8250.c	2004-01-12 19:10:45.000000000 +0530
@@ -1198,12 +1198,17 @@ static void serial8250_break_ctl(struct 
 	spin_unlock_irqrestore(&up->port.lock, flags);
 }
 
+static int released_irq = -1;
+
 static int serial8250_startup(struct uart_port *port)
 {
 	struct uart_8250_port *up = (struct uart_8250_port *)port;
 	unsigned long flags;
 	int retval;
 
+	if (up->port.irq == released_irq)
+		return -EBUSY;
+
 	up->capabilities = uart_config[up->port.type].flags;
 
 	if (up->port.type == PORT_16C950) {
@@ -1869,6 +1874,10 @@ static void __init serial8250_register_p
 	for (i = 0; i < UART_NR; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
 
+		if (up->port.irq == released_irq) {
+			continue;
+		}
+
 		up->port.line = i;
 		up->port.ops = &serial8250_pops;
 		init_timer(&up->timer);
@@ -2138,6 +2147,36 @@ void serial8250_resume_port(int line)
 	uart_resume_port(&serial8250_reg, &serial8250_ports[line].port);
 }
 
+/*
+ * Find all the ports using the given irq and shut them down.
+ * Result should be that the irq will be released.
+ * At most one irq can be released this way.
+ * Once an irq is released, any attempts to initialize a port with that irq
+ * will fail with EBUSY.
+ */
+
+int serial8250_release_irq(int irq)
+{
+        struct uart_8250_port *up;
+	int ttyS;
+
+	if (released_irq != -1) {
+		return 1;
+	}
+	released_irq = irq;
+	for (ttyS = 0; ttyS < UART_NR; ttyS++){
+		up =  &serial8250_ports[ttyS];
+		if (up->port.irq == irq && (irq_lists + irq)->head) {
+#ifdef CONFIG_DEBUG_SPINLOCK   /* ugly business... */
+			if(up->port.lock.magic != SPINLOCK_MAGIC)
+				spin_lock_init(&up->port.lock);
+#endif
+			serial8250_shutdown(&up->port);
+		}
+        }
+	return 0;
+}
+
 static int __init serial8250_init(void)
 {
 	int ret, i;
diff -Naurp linux-2.6.1/drivers/serial/Kconfig linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/Kconfig
--- linux-2.6.1/drivers/serial/Kconfig	2003-11-24 07:03:13.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/Kconfig	2004-01-12 19:10:45.000000000 +0530
@@ -6,6 +6,13 @@
 
 menu "Serial drivers"
 
+config KGDB_8250
+	bool "KGDB: On generic serial port (8250)"
+	depends on KGDB
+	help
+	  Uses generic serial port (8250) for kgdb. This is independent of the
+	  option 9250/16550 and compatible serial port.
+
 #
 # The new 8250/16550 serial drivers
 config SERIAL_8250

--- linux-2.6.1/drivers/serial/kgdb_8250.c	1970-01-01 05:30:00.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/kgdb_8250.c	2004-01-17 13:55:58.000000000 +0530
@@ -0,0 +1,408 @@
....second copy of serial driver I guess....

--- linux-2.6.1/drivers/serial/Makefile	2003-11-24 07:01:55.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/Makefile	2004-01-12 19:10:45.000000000 +0530
@@ -32,3 +32,5 @@ obj-$(CONFIG_SERIAL_COLDFIRE) += mcfseri
 obj-$(CONFIG_V850E_UART) += v850e_uart.o
 obj-$(CONFIG_SERIAL98) += serial98.o
 obj-$(CONFIG_SERIAL_PMACZILOG) += pmac_zilog.o
+
+obj-$(CONFIG_KGDB_8250) += kgdb_8250.o

--- linux-2.6.1/drivers/serial/serial_core.c	2004-01-10 11:01:45.000000000 +0530
+++ linux-2.6.1-kgdb-2.1.0-8250/drivers/serial/serial_core.c	2004-01-12 19:10:45.000000000 +0530
@@ -1209,7 +1209,11 @@ static void uart_set_termios(struct tty_
 static void uart_close(struct tty_struct *tty, struct file *filp)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->port;
+	struct uart_port *port;
+
+	if (!state)
+		return;
+	port = state->port;
 
 	BUG_ON(!kernel_locked());
 	DPRINTK("uart_close(%d) called\n", port->line);

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: kgdb support in vanilla 2.6.2

[George Anzinger]

Andrew Morton wrote:
~
>>>
>>>I wouldn't support inclusion of i386 kgdb until it has had a lot of
>>>cleanup, possible de-featuritisification and some thought has been applied
>>>to splitting it into arch and generic bits.  It's quite a lot of work.
>>> 
>>>
>>
>>Amit has started at least the third activity--he has split much of kgdb
>>into arch and generic bits.
> 
> 
> Yes.
> 
> 
>>Could you elaborate a little on the first two?
>>
>>What major kinds of cleanup are we talking about?  Style issues?
> 
> 
> Coding style compliance, reduction of ifdefs, etc.  Reduction of patch
> footprint.  There are a few features in the patch in -mm which I am not
> aware of anyone having used.
> 
> 
>>What features (or classes of features) do you find excessive?  Would
>>it be sufficient to add a few config items to control subfeatures
>>of kgdb?
>>
> 
> 
> People have added timestamping infrastructure, stack overflow testing and
> inbuilt assertion frameworks to various gdb stubs at various times.  We
> need to take a look at such things and really convice ourselves that
> they're worthwhile.  Personally, I'd only be interested in the basic stub.
> 
Well, I have NEVER had a stack overflow, but then it is only a few lines. 
Likewise the assertions I think can go in favor of BUG and friends.

The timestamping I added at a time when the alternative that was suggested was 
LTT, which, at the time did not even come close to touching what I needed to do. 
  The timestamp code is designed to catch things that can only be observed when 
running close to real speed.  Especially on SMP systems, relative arrival times 
at spinlocks and friends can be very useful.  Would there be less objection if 
we removed the configure option for it ;)

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: kgdb support in vanilla 2.6.2

[Pavel Machek]

Hi!

> >I wouldn't support inclusion of i386 kgdb until it has had a lot of
> >cleanup, possible de-featuritisification and some thought has been applied
> >to splitting it into arch and generic bits.  It's quite a lot of work.
> 
> Amit has started at least the third activity--he has split much of kgdb
> into arch and generic bits.
> 
> Could you elaborate a little on the first two?
> 
> What major kinds of cleanup are we talking about?  Style issues?
> 
> What features (or classes of features) do you find excessive?  Would
> it be sufficient to add a few config items to control subfeatures
> of kgdb?
> 
> These are not idle questions.  If the amount of work to get it ready
> for acceptance is tractable, I know a company that may be willing to
> pay to have the work done.

Well, I guess I know two more such companies. Seems like everyone and
their aunt want kgdb these days :-).
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Thu, Feb 05, 2004 at 12:01:33AM +0100, Pavel Machek wrote:

> Hi!
> 
> It seems that some kgdb support is in 2.6.2-linus:
> 
> +++ b/Documentation/sh/kgdb.txt Tue Feb  3 19:45:43 2004
> @@ -0,0 +1,179 @@
> +
> +This file describes the configuration and behavior of KGDB for the SH
> +kernel. Based on a description from Henry Bell <henry.bell@st.com>, it
> +has been modified to account for quirks in the current implementation.
> +
> 
> That's great, can we get i386 kgdb, too? Or at least amd64 kgdb
> ;-). [Or was it a mistake? It seems unlikely that kgdb could enter
> Linus tree without major flamewar...]

FWIW, there has been PPC32 KGDB support in kernel.org for ages.  OTOH,
I'm quite happy that SH kgdb support came in (mental note made to talk
to Henry about the KGDB merging stuffs).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: kgdb support in vanilla 2.6.2

[Paul Mundt]

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

On Wed, Feb 04, 2004 at 04:52:15PM -0700, Tom Rini wrote:
> > +++ b/Documentation/sh/kgdb.txt Tue Feb  3 19:45:43 2004
> > @@ -0,0 +1,179 @@
> > +
> > +This file describes the configuration and behavior of KGDB for the SH
> > +kernel. Based on a description from Henry Bell <henry.bell@st.com>, it
> > +has been modified to account for quirks in the current implementation.
> > +
> > 
> > That's great, can we get i386 kgdb, too? Or at least amd64 kgdb
> > ;-). [Or was it a mistake? It seems unlikely that kgdb could enter
> > Linus tree without major flamewar...]
> 
> FWIW, there has been PPC32 KGDB support in kernel.org for ages.  OTOH,
> I'm quite happy that SH kgdb support came in (mental note made to talk
> to Henry about the KGDB merging stuffs).
> 
The SH kgdb work is a combination of effort by Henry Bell and Jeremy Siegel,
(ST and MV both had their own versions, Jeremy did the sync work between
the two) neither of which have touched it since mid 2.4 or so when it was
first merged into the LinuxSH tree.

Getting the SH kgdb stuff updated is on my TODO list, I'd definitely be
interested in getting this stuff in sync with Amit's work as well. Any
pointers?


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: kgdb support in vanilla 2.6.2

[Tom Rini]

On Wed, Feb 04, 2004 at 07:17:33PM -0500, Paul Mundt wrote:
> On Wed, Feb 04, 2004 at 04:52:15PM -0700, Tom Rini wrote:
> > > +++ b/Documentation/sh/kgdb.txt Tue Feb  3 19:45:43 2004
> > > @@ -0,0 +1,179 @@
> > > +
> > > +This file describes the configuration and behavior of KGDB for the SH
> > > +kernel. Based on a description from Henry Bell <henry.bell@st.com>, it
> > > +has been modified to account for quirks in the current implementation.
> > > +
> > > 
> > > That's great, can we get i386 kgdb, too? Or at least amd64 kgdb
> > > ;-). [Or was it a mistake? It seems unlikely that kgdb could enter
> > > Linus tree without major flamewar...]
> > 
> > FWIW, there has been PPC32 KGDB support in kernel.org for ages.  OTOH,
> > I'm quite happy that SH kgdb support came in (mental note made to talk
> > to Henry about the KGDB merging stuffs).
> > 
> The SH kgdb work is a combination of effort by Henry Bell and Jeremy Siegel,
> (ST and MV both had their own versions, Jeremy did the sync work between
> the two) neither of which have touched it since mid 2.4 or so when it was
> first merged into the LinuxSH tree.
> 
> Getting the SH kgdb stuff updated is on my TODO list, I'd definitely be
> interested in getting this stuff in sync with Amit's work as well. Any
> pointers?

What Amit has is at http://kgdb.sf.net/
What I've done on top of this is at bk://ppc.bkbits.net/linux-2.6-kgdb
and http://www.codemonkey.org.uk/projects/bitkeeper/kgdb .

-- 
Tom Rini
http://gate.crashing.org/~trini/