2004-01-16 12:30:48

by Amit S. Kale

[permalink] [raw]
Subject: KGDB 2.0.3 with fixes and development in ethernet interface

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


2004-01-16 12:58:27

by Pavel Machek

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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?]

2004-01-16 13:07:29

by Andi Kleen

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

On Fri, 16 Jan 2004 17:59:59 +0530
"Amit S. Kale" <[email protected]> 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


2004-01-16 14:07:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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.

2004-01-16 14:23:53

by Amit S. Kale

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

On Friday 16 Jan 2004 6:35 pm, Andi Kleen wrote:
> On Fri, 16 Jan 2004 17:59:59 +0530
>
> "Amit S. Kale" <[email protected]> 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)

2004-01-16 14:18:16

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-16 14:26:56

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-16 14:46:09

by Pavel Machek

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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?]

2004-01-16 14:46:50

by Pavel Machek

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

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?]

2004-01-16 14:48:55

by Pavel Machek

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

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?]

2004-01-16 15:13:11

by Amit S. Kale

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

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)

2004-01-16 15:16:43

by Amit S. Kale

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

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)

2004-01-16 15:53:42

by Pavel Machek

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

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 <[email protected]>
+
+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?]

2004-01-16 16:15:00

by Pavel Machek

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 <[email protected]>
+ * Some cleanups by Pavel Machek <[email protected]>
*/

#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 <[email protected]>
*
* 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 <[email protected]>
* Integrated into 2.2.5 kernel by Tigran Aivazian <[email protected]>
- *
-*
*/

#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?]

2004-01-16 18:40:47

by Matt Mackall

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

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" <[email protected]> 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

2004-01-16 20:49:18

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-16 22:07:37

by Pavel Machek

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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?]

2004-01-17 01:23:59

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-17 06:28:14

by Amit S. Kale

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

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 <[email protected]>
> +
> +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)

2004-01-17 09:01:01

by George Anzinger

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

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-17 09:29:39

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-17 19:55:16

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-21 13:47:07

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-21 18:39:51

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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

2004-01-21 23:01:21

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-21 23:16:18

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-22 05:09:39

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-22 05:49:45

by Amit S. Kale

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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)

2004-01-22 20:33:38

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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

2004-01-22 22:55:40

by George Anzinger

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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 [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2004-01-22 17:20:49

by Tom Rini

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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/

2004-01-22 22:57:35

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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

2004-01-23 19:11:07

by Tom Rini

[permalink] [raw]
Subject: Re: KGDB 2.0.3 with fixes and development in ethernet interface

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/