KGDB 2.0.3 with fixes and development in ethernet interface

KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

Hi,

KGDB 2.0.3 is available at 
http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2

Ethernet interface still doesn't work. It responds to gdb for a couple of 
packets and then panics. gdb log for ethernet interface is pasted below.

It panics and enter kgdb_handle_exception recursively and silently. To see the 
panic on screen make kgdb_handle_exception return immediately if 
kgdb_connected is non-zero. 

Panic trace is pasted below. It panics in skb_release_data. Looks like skb 
handling will have to changed to be have kgdb specific buffers.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)


(gdb) set debug remote 1
(gdb) tar re udp:192.168.1.4:5001
warning: The remote protocol may be unreliable over UDP.
warning: Some events may be lost, rendering further debugging impossible.
Remote debugging using udp:192.168.1.4:5001
Sending packet: $Hc-1#09...Ack
Packet received: OK
Sending packet: $qC#b4...Ack
Timed out.
Timed out.



Jan 16 17:17:10 pc1 kernel: Unable to handle kernel paging request at virtual 
a
dress 39233134
Starting xinetd: Jan 16 17:17:10 pc1 kernel:  printing eip:
Jan 16 17:17:10 pc1 kernel: c0239074
Jan 16 17:17:10 pc1 kernel: *pde = 00000000
Jan 16 17:17:10 pc1 kernel: Oops: 0000 [#1]
Jan 16 17:17:11 pc1 kernel: CPU:    0
Jan 16 17:17:11 pc1 kernel: EIP:    0060:[<c0239074>]    Not tainted
Jan 16 17:17:11 pc1 kernel: EFLAGS: 00010283
Jan 16 17:17:11 pc1 kernel: EIP is at skb_release_data+0x44/0xb0               
:
Jan 16 17:17:11 pc1 kernel: eax: c7deb220   ebx: c7708c00   ecx: c7deb220   
edx
 39233134                                                                      
:
Jan 16 17:17:11 pc1 kernel: esi: 00000000   edi: c7deb1fb   ebp: c7707e24   
esp
 c7707e18
Jan 16 17:17:11 pc1 kernel: ds: 007b   es: 007b   ss: 0068                     
=
Jan 16 17:17:11 pc1 kernel: Process kgdbeth (pid: 833, threadinfo=c7706000 
task
c12946d0)
Jan 16 17:17:11 pc1 kernel: Stack: ffffffff c7708c00 c7708c00 c7707e38 
c02390f4
c7708c00 00000292 00000000
Jan 16 17:17:12 pc1 kernel:        c7707e50 c02391aa c7708c00 c7e8a600 
c7708c00
c7e8a600 c7707e78 c01f630e
Jan 16 17:17:12 pc1 kernel:        c7708c00 c72c2000 00000000 00000040 
c880d000
c7708c00 c031fbf2 c7deb1fb
Jan 16 17:17:12 pc1 kernel: Call Trace:
Jan 16 17:17:12 pc1 kernel:  [<c02390f4>] kfree_skbmem+0x14/0x30
Jan 16 17:17:12 pc1 kernel:  [<c02391aa>] __kfree_skb+0x9a/0x130
Jan 16 17:17:12 pc1 kernel:  [<c01f630e>] rtl8139_start_xmit+0x7e/0x110
Jan 16 17:17:12 pc1 kernel:  [<c01f3356>] write_buffer+0x256/0x2e0
Jan 16 17:17:12 pc1 kernel:  [<c01f3403>] kgdbeth_flush+0x23/0x30
Jan 16 17:17:12 pc1 kernel:  [<c0133fdd>] putpacket+0x14d/0x170
Jan 16 17:17:12 pc1 kernel:  [<c0133e26>] getpacket+0xd6/0x140
Jan 16 17:17:12 pc1 kernel:  [<c0134875>] kgdb_handle_exception+0x215/0xae0
Jan 16 17:17:12 pc1 kernel:  [<c011d660>] __call_console_drivers+0x60/0x70
Jan 16 17:17:12 pc1 kernel:  [<c010b910>] do_int3+0x0/0x100
Jan 16 17:17:12 pc1 kernel:  [<c010ba07>] do_int3+0xf7/0x100
Jan 16 17:17:12 pc1 kernel:  [<c010b129>] error_code+0x2d/0x38
Jan 16 17:17:12 pc1 kernel:  [<c01351f7>] breakpoint+0x17/0x20
Jan 16 17:17:12 pc1 kernel:  [<c01352b0>] kgdb_entry+0xa0/0xb0
Jan 16 17:17:12 pc1 kernel:  [<c01f3bed>] kgdbeth_thread+0x3d/0x70
Jan 16 17:17:12 pc1 kernel:  [<c01f3bb0>] kgdbeth_thread+0x0/0x70
Jan 16 17:17:12 pc1 kernel:  [<c0109009>] kernel_thread_helper+0x5/0xc
Jan 16 17:17:12 pc1 kernel:                                                    
4
Jan 16 17:17:12 pc1 kernel: Code: 8b 02 a9 00 08 00 00 75 17 8b 42 04 85 c0 74
5 ff 4a 04 0f

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> KGDB 2.0.3 is available at 
> http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> 
> Ethernet interface still doesn't work. It responds to gdb for a couple of 
> packets and then panics. gdb log for ethernet interface is pasted
> below.


++int kgdbeth_thread(void *data)
++{
++      struct net_device *ndev = (struct net_device *)data;
++      daemonize("kgdbeth");
++      while (!ndev->ip_ptr) {
++              schedule();
++      }
++      debugger_entry();
++      return 0;

Don't you need some locking around ndev->ip_ptr? [Okay, it probably
only matters on SMP, so it is not causing your problems..]



								Pavel

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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Christoph Hellwig]

On Fri, Jan 16, 2004 at 01:58:06PM +0100, Pavel Machek wrote:
> ++int kgdbeth_thread(void *data)
> ++{
> ++      struct net_device *ndev = (struct net_device *)data;
> ++      daemonize("kgdbeth");
> ++      while (!ndev->ip_ptr) {
> ++              schedule();
> ++      }
> ++      debugger_entry();
> ++      return 0;
> 
> Don't you need some locking around ndev->ip_ptr? [Okay, it probably
> only matters on SMP, so it is not causing your problems..]

Not to mention it should use a proper wait_event instead of this
really stupid loop.

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Friday 16 Jan 2004 7:37 pm, Christoph Hellwig wrote:
> On Fri, Jan 16, 2004 at 01:58:06PM +0100, Pavel Machek wrote:
> > ++int kgdbeth_thread(void *data)
> > ++{
> > ++      struct net_device *ndev = (struct net_device *)data;
> > ++      daemonize("kgdbeth");
> > ++      while (!ndev->ip_ptr) {
> > ++              schedule();
> > ++      }
> > ++      debugger_entry();
> > ++      return 0;
> >
> > Don't you need some locking around ndev->ip_ptr? [Okay, it probably
> > only matters on SMP, so it is not causing your problems..]
>
> Not to mention it should use a proper wait_event instead of this
> really stupid loop.

