kdb
[Top] [All Lists]

one more try (at SMP)

To: kdb@xxxxxxxxxxx
Subject: one more try (at SMP)
From: Ethan Solomita <ethan@xxxxxxxxxxxxxxx>
Date: Wed, 08 May 2002 20:13:23 -0700
Sender: owner-kdb@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc1) Gecko/20020417
I decided to get my hands on an SMP i386 so I could play with it too. I believe I've got something that works. There were a few issues with setting and clearing of the single-step flags. kdba_clearsinglestep() for some reason wasn't actually clearing EF_TF. It was being cleared automagically in the kdba_db_trap() code, which I hadn't anticipated.

        So I cleaned up a few things. Here's the summary:

kdb_bp.c: redundantly setting DOING_SS and DOING_SSB. removed.

kdb_io.c: kdb_printf didn't handle recursion correctly. Also fixed sparc64 issues.

kdbmain.c: call kdba_setsinglestep in exactly one place: before exiting kdb() if DOING_SS.

kdba_bp.c: fixes as discussed already. do not clear single-step EF_TF flag manually.

kdbasupport.c: have setsinglestep and clearsinglestep check whether EF_TF is already set/clear. Make clearsinglestep actually do what it's name implies.

Diffs are attached. They are on top of everything already sent out, including the two line fix I emailed out. Once this all seems good and solid, I'll make a single diff.
        -- Ethan
==== //depot/software/kernel/linux2.4/linux_kdb/kdb/kdb_bp.c#3 (ktext) - 
//depot/software/kernel/linux2.4/linux_kdb/kdb/kdb_bp.c#4 (ktext) ==== content
@@ -557,15 +557,6 @@
        if (argc != 0)
                return KDB_ARGCOUNT;
 
-       /*
-        * Set trace flag and go.
-        */
-       KDB_STATE_SET(DOING_SS);
-       if (ssb)
-               KDB_STATE_SET(DOING_SSB);
-
-       kdba_setsinglestep(ef);         /* Enable single step */
-
        if (ssb)
                return KDB_CMD_SSB;
        return KDB_CMD_SS;
==== //depot/software/kernel/linux2.4/linux_kdb/kdb/kdb_io.c#5 (text) - 
//depot/software/kernel/linux2.4/linux_kdb/kdb/kdb_io.c#7 (text) ==== content
@@ -158,17 +158,22 @@
        int linecount;
        int logging, saved_loglevel = 0;
        int do_longjmp = 0;
+#ifndef CONFIG_SPARC64
        struct console *c = console_drivers;
+#endif
        static spinlock_t kdb_printf_lock = SPIN_LOCK_UNLOCKED;
+       static int kdb_printf_count;
 
        /* Serialize kdb_printf if multiple cpus try to write at once.
         * But if any cpu goes recursive in kdb, just print the output,
         * even if it is interleaved with any other text.
         */
        if (!KDB_STATE(PRINTF_LOCK)) {
+               spin_lock(&kdb_printf_lock);
                KDB_STATE_SET(PRINTF_LOCK);
-               spin_lock(&kdb_printf_lock);
-       }
+               kdb_printf_count = 1;
+       } else
+               kdb_printf_count++;
 
        diag = kdbgetintenv("LINES", &linecount);
        if (diag)
@@ -186,14 +191,13 @@
         * Write to all consoles.
         */
 #ifdef CONFIG_SPARC64
-       if (c == NULL)
-               prom_printf("%s", buffer);
-       else
-#endif
+       prom_printf("%s",buffer);
+#else
        while (c) {
                c->write(c, buffer, strlen(buffer));
                c = c->next;
        }
+#endif
        if (logging) {
                saved_loglevel = console_loglevel;
                console_loglevel = 0;
@@ -233,16 +237,16 @@
                }
 #endif
 
+#ifdef CONFIG_SPARC64
+               prom_printf("%s", moreprompt);
+
+#else
                c = console_drivers;
-#ifdef CONFIG_SPARC64
-               if (c == NULL)
-                       prom_printf("%s", moreprompt);
-               else
-#endif
                while (c) {
                        c->write(c, moreprompt, strlen(moreprompt));
                        c = c->next;
                }
+#endif
                if (logging)
                        printk("%s", moreprompt);
 
@@ -259,9 +263,11 @@
        if (logging) {
                console_loglevel = saved_loglevel;
        }
