lkml.org 
[lkml]   [2004]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [Kgdb-bugreport] [PATCH][3/3] Update CVS KGDB's wrt connect / detach
    Date
    On Friday 27 Feb 2004 5:00 am, George Anzinger wrote:
    > Amit S. Kale wrote:
    > > On Thursday 26 Feb 2004 3:23 am, Tom Rini wrote:
    > >>The following patch fixes a number of little issues here and there, and
    > >>ends up making things more robust.
    > >>- We don't need kgdb_might_be_resumed or kgdb_killed_or_detached.
    > >> GDB attaching is GDB attaching, we haven't preserved any of the
    > >> previous context anyhow.
    > >
    > > If gdb is restarted, kgdb has to remove all breakpoints. Present kgdb
    > > does that in the code this patch removes:
    > >
    > > - if (remcom_in_buffer[0] == 'H' && remcom_in_buffer[1] == 'c') {
    > > - remove_all_break();
    > > - atomic_set(&kgdb_killed_or_detached, 0);
    > > - ok_packet(remcom_out_buffer);
    > >
    > > If we don't remove breakpoints, they stay in kgdb without gdb not knowing
    > > it and causes consistency problems.
    >
    > I wonder if this is worth the trouble. Does kgdb need to know about
    > breakpoints at all? Is there some other reason it needs to track them?

    kgdb tracks breakpoints in the cvs version.

    gdb many a times has a shorter life compared to kgdb (unfortunate but true).
    If gdb dies when breakpoints are present in a kernel, the kernel too has to
    be killed as there is no way to correct the kernel code modified by gdb.

    >
    > >>- Don't try and look for a connection in put_packet, after we've tried
    > >> to put a packet. Instead, when we receive a packet, GDB has
    > >> connected.
    > >
    > > We have to check for gdb connection in putpacket or else following
    > > problem occurs.
    > >
    > > 1. kgdb console messages are to be put.
    > > 2. gdb dies
    > > 3. putpacket writes the packet and waits for a '+'
    >
    > Oops! Tom, this '+' will be sent under interrupt and while kgdb is not
    > connected. Looks like it needs to be passed through without causing a
    > breakpoint. Possible salvation if we disable interrupts while waiting for
    > the '+' but I don't think that is a good idea.

    Yes. That's why I added a paramter to putpacket to skip causing a breakpoint
    when not required.

    -Amit

    >
    > -g
    >
    > > 4. new gdb sends a protocol initialization packet
    > > 5. putpacket reads characters in that packet hoping for an incoming '+'
    > > sending out console message packet on each incoming character
    > > 6. gdb receives and rejects each console message packet
    > >
    > >>- Remove ok_packet(), excessive, IMHO.
    > >
    > > ok_packet is better than littering "OK" all over the place.
    > >
    > > Other lindent changes are good.
    > >
    > > -Amit
    > >
    > >># This is a BitKeeper generated patch for the following project:
    > >># Project Name: Linux kernel tree
    > >># This patch format is intended for GNU patch command version 2.5 or
    > >>higher. # This patch includes the following deltas:
    > >># ChangeSet 1.1664 -> 1.1665
    > >># arch/ppc/8260_io/uart.c 1.31 -> 1.32
    > >># kernel/kgdb.c 1.4 -> 1.5
    > >>#
    > >># The following is the BitKeeper ChangeSet Log
    > >># --------------------------------------------
    > >># 04/02/25 trini@kernel.crashing.org 1.1665
    > >># - Kill kgdb_might_be_resumed and kgdb_killed_or_detached.
    > >># - Spacing and comments.
    > >># - put_packet doesn't try and check for a connect now.
    > >># - Kill ok_packet().
    > >># - Only send an initial packet in kgdb_handle_exception if
    > >># kgdb has connected already.
    > >># --------------------------------------------
    > >>#
    > >>diff -Nru a/arch/ppc/8260_io/uart.c b/arch/ppc/8260_io/uart.c
    > >>--- a/arch/ppc/8260_io/uart.c Wed Feb 25 14:21:38 2004
    > >>+++ b/arch/ppc/8260_io/uart.c Wed Feb 25 14:21:38 2004
    > >>@@ -396,14 +396,8 @@
    > >> #ifdef CONFIG_KGDB
    > >> if (info->state->smc_scc_num == KGDB_SER_IDX) {
    > >> while (i-- > 0) {
    > >>- if (*cp == 0x03) {
    > >>+ if (*cp == 0x03 || *cp == '$') {
    > >> breakpoint();
    > >>- return;
    > >>- }
    > >>- if (*cp == '$') {
    > >>- atomic_set(&kgdb_might_be_resumed, 1);
    > >>- breakpoint();
    > >>- atomic_set(&kgdb_might_be_resumed, 0);
    > >> return;
    > >> }
    > >> cp++;
    > >>diff -Nru a/kernel/kgdb.c b/kernel/kgdb.c
    > >>--- a/kernel/kgdb.c Wed Feb 25 14:21:38 2004
    > >>+++ b/kernel/kgdb.c Wed Feb 25 14:21:38 2004
    > >>@@ -15,6 +15,7 @@
    > >> * Copyright (C) 2002-2004 Timesys Corporation
    > >> * Copyright (C) 2003-2004 Amit S. Kale
    > >> * Copyright (C) 2004 Pavel Machek <pavel@suse.cz>
    > >>+ * Copyright (C) 2004 Tom Rini <trini@kernel.crashing.org>
    > >> *
    > >> * Restructured KGDB for 2.6 kernels.
    > >> * thread support, support for multiple processors,support for
    > >> ia-32(x86) @@ -87,8 +88,6 @@
    > >> static spinlock_t slavecpulocks[KGDB_MAX_NO_CPUS];
    > >> static volatile int procindebug[KGDB_MAX_NO_CPUS];
    > >> atomic_t kgdb_setting_breakpoint;
    > >>-atomic_t kgdb_killed_or_detached;
    > >>-atomic_t kgdb_might_be_resumed;
    > >> struct task_struct *kgdb_usethread, *kgdb_contthread;
    > >>
    > >> int debugger_step;
    > >>@@ -212,8 +211,10 @@
    > >> char ch;
    > >>
    > >> do {
    > >>- /* wait around for the start character, ignore all other characters */
    > >>- while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$') ;
    > >>+ /* wait around for the start character, ignore all other
    > >>+ * characters */
    > >>+ while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$')
    > >>+ ; /* Spin. */
    > >> kgdb_connected = 1;
    > >> checksum = 0;
    > >> xmitcsum = -1;
    > >>@@ -249,27 +250,22 @@
    > >> * Send the packet in buffer.
    > >> * Check for gdb connection if asked for.
    > >> */
    > >>-static void put_packet(char *buffer, int checkconnect)
    > >>+static void put_packet(char *buffer)
    > >> {
    > >> unsigned char checksum;
    > >> int count;
    > >> char ch;
    > >>- static char gdbseq[] = "$Hc-1#09";
    > >>- int i;
    > >>- int send_count;
    > >>
    > >> /* $<packet info>#<checksum>. */
    > >> do {
    > >> kgdb_serial->write_char('$');
    > >> checksum = 0;
    > >> count = 0;
    > >>- send_count = 0;
    > >>
    > >> while ((ch = buffer[count])) {
    > >> kgdb_serial->write_char(ch);
    > >> checksum += ch;
    > >> count++;
    > >>- send_count++;
    > >> }
    > >>
    > >> kgdb_serial->write_char('#');
    > >>@@ -277,30 +273,7 @@
    > >> kgdb_serial->write_char(hexchars[checksum % 16]);
    > >> if (kgdb_serial->flush)
    > >> kgdb_serial->flush();
    > >>-
    > >>- i = 0;
    > >>- while ((ch = kgdb_serial->read_char()) == gdbseq[i++] &&
    > >>- checkconnect) {
    > >>- if (!gdbseq[i]) {
    > >>- kgdb_serial->write_char('+');
    > >>- if (kgdb_serial->flush)
    > >>- kgdb_serial->flush();
    > >>- breakpoint();
    > >>-
    > >>- /*
    > >>- * GDB is available now.
    > >>- * Retransmit this packet.
    > >>- */
    > >>- break;
    > >>- }
    > >>- }
    > >>- if (checkconnect && ch == 3) {
    > >>- kgdb_serial->write_char('+');
    > >>- if (kgdb_serial->flush)
    > >>- kgdb_serial->flush();
    > >>- breakpoint();
    > >>- }
    > >>- } while ((ch & 0x7f) != '+');
    > >>+ } while ((kgdb_serial->read_char() & 0x7f) != '+');
    > >>
    > >> }
    > >>
    > >>@@ -427,11 +400,6 @@
    > >> pkt[3] = '\0';
    > >> }
    > >>
    > >>-static void ok_packet(char *pkt)
    > >>-{
    > >>- strcpy(pkt, "OK");
    > >>-}
    > >>-
    > >> static char *pack_threadid(char *pkt, threadref * id)
    > >> {
    > >> char *limit;
    > >>@@ -502,7 +470,8 @@
    > >> procindebug[processor] = 1;
    > >> current->thread.debuggerinfo = regs;
    > >>
    > >>- /* Wait till master processor goes completely into the debugger. FIXME:
    > >>this looks racy */ + /* 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 */
    > >>
    > >>@@ -701,17 +670,7 @@
    > >> /* Master processor is completely in the debugger */
    > >> kgdb_post_master_code(linux_regs, exVector, err_code);
    > >>
    > >>- if (atomic_read(&kgdb_killed_or_detached) &&
    > >>- atomic_read(&kgdb_might_be_resumed)) {
    > >>- get_packet(remcom_in_buffer);
    > >>- if (remcom_in_buffer[0] == 'H' && remcom_in_buffer[1] == 'c') {
    > >>- remove_all_break();
    > >>- atomic_set(&kgdb_killed_or_detached, 0);
    > >>- ok_packet(remcom_out_buffer);
    > >>- } else
    > >>- return 1;
    > >>- } else {
    > >>-
    > >>+ if (kgdb_connected) {
    > >> /* reply to host that an exception has occurred */
    > >> ptr = remcom_out_buffer;
    > >> *ptr++ = 'T';
    > >>@@ -721,11 +680,9 @@
    > >> int_to_threadref(&thref, shadow_pid(current->pid));
    > >> ptr = pack_threadid(ptr, &thref);
    > >> *ptr++ = ';';
    > >>- *ptr = '\0';
    > >>- }
    > >>
    > >>- put_packet(remcom_out_buffer, 0);
    > >>- kgdb_connected = 1;
    > >>+ put_packet(remcom_out_buffer);
    > >>+ }
    > >>
    > >> kgdb_usethread = current;
    > >> kgdb_usethreadid = shadow_pid(current->pid);
    > >>@@ -798,7 +755,7 @@
    > >> else {
    > >> gdb_regs_to_regs(gdb_regs, (struct pt_regs *)
    > >> current->thread.debuggerinfo);
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> }
    > >>
    > >> break;
    > >>@@ -838,10 +795,10 @@
    > >> if ((error = remove_all_break()) < 0) {
    > >> error_packet(remcom_out_buffer, error);
    > >> } else {
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> kgdb_connected = 0;
    > >> }
    > >>- put_packet(remcom_out_buffer, 0);
    > >>+ put_packet(remcom_out_buffer);
    > >> goto default_handle;
    > >>
    > >> case 'k':
    > >>@@ -947,11 +904,10 @@
    > >> }
    > >> kgdb_usethread = thread;
    > >> kgdb_usethreadid = threadid;
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> break;
    > >>
    > >> case 'c':
    > >>- atomic_set(&kgdb_killed_or_detached, 0);
    > >> ptr = &remcom_in_buffer[2];
    > >> kgdb_hex2long(&ptr, &threadid);
    > >> if (!threadid) {
    > >>@@ -966,7 +922,7 @@
    > >> }
    > >> kgdb_contthread = thread;
    > >> }
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> break;
    > >> }
    > >> break;
    > >>@@ -977,7 +933,7 @@
    > >> kgdb_hex2long(&ptr, &threadid);
    > >> thread = getthread(linux_regs, threadid);
    > >> if (thread)
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> else
    > >> error_packet(remcom_out_buffer, -EINVAL);
    > >> break;
    > >>@@ -1018,7 +974,7 @@
    > >> }
    > >>
    > >> if (error == 0)
    > >>- ok_packet(remcom_out_buffer);
    > >>+ strcpy(remcom_out_buffer, "OK");
    > >> else
    > >> error_packet(remcom_out_buffer, error);
    > >>
    > >>@@ -1039,7 +995,7 @@
    > >> } /* switch */
    > >>
    > >> /* reply to the request */
    > >>- put_packet(remcom_out_buffer, 0);
    > >>+ put_packet(remcom_out_buffer);
    > >> }
    > >>
    > >> kgdb_exit:
    > >>@@ -1063,7 +1019,6 @@
    > >> }
    > >>
    > >> /* Free debugger_active */
    > >>- atomic_set(&kgdb_killed_or_detached, 1);
    > >> atomic_set(&debugger_active, 0);
    > >> local_irq_restore(flags);
    > >>
    > >>@@ -1132,12 +1087,6 @@
    > >> /* Free debugger_active */
    > >> atomic_set(&debugger_active, 0);
    > >>
    > >>- /* This flag is used, if gdb has detached and wants to start
    > >>- * another session
    > >>- */
    > >>- atomic_set(&kgdb_killed_or_detached, 1);
    > >>- atomic_set(&kgdb_might_be_resumed, 0);
    > >>-
    > >> for (i = 0; i < MAX_BREAKPOINTS; i++)
    > >> kgdb_break[i].state = bp_disabled;
    > >>
    > >>@@ -1222,7 +1171,7 @@
    > >> *bufptr = '\0';
    > >> s += wcount;
    > >>
    > >>- put_packet(kgdbconbuf, 1);
    > >>+ put_packet(kgdbconbuf);
    > >>
    > >> }
    > >> local_irq_restore(flags);
    > >
    > > -
    > > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
    > > in the body of a message to majordomo@vger.kernel.org
    > > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > > Please read the FAQ at http://www.tux.org/lkml/

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

    \
     
     \ /
      Last update: 2005-03-22 14:01    [W:2.462 / U:1.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site