Yep. Will do that. This is just first version to get some thing going.

Things that'll have to be fixed before this is usable as a debugger.
1. Change skbuff handling to use kgdb-specific buffers when 
kgdb_handle_exception begins.
2. Get rid of this way of bringing up ethernet interface.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Friday 16 Jan 2004 6:28 pm, Pavel Machek wrote:
> Hi!
>
> > KGDB 2.0.3 is available at
> > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> >
> > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > packets and then panics. gdb log for ethernet interface is pasted
> > below.
>
> ++int kgdbeth_thread(void *data)
> ++{
> ++      struct net_device *ndev = (struct net_device *)data;
> ++      daemonize("kgdbeth");
> ++      while (!ndev->ip_ptr) {
> ++              schedule();
> ++      }
> ++      debugger_entry();
> ++      return 0;
>
> Don't you need some locking around ndev->ip_ptr? [Okay, it probably
> only matters on SMP, so it is not causing your problems..]

Yes. Some locking will be needed. I haven't yet figured out the exact sequence 
of function calls during configuration of an interface from userland.

Is there a hold-count kind of a thing on network interface components (like 
inodes, dentries)?

I am still using userland to bring an interface up. I guess it's best done 
inside the kernel instead of using notifications and spawning a thread. Then 
the interface would be usable much earlier.

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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> > > KGDB 2.0.3 is available at
> > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > >
> > > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > > packets and then panics. gdb log for ethernet interface is pasted
> > > below.
> >
> > ++int kgdbeth_thread(void *data)
> > ++{
> > ++      struct net_device *ndev = (struct net_device *)data;
> > ++      daemonize("kgdbeth");
> > ++      while (!ndev->ip_ptr) {
> > ++              schedule();
> > ++      }
> > ++      debugger_entry();
> > ++      return 0;
> >
> > Don't you need some locking around ndev->ip_ptr? [Okay, it probably
> > only matters on SMP, so it is not causing your problems..]
> 
> Yes. Some locking will be needed. I haven't yet figured out the exact sequence 
> of function calls during configuration of an interface from userland.
> 
> Is there a hold-count kind of a thing on network interface components (like 
> inodes, dentries)?
> 
> I am still using userland to bring an interface up. I guess it's best done 
> inside the kernel instead of using notifications and spawning a thread. Then 
> the interface would be usable much earlier.

So.. if it works, I'll ifconfig eth0 up, it will print "waiting for
kgdb to connect", I'll connect with gdb, and it will work?
								Pavel

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

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Andi Kleen]

On Fri, 16 Jan 2004 17:59:59 +0530
"Amit S. Kale" <amitkale@emsyssoft.com> wrote:

> Hi,
> 
> KGDB 2.0.3 is available at 
> http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> 
> Ethernet interface still doesn't work. It responds to gdb for a couple of 
> packets and then panics. gdb log for ethernet interface is pasted below.

Did you add the fix for netpoll Jim posted? 

-Andi

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Friday 16 Jan 2004 6:35 pm, Andi Kleen wrote:
> On Fri, 16 Jan 2004 17:59:59 +0530
>
> "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > Hi,
> >
> > KGDB 2.0.3 is available at
> > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> >
> > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > packets and then panics. gdb log for ethernet interface is pasted below.
>
> Did you add the fix for netpoll Jim posted?

I am not using netpoll (yet). My patch doesn't require any driver 
modifications, that the mm ethernet patch required.

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

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> > > KGDB 2.0.3 is available at
> > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > >
> > > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > > packets and then panics. gdb log for ethernet interface is pasted below.
> >
> > Did you add the fix for netpoll Jim posted?
> 
> I am not using netpoll (yet). My patch doesn't require any driver 
> modifications, that the mm ethernet patch required.

Does that mean that you are going to use netpoll, eventually?

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

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Friday 16 Jan 2004 8:15 pm, Pavel Machek wrote:
> Hi!
>
> > > > KGDB 2.0.3 is available at
> > > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > > >
> > > > Ethernet interface still doesn't work. It responds to gdb for a
> > > > couple of packets and then panics. gdb log for ethernet interface is
> > > > pasted below.
> > >
> > > Did you add the fix for netpoll Jim posted?
> >
> > I am not using netpoll (yet). My patch doesn't require any driver
> > modifications, that the mm ethernet patch required.
>
> Does that mean that you are going to use netpoll, eventually?

I don't know yet. I don't like the idea of a debugger using a lot of kernel 
code.  I am trying to implement kgdboe without netpoll and without making 
changes to ethernet drivers.

Perhaps I'll have to use netpoll eventually!
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!


> > > KGDB 2.0.3 is available at
> > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > >
> > > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > > packets and then panics. gdb log for ethernet interface is pasted below.
> >
> > Did you add the fix for netpoll Jim posted?
> 
> I am not using netpoll (yet). My patch doesn't require any driver 
> modifications, that the mm ethernet patch required.