-       if (KDB_STATE(PRINTF_LOCK)) {
+       if (!KDB_STATE(PRINTF_LOCK))
+               kdb_printf("kdb_printf panic: lock not set!\n");
+       else if (--kdb_printf_count == 0) {
+               KDB_STATE_CLEAR(PRINTF_LOCK);
                spin_unlock(&kdb_printf_lock);
-               KDB_STATE_CLEAR(PRINTF_LOCK);
        }
        if (do_longjmp)
 #ifdef KDB_HAVE_LONGJMP
==== //depot/software/kernel/linux2.4/linux_kdb/kdb/kdbmain.c#9 (ktext) - 
//depot/software/kernel/linux2.4/linux_kdb/kdb/kdbmain.c#12 (ktext) ==== content
@@ -1085,7 +1085,6 @@
                        KDB_DEBUG_STATE("kdb_main_loop 4a", reason);
                        KDB_STATE_SET(DOING_SS);
                        KDB_STATE_SET(DOING_SSBPT);
-                       kdba_setsinglestep(ef);
                        break;
                }
 
@@ -1453,6 +1452,9 @@
        KDB_STATE_CLEAR(KDB);           /* Main kdb state has been cleared */
        KDB_STATE_CLEAR(LEAVING);       /* Elvis has left the building ... */
        KDB_STATE_CLEAR(NEED_SSBPT);
+       if (KDB_STATE(DOING_SS))
+               kdba_setsinglestep(ef);
+       
        KDB_DEBUG_STATE("kdb 14", result);
 
        if (cpu == kdb_initial_cpu &&
==== //depot/software/kernel/linux2.4/linux_kdb/arch/i386/kdb/kdba_bp.c#2 
(ktext) - //depot/software/kernel/linux2.4/linux_kdb/arch/i386/kdb/kdba_bp.c#3 
(ktext) ==== content
@@ -95,44 +95,22 @@
        if (KDB_DEBUG(BP))
                kdb_printf("kdb: dr6 0x%lx dr7 0x%lx\n", dr6, dr7);
        if (dr6 & DR6_BS) {
-               if (KDB_STATE(SSBPT)) {
-                       if (KDB_DEBUG(BP))
-                               kdb_printf("ssbpt\n");
-                       KDB_STATE_CLEAR(SSBPT);
-                       for(i=0,bp=kdb_breakpoints;
-                           i < KDB_MAXBPT;
-                           i++, bp++) {
-                               if (KDB_DEBUG(BP))
-                                       kdb_printf("bp 0x%p enabled %d delayed 
%d global %d cpu %d\n",
-                                                  bp, bp->bp_enabled, 
bp->bp_delayed, bp->bp_global, bp->bp_cpu);
-                               if (!bp->bp_enabled)
-                                       continue;
-                               if (!bp->bp_global && bp->bp_cpu != 
smp_processor_id())
-                                       continue;
-                               if (KDB_DEBUG(BP))
-                                       kdb_printf("bp for this cpu\n");
-                               if (bp->bp_delayed) {
-                                       bp->bp_delayed = 0;
-                                       if (KDB_DEBUG(BP))
-                                               kdb_printf("kdba_installbp\n");
-                                       kdba_installbp(ef, bp);
-                                       if (!KDB_STATE(DOING_SS)) {
-                                               ef->eflags &= ~EF_TF;
-                                               return(KDB_DB_SSBPT);
-                                       }
-                                       break;
-                               }
-                       }
-                       if (i == KDB_MAXBPT) {
-                               kdb_printf("kdb: Unable to find delayed 
breakpoint\n");
-                       }
-                       if (!KDB_STATE(DOING_SS)) {
-                               ef->eflags &= ~EF_TF;
-                               return(KDB_DB_NOBPT);
-                       }
-                       /* FALLTHROUGH */
-               }
+               if (!KDB_STATE(DOING_SS))
+                       goto unknown;
+
+               KDB_STATE_CLEAR(DOING_SS);
+               
+               if (!KDB_STATE(DOING_SSBPT))
+                       goto not_ssbpt;
+
+               KDB_STATE_CLEAR(DOING_SSBPT);
+
+               if (KDB_DEBUG(BP))
+                       kdb_printf("ssbpt\n");
+               
+               return (KDB_DB_SSBPT);
 
+not_ssbpt:
                /*
                 * KDB_STATE_DOING_SS is set when the kernel debugger is using
                 * the processor trap flag to single-step a processor.  If a
@@ -140,8 +118,6 @@
                 * will be ignored by KDB and the kernel will be allowed to deal
                 * with it as necessary (e.g. for ptrace).
                 */
-               if (!KDB_STATE(DOING_SS))
-                       goto unknown;
 
                /* single step */
                rv = KDB_DB_SS;         /* Indicate single step */
@@ -162,8 +138,8 @@
                                 * End the ssb command here.
                                 */
                                KDB_STATE_CLEAR(DOING_SSB);
-                               KDB_STATE_CLEAR(DOING_SS);
                        } else {
+                               KDB_STATE_SET(DOING_SS); /* still need to step 
*/
                                rv = KDB_DB_SSB; /* Indicate ssb - dismiss 
immediately */
                        }
                } else {
@@ -173,11 +149,7 @@
                        kdb_printf("SS trap at ");
                        kdb_symbol_print(ef->eip, NULL, 
KDB_SP_DEFAULT|KDB_SP_NEWLINE);
                        kdb_id1(ef->eip);
-                       KDB_STATE_CLEAR(DOING_SS);
                }
-
-               if (rv != KDB_DB_SSB)
-                       ef->eflags &= ~EF_TF;
        }
 
        if (dr6 & DR6_B0) {
@@ -244,6 +216,7 @@
 
 unknown:
        ef->eflags |= EF_RF;    /* Supress further faults */
+       kdb_printf("kdb: debug trap, unknown cause\n");
        rv = KDB_DB_NOBPT;      /* Cause kdb() to return */
 
 handled:
@@ -307,6 +280,9 @@
                           "eflags=0x%lx ef=0x%p esp=0x%lx\n",
                           ef->eip, ef->eflags, ef, ef->esp);
 