int kgdbeth_event(struct notifier_block * self, unsigned long val,
void * data)
{
        if (strcmp(((struct net_device *)data)->name, "eth0")) {
                goto out;
        }
        if (val!= NETDEV_UP)
                goto out;

Do I read it correctly as "eth0 is not to be used for debugging"? So
if I only have eth0 here, I have to comment it out, right?

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

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Friday 16 Jan 2004 8:17 pm, Pavel Machek wrote:
> Hi!
>
> > > > KGDB 2.0.3 is available at
> > > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > > >
> > > > Ethernet interface still doesn't work. It responds to gdb for a
> > > > couple of packets and then panics. gdb log for ethernet interface is
> > > > pasted below.
> > >
> > > Did you add the fix for netpoll Jim posted?
> >
> > I am not using netpoll (yet). My patch doesn't require any driver
> > modifications, that the mm ethernet patch required.
>
> int kgdbeth_event(struct notifier_block * self, unsigned long val,
> void * data)
> {
>         if (strcmp(((struct net_device *)data)->name, "eth0")) {
>                 goto out;
>         }
>         if (val!= NETDEV_UP)
>                 goto out;
>
> Do I read it correctly as "eth0 is not to be used for debugging"? So
> if I only have eth0 here, I have to comment it out, right?

Hi Pavel,

No. It uses only "eth0" for debugging. If you have only eth0, it'll use that 
for debugging.

Bad code again. It should be using kgdb_netdev variable [after making it 
global].

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

Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> > int kgdbeth_event(struct notifier_block * self, unsigned long val,
> > void * data)
> > {
> >         if (strcmp(((struct net_device *)data)->name, "eth0")) {
> >                 goto out;
> >         }
> >         if (val!= NETDEV_UP)
> >                 goto out;
> >
> > Do I read it correctly as "eth0 is not to be used for debugging"? So
> > if I only have eth0 here, I have to comment it out, right?
> 
> No. It uses only "eth0" for debugging. If you have only eth0, it'll use that 
> for debugging.

Perhaps this is good idea? It should be documented
somewhere... Please apply,
								Pavel

--- /dev/null	2003-09-12 10:38:14.000000000 +0200
+++ linux/Documentation/kgdb/ethernet.txt	2004-01-16 16:43:34.000000000 +0100
@@ -0,0 +1,15 @@
+Some notes about kgdb over ethernet
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ 2004 Pavel Machek <pavel@suse.cz>
+
+Pass this on kernel commandline:
+
+	kgdbeth=interfacenum,localmac,listenport,remoteip,remotemac gdb
+
+Boot local machine. At the point where you enable eth0, machine will
+hang, waiting for remote gdb to connect. At that point, type this on
+remote machine:
+
+   $ gdb ./vmlinux
+   (gdb) target remote udp:HOSTNAME:6443


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

KGDB documentation [Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface]

[Amit S. Kale]

Hi All,

I extracted this part of Pavel the patch he had submitted for 2.0.3 and 
appended it to README file. Since Pavel has't noticed it, I am assuming that 
most people won't notice it either.

Do people think pushing documentation into Documentation/kgdb directory is a 
better idea?

Another note about kgdb documentation - 
There is a lot of documentation at kgdb.sourceforge.net. It's more of howto 
type rather than manpages. Will it be too much as a documentation in kernel 
sources.

Any ideas on which things to put into Documentation/kgdb and which to have on 
a website.

On Friday 16 Jan 2004 9:22 pm, Pavel Machek wrote:
> Hi!
>
> > > int kgdbeth_event(struct notifier_block * self, unsigned long val,
> > > void * data)
> > > {
> > >         if (strcmp(((struct net_device *)data)->name, "eth0")) {
> > >                 goto out;
> > >         }
> > >         if (val!= NETDEV_UP)
> > >                 goto out;
> > >
> > > Do I read it correctly as "eth0 is not to be used for debugging"? So
> > > if I only have eth0 here, I have to comment it out, right?
> >
> > No. It uses only "eth0" for debugging. If you have only eth0, it'll use
> > that for debugging.
>
> Perhaps this is good idea? It should be documented
> somewhere... Please apply,
> 								Pavel
>
> --- /dev/null	2003-09-12 10:38:14.000000000 +0200
> +++ linux/Documentation/kgdb/ethernet.txt	2004-01-16 16:43:34.000000000
> +0100 @@ -0,0 +1,15 @@
> +Some notes about kgdb over ethernet
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> + 2004 Pavel Machek <pavel@suse.cz>
> +
> +Pass this on kernel commandline:
> +
> +	kgdbeth=interfacenum,localmac,listenport,remoteip,remotemac gdb
> +
> +Boot local machine. At the point where you enable eth0, machine will
> +hang, waiting for remote gdb to connect. At that point, type this on
> +remote machine:
> +
> +   $ gdb ./vmlinux
> +   (gdb) target remote udp:HOSTNAME:6443

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

Re: KGDB documentation [Re: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface]

[George Anzinger]

Amit S. Kale wrote:
> Hi All,
> 
> I extracted this part of Pavel the patch he had submitted for 2.0.3 and 
> appended it to README file. Since Pavel has't noticed it, I am assuming that 
> most people won't notice it either.
> 
> Do people think pushing documentation into Documentation/kgdb directory is a 
> better idea?
> 
> Another note about kgdb documentation - 
> There is a lot of documentation at kgdb.sourceforge.net. It's more of howto 
> type rather than manpages. Will it be too much as a documentation in kernel 
> sources.
> 
> Any ideas on which things to put into Documentation/kgdb and which to have on 
> a website.

It should all be in the kernel tree.  See, for example, the mm patch.

-g

-- 
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: [discuss] KGDB 2.0.3 with fixes and development in ethernet interface

[Matt Mackall]

On Fri, Jan 16, 2004 at 07:51:51PM +0530, Amit S. Kale wrote:
> On Friday 16 Jan 2004 6:35 pm, Andi Kleen wrote:
> > On Fri, 16 Jan 2004 17:59:59 +0530
> >
> > "Amit S. Kale" <amitkale@emsyssoft.com> wrote:
> > > Hi,
> > >
> > > KGDB 2.0.3 is available at
> > > http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> > >
> > > Ethernet interface still doesn't work. It responds to gdb for a couple of
> > > packets and then panics. gdb log for ethernet interface is pasted below.
> >
> > Did you add the fix for netpoll Jim posted?
> 
> I am not using netpoll (yet). My patch doesn't require any driver 
> modifications, that the mm ethernet patch required.

I went the no-modified-drivers route originally and a long discussion
with Jeff Garzik eventually convinced me of the error of my ways.
There are a bunch of paths that are racy if you try to make a generic
poll function. 

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> KGDB 2.0.3 is available at 
> http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> 
> Ethernet interface still doesn't work. It responds to gdb for a couple of 
> packets and then panics. gdb log for ethernet interface is pasted below.
> 
> It panics and enter kgdb_handle_exception recursively and silently. To see the 
> panic on screen make kgdb_handle_exception return immediately if 
> kgdb_connected is non-zero. 
> 
> Panic trace is pasted below. It panics in skb_release_data. Looks like skb 
> handling will have to changed to be have kgdb specific buffers.

Here are some cleanups. Added statics where possible, killed
superfluous volatile (what should it be good for?), changed calling
convention from 0 on success, 1 on failure to 0 on success, -ERRNO on
fail to be more consistent with rest of kernel, KGDB_DEG was not used
-> killed, fixed some comments to be more linux-like, and added
KERN_CRIT loglevel to "waiting for connection from remote gdb" -- to
ensure user sees the message.

Please apply,
								Pavel

--- tmp/linux/drivers/net/kgdb_eth.c	2004-01-16 17:00:07.000000000 +0100
+++ linux/drivers/net/kgdb_eth.c	2004-01-16 16:56:10.000000000 +0100
@@ -10,6 +10,7 @@
  *
  * Restructured for generic a gdb interface 
  * 	by Amit S. Kale <amitkale@emsyssoft.com>
+ * Some cleanups by Pavel Machek <pavel@suse.cz>
  */
 
 #include <linux/module.h>
@@ -47,10 +48,10 @@
 #define CHUNKSIZE 30
 #define MAXINCHUNK (CHUNKSIZE + 8)
 
-static char	kgdb_buf[GDB_BUF_SIZE] ;
-static int	kgdb_buf_in_inx ;
-static atomic_t	kgdb_buf_in_cnt ;
-static int	kgdb_buf_out_inx ;
+static char	kgdb_buf[GDB_BUF_SIZE];
+static int	kgdb_buf_in_inx;
+static atomic_t	kgdb_buf_in_cnt;
+static int	kgdb_buf_out_inx;
 
 static unsigned int	kgdb_remoteip = 0;
 static unsigned short	kgdb_listenport = 6443;
@@ -59,11 +60,12 @@
 static unsigned char	kgdb_remotemac[6] = {0xff,0xff,0xff,0xff,0xff,0xff};
 static unsigned char	kgdb_localmac[6] = {0xff,0xff,0xff,0xff,0xff,0xff};
 
-struct net_device *kgdb_netdevice = NULL;
-char		kgdbeth_sendbuf[MAXINCHUNK];
-int		kgdbeth_sendbufchars;
-irqreturn_t (*kgdbeth_irqhandler)(int, void *, struct pt_regs *) = NULL;
+static char		kgdbeth_sendbuf[MAXINCHUNK];
+static int		kgdbeth_sendbufchars;
+static irqreturn_t	(*kgdbeth_irqhandler)(int, void *, struct pt_regs *) = NULL;
+
 int		kgdbeth_is_trapped;
+struct net_device *kgdb_netdevice = NULL;
 
 /*
  * Get a char if available, return -1 if nothing available.
@@ -76,9 +78,9 @@
 	if (atomic_read(&kgdb_buf_in_cnt) != 0) {
 		int chr;
 
-		chr = kgdb_buf[kgdb_buf_out_inx++] ;
-		kgdb_buf_out_inx &= (GDB_BUF_SIZE - 1) ;
-		atomic_dec(&kgdb_buf_in_cnt) ;
+		chr = kgdb_buf[kgdb_buf_out_inx++];
+		kgdb_buf_out_inx &= (GDB_BUF_SIZE - 1);
+		atomic_dec(&kgdb_buf_in_cnt);
 		return chr;
 	}
 
@@ -382,7 +384,7 @@
 	extern void kgdb_respond_ok(void);
 	struct irqaction *ia_ptr;
 
-	sprintf(kgdb_netdev,"eth%d",kgdb_eth);
+	sprintf(kgdb_netdev, "eth%d", kgdb_eth);
 
 	for (kgdb_netdevice = dev_base;
 		kgdb_netdevice != NULL;
@@ -392,7 +394,7 @@
 		}
 	}
 	if (!kgdb_netdevice) {
-		printk("kgdbeth: Unable to find interface %s\n",kgdb_netdev);
+		printk("kgdbeth: Unable to find interface %s\n", kgdb_netdev);
 		return -1;
 	}
 	if (!(kgdb_netdevice->flags & IFF_UP)) {
@@ -418,7 +420,6 @@
 /*
  * Poll an ethernet interface for incoming characters
  */
-
 static void kgdbeth_rx_poll(void)
 {
 	if (!kgdbeth_is_trapped)
@@ -439,7 +440,7 @@
 static int
 kgdbeth_read_char(void)
 {
-	volatile int	chr;
+	int	chr;
 
 	while ((chr = read_char()) < 0) {
 		if (send_skb) {
@@ -488,12 +489,10 @@
 			(*ptr)++;
 		while((**ptr != delimiter) && (*(*ptr -1) != ':'));
 		if (i > 6)
-			break;
-	}
-	if (i != 6) {
-		return 1;
+			return -EINVAL;
 	}
-
+	if (i != 6)
+		return -EINVAL;
 	return 0;
 }
 
@@ -579,7 +578,6 @@
 		goto errout;
 
 	kgdb_serial = &kgdbeth_serial;
-
 	return 1;
 
 errout:
--- tmp/linux/kernel/kgdbstub.c	2004-01-16 17:00:07.000000000 +0100
+++ linux/kernel/kgdbstub.c	2004-01-16 16:56:51.000000000 +0100
@@ -8,13 +8,13 @@
  * WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
  */
 
 /*
  * Copyright (C) 2000-2001 VERITAS Software Corporation.
  * Copyright (C) 2002-2004 Timesys Corporation
  * Copyright (C) 2003-2004 Amit S. Kale
+ * Copyright (C) 2004 Pavel Machek <pavel@suse.cz>
  *
  * Restructured KGDB for 2.6 kernels.
  * thread support, support for multiple processors,support for ia-32(x86) 
@@ -31,8 +31,6 @@
  * Modified for 386 by Jim Kingdon, Cygnus Support.
  * Origianl kgdb, compatibility with 2.1.xx kernel by David Grothe <dave@gcom.com>
  * Integrated into 2.2.5 kernel by Tigran Aivazian <tigran@sco.com>
- *  
-*  
  */
 
 #include <linux/string.h>
@@ -50,18 +48,14 @@
 #include <linux/notifier.h>
 #include <linux/module.h>
 #include <asm/cacheflush.h>
- 
+
 #ifdef CONFIG_KGDB_CONSOLE
 #include <linux/console.h>
 #endif
  
 #include <linux/init.h>
 
-
-
-/* DEBUGGING THE DEBUGGER */
-#undef KGDB_DEG
-/**************************/
+
 
 struct kgdb_arch *kgdb_ops = &arch_kgdb_ops;
 gdb_breakpoint_t kgdb_break[MAX_BREAKPOINTS];
@@ -84,10 +78,8 @@
 volatile int kgdb_connected;
 struct task_struct *kgdb_usethread, *kgdb_contthread;
 
-
 int debugger_step;
 atomic_t debugger_active;
-volatile int debugger_memerr_expected;
 
 /* 
  * Indicate to caller of kgdb_mem2hex or hex2mem that there has been an
@@ -245,11 +237,12 @@
 
 }
 
-/* convert the memory pointed to by mem into hex, placing result in buf */
-/* return a pointer to the last char put in buf (null) */
-/* If MAY_FAULT is non-zero, then we should set kgdb_memerr in response to
-   a fault; if zero treat a fault like any other fault in the stub.  */
-
+/*
+ * convert the memory pointed to by mem into hex, placing result in buf
+ * return a pointer to the last char put in buf (null)
+ * If MAY_FAULT is non-zero, then we should set kgdb_memerr in response to
+ * a fault; if zero treat a fault like any other fault in the stub.
+ */
 char *kgdb_mem2hex(char *mem, char *buf, int count, int can_fault)
 {
 	int i;
@@ -285,8 +278,8 @@
 }
 
 /*
- * WHILE WE FIND NICE HEX CHARS, BUILD A LONG
- * RETURN NUMBER OF CHARS PROCESSED           
+ * While we find nice hex chars, build a longValue.
+ * Return number of chars processed.
  */
 int kgdb_hexToLong(char **ptr, long *longValue)
 {
@@ -388,7 +381,7 @@
 	procindebug[processor] = 1;
 	current->thread.debuggerinfo = regs;
 
-	/* Wait till master processor goes completely into the debugger */
+	/* Wait till master processor goes completely into the debugger. FIXME: this looks racy */
 	while (!procindebug[atomic_read(&debugger_active) - 1]) {
 		int i = 10;	/* an arbitrary number */
 
@@ -411,7 +404,6 @@
 	spin_unlock(slavecpulocks + processor);
 	local_irq_restore(flags);
 }
-
 #endif
 
 static void get_mem (char *addr, unsigned char *buf, int count)
@@ -551,7 +543,7 @@
 	return ret;
 }
 
-int shadow_pid(int realpid)
+static inline int shadow_pid(int realpid)
 {
 	if (realpid) {
 		return realpid;
@@ -562,7 +554,6 @@
 /*
  * This function does all command procesing for interfacing to gdb.
  */
-
 int kgdb_handle_exception(int exVector, int signo, int err_code, 
                      struct pt_regs *linux_regs)
 {
@@ -1117,15 +1108,15 @@
 	/*
 	 * Call GDB routine to setup the exception vectors for the debugger
 	 */
-	set_debug_traps() ;
+	set_debug_traps();
 
 	/*
 	 * Call the breakpoint() routine in GDB to start the debugging
 	 * session.
 	 */
-	printk("Waiting for connection from remote gdb... ");
+	printk(KERN_CRIT "Waiting for connection from remote gdb... ");
 	breakpoint() ;
-	printk("Connected.\n");
+	printk(KERN_CRIT "Connected.\n");
 }
 
 #ifdef CONFIG_KGDB_CONSOLE

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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Amit S. Kale wrote:
> Hi,
> 
> KGDB 2.0.3 is available at 
> http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> 
> Ethernet interface still doesn't work. It responds to gdb for a couple of 
> packets and then panics. gdb log for ethernet interface is pasted below.
> 
> It panics and enter kgdb_handle_exception recursively and silently. To see the 
> panic on screen make kgdb_handle_exception return immediately if 
> kgdb_connected is non-zero. 
> 
> Panic trace is pasted below. It panics in skb_release_data. Looks like skb 
> handling will have to changed to be have kgdb specific buffers.

Well, this is one step to independence.  That would imply a small memory 
allocator in kgdb itself.  A good thing IMHO.

-- 
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 2.0.3 with fixes and development in ethernet interface

[Pavel Machek]

Hi!

> KGDB 2.0.3 is available at 
> http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> 
> Ethernet interface still doesn't work. It responds to gdb for a couple of 
> packets and then panics. gdb log for ethernet interface is pasted below.
> 
> It panics and enter kgdb_handle_exception recursively and silently. To see the 
> panic on screen make kgdb_handle_exception return immediately if 
> kgdb_connected is non-zero. 
> 
> Panic trace is pasted below. It panics in skb_release_data. Looks like skb 
> handling will have to changed to be have kgdb specific buffers.

This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
compile error on x86-64).
								Pavel

--- linux/kernel/kgdbstub.c	2004-01-16 16:56:51.000000000 +0100
+++ linux-cvs/kernel/kgdbstub.c	2004-01-16 20:11:45.000000000 +0100
@@ -1178,7 +1178,9 @@
 #endif
 
 EXPORT_SYMBOL(breakpoint);
+#ifdef CONFIG_KGDB_THREAD
 EXPORT_SYMBOL(kern_schedule);
+#endif
 
 static int __init opt_gdb(char *str)
 {


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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Pavel Machek wrote:
> Hi!
> 
> 
>>KGDB 2.0.3 is available at 
>>http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
>>
>>Ethernet interface still doesn't work. It responds to gdb for a couple of 
>>packets and then panics. gdb log for ethernet interface is pasted below.
>>
>>It panics and enter kgdb_handle_exception recursively and silently. To see the 
>>panic on screen make kgdb_handle_exception return immediately if 
>>kgdb_connected is non-zero. 
>>
>>Panic trace is pasted below. It panics in skb_release_data. Looks like skb 
>>handling will have to changed to be have kgdb specific buffers.
> 
> 
> This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
> compile error on x86-64).

Amit, could you explain why this is an option?  It seems very useful and not 
really too much code...

-- 
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 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
> Pavel Machek wrote:
> > Hi!
> >
> >>KGDB 2.0.3 is available at
> >>http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> >>
> >>Ethernet interface still doesn't work. It responds to gdb for a couple of
> >>packets and then panics. gdb log for ethernet interface is pasted below.
> >>
> >>It panics and enter kgdb_handle_exception recursively and silently. To
> >> see the panic on screen make kgdb_handle_exception return immediately if
> >> kgdb_connected is non-zero.
> >>
> >>Panic trace is pasted below. It panics in skb_release_data. Looks like
> >> skb handling will have to changed to be have kgdb specific buffers.
> >
> > This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
> > compile error on x86-64).
>
> Amit, could you explain why this is an option?  It seems very useful and
> not really too much code...

It saves all registers before switch_to. It does that two times at present. 
Once (implicit) register save by gcc and an explicit register save in 
arch/<proc>/kernel/entry.S Second register save in kern_schedule generates a 
pt_regs structure which gdb can get all registers at once from. If it's 
omited, gdb has to figure out where gcc has saved registers from the 
non-standards code in switch_to, which it can't do correctly all the time.

We can have a check for (a new variable) debugger_enabled before calling 
kern_schedule. That'll be add negligible overhead and will allow extra thread 
info to be saved only when a debugger is enabled. There will not be any need 
for CONFIG_KGDB_THREAD also.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Amit S. Kale wrote:
> On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
> 
>>Pavel Machek wrote:
>>
>>>Hi!
>>>
>>>
>>>>KGDB 2.0.3 is available at
>>>>http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
>>>>
>>>>Ethernet interface still doesn't work. It responds to gdb for a couple of
>>>>packets and then panics. gdb log for ethernet interface is pasted below.
>>>>
>>>>It panics and enter kgdb_handle_exception recursively and silently. To
>>>>see the panic on screen make kgdb_handle_exception return immediately if
>>>>kgdb_connected is non-zero.
>>>>
>>>>Panic trace is pasted below. It panics in skb_release_data. Looks like
>>>>skb handling will have to changed to be have kgdb specific buffers.
>>>
>>>This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
>>>compile error on x86-64).
>>
>>Amit, could you explain why this is an option?  It seems very useful and
>>not really too much code...
> 
> 
> It saves all registers before switch_to. It does that two times at present. 
> Once (implicit) register save by gcc and an explicit register save in 
> arch/<proc>/kernel/entry.S Second register save in kern_schedule generates a 
> pt_regs structure which gdb can get all registers at once from. If it's 
> omited, gdb has to figure out where gcc has saved registers from the 
> non-standards code in switch_to, which it can't do correctly all the time.
> 
> We can have a check for (a new variable) debugger_enabled before calling 
> kern_schedule. That'll be add negligible overhead and will allow extra thread 
> info to be saved only when a debugger is enabled. There will not be any need 
> for CONFIG_KGDB_THREAD also.

I don't seem to have such a problem with the mm kgdb.  No kern_schedule there...

-- 
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 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Sunday 18 Jan 2004 1:24 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
> >>Pavel Machek wrote:
> >>>Hi!
> >>>
> >>>>KGDB 2.0.3 is available at
> >>>>http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.0.3.tar.bz2
> >>>>
> >>>>Ethernet interface still doesn't work. It responds to gdb for a couple
> >>>> of packets and then panics. gdb log for ethernet interface is pasted
> >>>> below.
> >>>>
> >>>>It panics and enter kgdb_handle_exception recursively and silently. To
> >>>>see the panic on screen make kgdb_handle_exception return immediately
> >>>> if kgdb_connected is non-zero.
> >>>>
> >>>>Panic trace is pasted below. It panics in skb_release_data. Looks like
> >>>>skb handling will have to changed to be have kgdb specific buffers.
> >>>
> >>>This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
> >>>compile error on x86-64).
> >>
> >>Amit, could you explain why this is an option?  It seems very useful and
> >>not really too much code...
> >
> > It saves all registers before switch_to. It does that two times at
> > present. Once (implicit) register save by gcc and an explicit register
> > save in arch/<proc>/kernel/entry.S Second register save in kern_schedule
> > generates a pt_regs structure which gdb can get all registers at once
> > from. If it's omited, gdb has to figure out where gcc has saved registers
> > from the non-standards code in switch_to, which it can't do correctly all
> > the time.
> >
> > We can have a check for (a new variable) debugger_enabled before calling
> > kern_schedule. That'll be add negligible overhead and will allow extra
> > thread info to be saved only when a debugger is enabled. There will not
> > be any need for CONFIG_KGDB_THREAD also.
>
> I don't seem to have such a problem with the mm kgdb.  No kern_schedule
> there...


Have you confirmed that you see correct values for all the registers? I had 
found some problems with gdb not being able to locate all the registers 
correctly. Explanation on the problem below:

Besides getting gdb's fault there  is one more benefit of kern_schedule. All 
threads are shown by gdb as sleeping _at_ the place where schedule call is 
present: So instead of gdb showing all threads sleeping in __switch_to, it 
shows several functions. That's better than having to look at back trace of 
each thread to figure out what it is.

This functionality is complemented by printing of thread names in 2.0.X kgdb 
info threads listing. 

Now back to gdb problem of not being able to locate registers.
schedule results in code of this form:

schedule:
framesetup
registers save
...
...
save registers
change esp
call switchto
restore registers
...
...

GDB can't analyze code other than frame setup and registers save. It may not 
show values of variables that are present in registers correctly. This used 
to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a better job.
hmm...
May be its time I should look at gdb's x86 register info code again.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Daniel Jacobowitz]

On Wed, Jan 21, 2004 at 07:16:48PM +0530, Amit S. Kale wrote:
> Now back to gdb problem of not being able to locate registers.
> schedule results in code of this form:
> 
> schedule:
> framesetup
> registers save
> ...
> ...
> save registers
> change esp
> call switchto
> restore registers
> ...
> ...
> 
> GDB can't analyze code other than frame setup and registers save. It may not 
> show values of variables that are present in registers correctly. This used 
> to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a better job.
> hmm...
> May be its time I should look at gdb's x86 register info code again.

You should try GDB 6.0, which will use the dwarf2 unwind information to
accurately locate registers in any GCC-compiled code with -gdwarf-2 (-g
on Linux targets).

As George is now painfully familiar with :)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Daniel Jacobowitz wrote:
> On Wed, Jan 21, 2004 at 07:16:48PM +0530, Amit S. Kale wrote:
> 
>>Now back to gdb problem of not being able to locate registers.
>>schedule results in code of this form:
>>
>>schedule:
>>framesetup
>>registers save
>>...
>>...
>>save registers
>>change esp
>>call switchto
>>restore registers
>>...
>>...
>>
>>GDB can't analyze code other than frame setup and registers save. It may not 
>>show values of variables that are present in registers correctly. This used 
>>to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a better job.
>>hmm...
>>May be its time I should look at gdb's x86 register info code again.
> 
> 
> You should try GDB 6.0, which will use the dwarf2 unwind information to
> accurately locate registers in any GCC-compiled code with -gdwarf-2 (-g
> on Linux targets).
> 
> As George is now painfully familiar with :)

Well, some of that may be in asm code which may need help.  Need to check this.
> 

-- 
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 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Thursday 22 Jan 2004 12:09 am, Daniel Jacobowitz wrote:
> On Wed, Jan 21, 2004 at 07:16:48PM +0530, Amit S. Kale wrote:
> > Now back to gdb problem of not being able to locate registers.
> > schedule results in code of this form:
> >
> > schedule:
> > framesetup
> > registers save
> > ...
> > ...
> > save registers
> > change esp
> > call switchto
> > restore registers
> > ...
> > ...
> >
> > GDB can't analyze code other than frame setup and registers save. It may
> > not show values of variables that are present in registers correctly.
> > This used to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a
> > better job. hmm...
> > May be its time I should look at gdb's x86 register info code again.
>
> You should try GDB 6.0, which will use the dwarf2 unwind information to
> accurately locate registers in any GCC-compiled code with -gdwarf-2 (-g
> on Linux targets).

Here is the code generated for function schedule. #APP where assembly inline 
code is emitted. I believe gdb has correct register information available 
from gcc upto .LBB197. kgdb reports a thread having stopped at label 1. 
eax->edx aren't saved explicitely, so we can't have debugging info telling it 
how they are saved. Figuring out eax->edx is of lesser importance as it's the 
function that calls schedule that we are usually interested in. If gdb gets 
ebp right, that can be done corretly. GDB 5.2 had a difficulties walking up a 
stack in absence of ebp. GDB 6.0 can work with just esp, I believe.

.LBE195:
	movl	104(%ebx), %eax
	testl	%eax, %eax
	je	.L466
.L421:
	.loc 1 823 0
.LBB197:
	movl	%ebx, %eax
	movl	%esi, %edx
#APP
	pushfl
	pushl %ebp
	movl %esp,852(%ebx)
	movl 852(%esi),%esp
	movl $1f,848(%ebx)
	pushl 848(%esi)
	jmp __switch_to
1:	popl %ebp
	popfl
#NO_APP
	movl	%eax, -48(%ebp)
	.loc 1 1592 0

I guess we can keep the CONFIG_KGDB_THREAD code in entry.S optional only, 
which should be enabled only if one wants to debug function schedule. It 
shouldn't be enabled on i386.

CONFIG_KGDB_THREAD save code will still be required on other processors. 
Powerpc for example has _switch assembly function that does a lot of things, 
including saving registers on stack.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Daniel Jacobowitz]

On Thu, Jan 22, 2004 at 11:19:23AM +0530, Amit S. Kale wrote:
> CONFIG_KGDB_THREAD save code will still be required on other processors. 
> Powerpc for example has _switch assembly function that does a lot of things, 
> including saving registers on stack.

Then this can also be described using dwarf2 annotation.  This is
precisely what it's for.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Amit S. Kale wrote:
> On Sunday 18 Jan 2004 1:24 am, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
>>>
:

>>>>>This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
>>>>>compile error on x86-64).
>>>>
>>>>Amit, could you explain why this is an option?  It seems very useful and
>>>>not really too much code...
>>>
>>>It saves all registers before switch_to. It does that two times at
>>>present. Once (implicit) register save by gcc and an explicit register
>>>save in arch/<proc>/kernel/entry.S Second register save in kern_schedule
>>>generates a pt_regs structure which gdb can get all registers at once
>>>from. If it's omited, gdb has to figure out where gcc has saved registers
>>>from the non-standards code in switch_to, which it can't do correctly all
>>>the time.
>>>
>>>We can have a check for (a new variable) debugger_enabled before calling
>>>kern_schedule. That'll be add negligible overhead and will allow extra
>>>thread info to be saved only when a debugger is enabled. There will not
>>>be any need for CONFIG_KGDB_THREAD also.
>>
>>I don't seem to have such a problem with the mm kgdb.  No kern_schedule
>>there...
> 
> 
> 
> Have you confirmed that you see correct values for all the registers? I had 
> found some problems with gdb not being able to locate all the registers 
> correctly. Explanation on the problem below:
> 
> Besides getting gdb's fault there  is one more benefit of kern_schedule. All 
> threads are shown by gdb as sleeping _at_ the place where schedule call is 
> present: So instead of gdb showing all threads sleeping in __switch_to, it 
> shows several functions. That's better than having to look at back trace of 
> each thread to figure out what it is.

The mm version of kgdb does this, but by lying to gdb during the info thread 
command.  IMNSHO, the real way to do this is to process the info thread output 
with an awk script, but I haven't done that just yet.  The problem with lying to 
gdb is that it sometimes remembers...
> 
> This functionality is complemented by printing of thread names in 2.0.X kgdb 
> info threads listing. 

as in the mm version.
> 
> Now back to gdb problem of not being able to locate registers.
> schedule results in code of this form:
> 
> schedule:
> framesetup
> registers save
> ...
> ...
> save registers
> change esp
> call switchto
> restore registers
> ...
I have not analyzed this as yet.  However, it does seem to me to be the same 
problem as trying to bt through an interrupt frame.  The correct way to do this 
is to build the dwarf frame descriptors.  I have done this for the interrupt 
frame and intend to send said patch out in a day or so.

Could be I should look at the above and do the right thing for it also.
> ...
> 
> GDB can't analyze code other than frame setup and registers save. It may not 
> show values of variables that are present in registers correctly. This used 
> to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a better job.
> hmm...
> May be its time I should look at gdb's x86 register info code again.

The latter gdbs (not sure when) are using dwarf frame code.  The asm code needs 
to be anotated to make this work.  There is a problem I reported a couple of 
days ago with the dwarf expression code.  I think it is fixed, but that would 
still be the cvs version of gdb.