+       if (KDB_STATE(DOING_SS))
+               kdb_printf("kdba_bp_trap: ??? should be single stepping!\n");
+
        rv = KDB_DB_NOBPT;      /* Cause kdb() to return */
 
        for(i=0, bp=kdb_breakpoints; i<KDB_MAXBPT; i++, bp++) {
@@ -321,7 +297,7 @@
                                  i, ef->eip);
                        kdb_id1(ef->eip);
                        rv = KDB_DB_BPT;
-                       bp->bp_delay = 1;
+                       KDB_STATE_SET(NEED_SSBPT);
                        break;
                }
        }
@@ -330,56 +306,6 @@
 }
 
 /*
- * kdba_handle_bp
- *
- *     Handle an instruction-breakpoint trap.  Called when re-installing
- *     an enabled breakpoint which has has the bp_delay bit set.
- *
- * Parameters:
- * Returns:
- * Locking:
- * Remarks:
- *
- * Ok, we really need to:
- *     1) Restore the original instruction byte
- *     2) Single Step
- *     3) Restore breakpoint instruction
- *     4) Continue.
- *
- *
- */
-
-static void
-kdba_handle_bp(kdb_eframe_t ef, kdb_bp_t *bp)
-{
-       if (!ef) {
-               kdb_printf("kdba_handle_bp: ef == NULL\n");
-               return;
-       }
-
-       if (KDB_DEBUG(BP))
-               kdb_printf("ef->eip = 0x%lx\n", ef->eip);
-
-       /*
-        * Setup single step
-        */
-       kdba_setsinglestep(ef);
-
-       /* KDB_STATE_SSBPT is set when the kernel debugger must single step
-        * a task in order to re-establish an instruction breakpoint which
-        * uses the instruction replacement mechanism. 
-        */
-       KDB_STATE_SET(SSBPT);
-
-       /*
-        * Reset delay attribute
-        */
-       bp->bp_delay = 0;
-       bp->bp_delayed = 1;
-}
-
-
-/*
  * kdba_bptype
  *
  *     Return a string describing type of breakpoint.
@@ -714,10 +640,6 @@
                                kdb_printf("kdba_installbp hardware reg %ld at 
" kdb_bfd_vma_fmt "\n",
                                           bp->bp_hard->bph_reg, bp->bp_addr);
                        }
-               } else if (bp->bp_delay) {
-                       if (KDB_DEBUG(BP))
-                               kdb_printf("kdba_installbp delayed bp\n");
-                       kdba_handle_bp(ef, bp);
                } else {
                        if (kdb_getarea_size(&(bp->bp_inst), bp->bp_addr, 1) ||
                            kdb_putword(bp->bp_addr, 
IA32_BREAKPOINT_INSTRUCTION, 1)) {
==== //depot/software/kernel/linux2.4/linux_kdb/arch/i386/kdb/kdbasupport.c#2 
(ktext) - 
//depot/software/kernel/linux2.4/linux_kdb/arch/i386/kdb/kdbasupport.c#3 
(ktext) ==== content
@@ -1001,6 +1001,9 @@
 void
 kdba_setsinglestep(struct pt_regs *regs)
 {
+       if (regs->eflags & EF_TF)
+               return;
+       
        if (regs->eflags & EF_IE)
                KDB_STATE_SET(A_IF);
        else
@@ -1011,10 +1014,14 @@
 void
 kdba_clearsinglestep(struct pt_regs *regs)
 {
+       if ((regs->eflags & EF_TF) == 0)
+               return;
+       
        if (KDB_STATE(A_IF))
                regs->eflags |= EF_IE;
        else
                regs->eflags &= ~EF_IE;
+       regs->eflags &= ~EF_TF;
 }
 
 int
<Prev in Thread] Current Thread [Next in Thread>
  • one more try (at SMP), Ethan Solomita <=