I think I would rather NOT modify kernel code and put up with the register loss 
in some cases.  I am MUCH more concerned with unexpected behavior due to the 
code changes.  Heisenberg is NOT our friend.

-- 
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 2.0.3 with fixes and development in ethernet interface

[Amit S. Kale]

On Thursday 22 Jan 2004 4:30 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > On Sunday 18 Jan 2004 1:24 am, George Anzinger wrote:
> >>Amit S. Kale wrote:
> >>>On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
> >>>>>This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
> >>>>>compile error on x86-64).
> >>>>
> >>>>Amit, could you explain why this is an option?  It seems very useful
> >>>> and not really too much code...
> >>>
> >>>It saves all registers before switch_to. It does that two times at
> >>>present. Once (implicit) register save by gcc and an explicit register
> >>>save in arch/<proc>/kernel/entry.S Second register save in kern_schedule
> >>>generates a pt_regs structure which gdb can get all registers at once
> >>>from. If it's omited, gdb has to figure out where gcc has saved
> >>> registers from the non-standards code in switch_to, which it can't do
> >>> correctly all the time.
> >>>
> >>>We can have a check for (a new variable) debugger_enabled before calling
> >>>kern_schedule. That'll be add negligible overhead and will allow extra
> >>>thread info to be saved only when a debugger is enabled. There will not
> >>>be any need for CONFIG_KGDB_THREAD also.
> >>
> >>I don't seem to have such a problem with the mm kgdb.  No kern_schedule
> >>there...
> >
> > Have you confirmed that you see correct values for all the registers? I
> > had found some problems with gdb not being able to locate all the
> > registers correctly. Explanation on the problem below:
> >
> > Besides getting gdb's fault there  is one more benefit of kern_schedule.
> > All threads are shown by gdb as sleeping _at_ the place where schedule
> > call is present: So instead of gdb showing all threads sleeping in
> > __switch_to, it shows several functions. That's better than having to
> > look at back trace of each thread to figure out what it is.
>
> The mm version of kgdb does this, but by lying to gdb during the info
> thread command.  IMNSHO, the real way to do this is to process the info
> thread output with an awk script, but I haven't done that just yet.  The
> problem with lying to gdb is that it sometimes remembers...
>
> > This functionality is complemented by printing of thread names in 2.0.X
> > kgdb info threads listing.
>
> as in the mm version.
>
> > Now back to gdb problem of not being able to locate registers.
> > schedule results in code of this form:
> >
> > schedule:
> > framesetup
> > registers save
> > ...
> > ...
> > save registers
> > change esp
> > call switchto
> > restore registers
> > ...
>
> I have not analyzed this as yet.  However, it does seem to me to be the
> same problem as trying to bt through an interrupt frame.  The correct way
> to do this is to build the dwarf frame descriptors.  I have done this for
> the interrupt frame and intend to send said patch out in a day or so.

Great! I had to do it this ackward way:

i386 ->

 	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
 
Powerpc ->
 	int irq, first = 1;
+	unsigned long *lrptr;
+	if (!(regs->msr & MSR_PR)) {
+		/* Came in from the kernel. Save call link. */
+		lrptr = (unsigned long *)(regs->gpr[1] + 4);
+		*lrptr = regs->nip;
+	}
   
I guess your patch will fix this problem for i386 only. Any ideas on doing it 
for powerpc too?


>
> Could be I should look at the above and do the right thing for it also.
>
> > ...
> >
> > GDB can't analyze code other than frame setup and registers save. It may
> > not show values of variables that are present in registers correctly.
> > This used to be a problem some time ago (gdb 5.X). Perhaps gdb 6.x does a
> > better job. hmm...
> > May be its time I should look at gdb's x86 register info code again.
>
> The latter gdbs (not sure when) are using dwarf frame code.  The asm code
> needs to be anotated to make this work.  There is a problem I reported a
> couple of days ago with the dwarf expression code.  I think it is fixed,
> but that would still be the cvs version of gdb.
>
> I think I would rather NOT modify kernel code and put up with the register
> loss in some cases.  I am MUCH more concerned with unexpected behavior due
> to the code changes.  Heisenberg is NOT our friend.

Agreed. I am not happy with the CONFIG_KGDB_THREAD code in entry.S That's the 
reason I have kept it optional.

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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Tom Rini]

On Thu, Jan 22, 2004 at 10:39:14AM +0530, Amit S. Kale wrote:
> On Thursday 22 Jan 2004 4:30 am, George Anzinger wrote:
> > Amit S. Kale wrote:
> > > Now back to gdb problem of not being able to locate registers.
> > > schedule results in code of this form:
> > >
> > > schedule:
> > > framesetup
> > > registers save
> > > ...
> > > ...
> > > save registers
> > > change esp
> > > call switchto
> > > restore registers
> > > ...
> >
> > I have not analyzed this as yet.  However, it does seem to me to be the
> > same problem as trying to bt through an interrupt frame.  The correct way
> > to do this is to build the dwarf frame descriptors.  I have done this for
> > the interrupt frame and intend to send said patch out in a day or so.
> 
> Great! I had to do it this ackward way:
> 
> i386 ->
[snip]
> I guess your patch will fix this problem for i386 only. Any ideas on doing it 
> for powerpc too?

Maybe I'm missing something, but aside from having to re-write the
solution in PPC asm, if it's in i386 asm, why wouldn't this work for PPC
as well?

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

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[George Anzinger]

Tom Rini wrote:
> On Thu, Jan 22, 2004 at 10:39:14AM +0530, Amit S. Kale wrote:
> 
>>On Thursday 22 Jan 2004 4:30 am, George Anzinger wrote:
>>
>>>Amit S. Kale wrote:
>>>
>>>>Now back to gdb problem of not being able to locate registers.
>>>>schedule results in code of this form:
>>>>
>>>>schedule:
>>>>framesetup
>>>>registers save
>>>>...
>>>>...
>>>>save registers
>>>>change esp
>>>>call switchto
>>>>restore registers
>>>>...
>>>
>>>I have not analyzed this as yet.  However, it does seem to me to be the
>>>same problem as trying to bt through an interrupt frame.  The correct way
>>>to do this is to build the dwarf frame descriptors.  I have done this for
>>>the interrupt frame and intend to send said patch out in a day or so.
>>
>>Great! I had to do it this ackward way:
>>
>>i386 ->
> 
> [snip]
> 
>>I guess your patch will fix this problem for i386 only. Any ideas on doing it 
>>for powerpc too?
> 
> 
> Maybe I'm missing something, but aside from having to re-write the
> solution in PPC asm, if it's in i386 asm, why wouldn't this work for PPC
> as well?
> 
I think the asm is not the issue.  the only stuff used is constant and pointer 
generation code, no machine instructions.  All that would have to be done is to 
describe, in the dwarft language, the interrupt frame.  This is different for 
different archs so this is where the work would be needed.

Daniel has suggested that we could just use the new bin tools where in the gas 
program will build the call frames.  I am not sure it can handle expressions, 
however.  And they are needed if you want to tie off the frame if the next step 
is user land.

By the way, I don't try to build a blow by blow of the frame.  Rather I assume 
it is only of interest at those points where calls are made out of the interrupt 
/trap code.  (Or, in some cases, jumps are made back into 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 2.0.3 with fixes and development in ethernet interface

[Daniel Jacobowitz]

On Thu, Jan 22, 2004 at 02:54:55PM -0800, George Anzinger wrote:
> Daniel has suggested that we could just use the new bin tools where in the 
> gas program will build the call frames.  I am not sure it can handle 
> expressions, however.  And they are needed if you want to tie off the frame 
> if the next step is user land.

Well, we could add a dwarf expression parser to gas quite easily if
desired.

> By the way, I don't try to build a blow by blow of the frame.  Rather I 
> assume it is only of interest at those points where calls are made out of 
> the interrupt /trap code.  (Or, in some cases, jumps are made back into it.)

Only matters if you can be in KGDB at a particular instruction.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: KGDB 2.0.3 with fixes and development in ethernet interface

[Tom Rini]

On Thu, Jan 22, 2004 at 10:39:14AM +0530, Amit S. Kale wrote:
> On Thursday 22 Jan 2004 4:30 am, George Anzinger wrote:
> > Amit S. Kale wrote:
> > > On Sunday 18 Jan 2004 1:24 am, George Anzinger wrote:
> > >>Amit S. Kale wrote:
> > >>>On Saturday 17 Jan 2004 6:53 am, George Anzinger wrote:
> > >>>>>This seems to be needed (if I unselect CONFIG_KGDB_THREAD, I get
> > >>>>>compile error on x86-64).
> > >>>>
> > >>>>Amit, could you explain why this is an option?  It seems very useful
> > >>>> and not really too much code...
> > >>>
> > >>>It saves all registers before switch_to. It does that two times at
> > >>>present. Once (implicit) register save by gcc and an explicit register
> > >>>save in arch/<proc>/kernel/entry.S Second register save in kern_schedule
> > >>>generates a pt_regs structure which gdb can get all registers at once
> > >>>from. If it's omited, gdb has to figure out where gcc has saved
> > >>> registers from the non-standards code in switch_to, which it can't do
> > >>> correctly all the time.
> > >>>
> > >>>We can have a check for (a new variable) debugger_enabled before calling
> > >>>kern_schedule. That'll be add negligible overhead and will allow extra
> > >>>thread info to be saved only when a debugger is enabled. There will not
> > >>>be any need for CONFIG_KGDB_THREAD also.
> > >>
> > >>I don't seem to have such a problem with the mm kgdb.  No kern_schedule
> > >>there...
> > >
> > > Have you confirmed that you see correct values for all the registers? I
> > > had found some problems with gdb not being able to locate all the
> > > registers correctly. Explanation on the problem below:
> > >
> > > Besides getting gdb's fault there  is one more benefit of kern_schedule.
> > > All threads are shown by gdb as sleeping _at_ the place where schedule
> > > call is present: So instead of gdb showing all threads sleeping in
> > > __switch_to, it shows several functions. That's better than having to
> > > look at back trace of each thread to figure out what it is.
> >
> > The mm version of kgdb does this, but by lying to gdb during the info
> > thread command.  IMNSHO, the real way to do this is to process the info
> > thread output with an awk script, but I haven't done that just yet.  The
> > problem with lying to gdb is that it sometimes remembers...
> >
> > > This functionality is complemented by printing of thread names in 2.0.X
> > > kgdb info threads listing.
> >
> > as in the mm version.
> >
> > > Now back to gdb problem of not being able to locate registers.
> > > schedule results in code of this form:
> > >
> > > schedule:
> > > framesetup
> > > registers save
> > > ...
> > > ...
> > > save registers
> > > change esp
> > > call switchto
> > > restore registers
> > > ...
> >
> > I have not analyzed this as yet.  However, it does seem to me to be the
> > same problem as trying to bt through an interrupt frame.  The correct way
> > to do this is to build the dwarf frame descriptors.  I have done this for
> > the interrupt frame and intend to send said patch out in a day or so.
> 
> Great! I had to do it this ackward way:
[snip]
> Powerpc ->
>  	int irq, first = 1;
> +	unsigned long *lrptr;
> +	if (!(regs->msr & MSR_PR)) {
> +		/* Came in from the kernel. Save call link. */
> +		lrptr = (unsigned long *)(regs->gpr[1] + 4);
> +		*lrptr = regs->nip;
> +	}

Amit, this is broken and causes a lockup on my machine.  I haven't tried
to find the 'correct' way to do this, but in the next verion of the kgdb
patches can you please if 0 this section?  Thanks.

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