diff --git a/docs/architecture/debugging.md b/docs/architecture/debugging.md index 6bac47f..dd6dfd0 100644 --- a/docs/architecture/debugging.md +++ b/docs/architecture/debugging.md @@ -200,10 +200,24 @@ fine. | | Works when | |---|---| -| `sysrq g` | always — every view to the console, no process, no filesystem, no client program | -| `/dev/dbg` | there is a scheduler and a filesystem | +| **Ctrl-G** on the console | there is a scheduler — so from early in boot, with nothing mounted and nothing in userspace started | +| `sysrq g` | `/proc` is mounted | +| `/dev/dbg` | `/dev` is mounted | | `schdbg` | there is a shell | +**Ctrl-G is the one that matters for boot problems**, because it is the only +one that does not need a filesystem. It sits beside the older Ctrl-P, which +dumps the process table — and the difference between them is worth knowing. +`procdump()` takes **no lock at all**, deliberately: a lock is a way to hang +while reporting a hang. The views cannot follow that rule, because a free list +or a socket table read torn is worse than not read. So they take locks, and a +lock must never be taken from an interrupt handler that may have interrupted +its holder. + +The key therefore *requests* and a kernel thread *performs*. The cost is stated +plainly: a scheduler too wedged to run a thread will not answer Ctrl-G. Ctrl-P +still will, and that division of labour is the honest one. + A watchpoint's *hits* go to the console rather than to any of these, because they happen while you are not looking, which is the point of having them. @@ -342,6 +356,10 @@ something that reports nonsense. watchpoint on something `cprintf` itself writes is silently dropped rather than looping; the handler guards against re-entering itself per CPU. - **A watchpoint takes up to one timer tick** to reach the other CPUs. +- **Ctrl-G needs a working scheduler**, per the note above. Before + `dbg_thread_start()` — which is the last thing `main()` does before entering + the scheduler — nothing answers but the log, and the log is enough because it + is read on the *next* boot. - **No network transport.** The plan puts a TCP bridge in a userspace daemon, deliberately outside the kernel; it does not exist, so the debugger is local or serial only. diff --git a/docs/architecture/graphics.md b/docs/architecture/graphics.md index da370fc..9aac02d 100644 --- a/docs/architecture/graphics.md +++ b/docs/architecture/graphics.md @@ -179,17 +179,36 @@ you it stopped — which is both truer and much less alarming. face that is obviously a cartoon, then the facts. Whoever is reading this is already having a bad time. -**It says considerably more than it needs to.** The screen is the only artefact -of a panic — no core dump, no log to go back to — so everything cheap to collect -is collected: stop code, fault address, which CPU, which process, how far into -the boot, where the kernel is loaded, and a call chain. Anything left out is -gone. All of it goes to the serial line in the same order, which is what -`bootcheck` reads and what you get when the crash happens before there is a -framebuffer at all. +**It says considerably more than it needs to.** Everything cheap to collect is +collected: stop code, fault address, which CPU, which process, how far into the +boot, where the kernel is loaded, and a call chain — and since +[debugging.md](debugging.md)'s symbol table, the call chain is *names*: +`panic+0x5f`, `proc_sysrq_cmd+0xb3`, `sys_writev+0x12b`. All of it goes to the +serial line in the same order, which is what `bootcheck` reads and what you get +when the crash happens before there is a framebuffer at all. Once painted, `panicked` is set and `consputc` spins rather than writing, so nothing can scribble over the report. +### It is no longer the only copy + +This screen used to be the only artefact of a panic — this file said so, and so +did `panic.c`'s own header. That stopped being true when the kernel log started +outliving the kernel, and the closing lines changed to match: + +``` +This report is in /proc/kmsg after a restart. +schdbg, or Ctrl-G on the console, asks more. +PSOGM -- Purple Screen of Guru Meditation +``` + +The first line replaced an `addr2line` command, which the call chain no longer +needs. It is the more useful instruction by a wide margin, and it is the one +nobody would guess: **restart and read it** beats photographing the screen. + +The QR code below is still drawn, and still earns its place — it is the only +channel that works when the machine will not come back at all. + ### The QR code `kernel/core/qr.c`. **A panic report is worth nothing if it cannot leave a diff --git a/include/defs.h b/include/defs.h index 9459fde..a96e353 100644 --- a/include/defs.h +++ b/include/defs.h @@ -373,6 +373,8 @@ int watch_trap(struct trapframe *tf); // dbg.c -- /dev/dbg, the introspection registry. See include/dbg.h. int dbg_read(struct inode*, uint64, char*, int); +void dbg_request_dump(void); +void dbg_thread_start(void); int dbg_write(struct inode*, uint64, char*, int); // klog.c -- the kernel log that outlives the kernel. See include/klog.h. diff --git a/kernel/core/console.c b/kernel/core/console.c index a2537a0..c4fb6b9 100644 --- a/kernel/core/console.c +++ b/kernel/core/console.c @@ -2,6 +2,7 @@ #include "types.h" #include "defs.h" #include "klog.h" +#include "dbg.h" #include "vt.h" #include "kbd.h" /* KEY_VT_SWITCH -- the terminal-switch chords */ #include "gfx.h" @@ -462,6 +463,19 @@ consoleintr(int (*getc)(void)) continue; } + // Ctrl-G: every debug view, on the console. + // + // The same thing `sysrq g` and `schdbg all` produce, reachable with no + // filesystem mounted and no shell running -- which is the point. It only + // asks; a kernel thread does the work, because the views take locks and + // this is an interrupt handler. See dbg_request_dump(). + if (c == C('G')) { + release(&input_lock); + dbg_request_dump(); + acquire(&input_lock); + continue; + } + if (v->tio.c_lflag & ICANON) { if (c == v->tio.c_cc[VERASE] || c == C('H')) { erase_one(v); diff --git a/kernel/core/main.c b/kernel/core/main.c index 212c6f3..af06c09 100644 --- a/kernel/core/main.c +++ b/kernel/core/main.c @@ -217,6 +217,12 @@ kmain(uint32 fb_addr) // until mpmain() below enters the scheduler. netinit(); if (boot_verbose) cprintf("verbose boot: netinit done\n"); + // The debugger's own thread, for the same reason netinit's comes after + // sinit: kthread_create needs a usable process table. From here on Ctrl-G + // on the console answers, with no filesystem mounted and nothing in + // userspace started -- which is the whole point of putting it this early. + dbg_thread_start(); + if (boot_verbose) cprintf("verbose boot: dbg thread started\n"); mpmain(); } diff --git a/kernel/core/panic.c b/kernel/core/panic.c index f1a1963..63c01f4 100644 --- a/kernel/core/panic.c +++ b/kernel/core/panic.c @@ -497,16 +497,25 @@ paint(char *reason, addr_t *pcs, int npcs) trace_line(line, sizeof(line), i, pcs[i]); gfx_text_at(vx - 3 * cw, y, line, PSOGM_FG); y += ch; - if (y > sh - ch * 5) + if (y > sh - ch * 6) break; } - // Two closing lines, bottom left: what to do with the numbers above, and - // what this screen is called, so it can be searched for by name. Shortened - // from the full command line, which at 62 characters ran under the QR card. - gfx_text_at(left, sh - ch * 3, - "Resolve: addr2line -e kernel
", PSOGM_DIM); - gfx_text_at(left, sh - ch * 3 + ch + 4, + // Three closing lines, bottom left. The first used to be an addr2line + // command, which the call chain above no longer needs -- it prints names. + // + // What replaces it is the thing nobody would guess: **this screen is no + // longer the only copy.** The log survives into the next boot (see + // include/klog.h), so the right instruction on a crash screen is now + // "restart and read it" rather than "photograph this". + // + // Kept under about 45 characters each: the QR card is on the right, and the + // 62-character command line this replaces used to run underneath it. + gfx_text_at(left, sh - ch * 4, + "This report is in /proc/kmsg after a restart.", PSOGM_DIM); + gfx_text_at(left, sh - ch * 4 + ch + 4, + "schdbg, or Ctrl-G on the console, asks more.", PSOGM_DIM); + gfx_text_at(left, sh - ch * 4 + (ch + 4) * 2, "PSOGM -- Purple Screen of Guru Meditation", PSOGM_DIM); } @@ -542,6 +551,14 @@ report_serial(char *reason, addr_t *pcs, int npcs) cprintf("\n Resolve with: addr2line -e build/kernel/kernel
\n"); else cprintf("\n For a line number: addr2line -e build/kernel/kernel
\n"); + + // Where to go next. Worth printing even though the machine is about to stop, + // because this text is also what ends up in the log -- and the log is the + // thing being pointed at. + cprintf(" This report is in /proc/kmsg after a restart.\n"); + cprintf(" Then: schdbg whose -- who owns a page\n"); + cprintf(" schdbg watch -- who writes to one\n"); + cprintf(" Ctrl-G on the console -- every view, no shell needed\n"); cprintf(" Reason: %s\n", reason); cprintf(" System halted.\n"); } diff --git a/kernel/dev/dbg.c b/kernel/dev/dbg.c index d926129..95bda6c 100644 --- a/kernel/dev/dbg.c +++ b/kernel/dev/dbg.c @@ -328,6 +328,56 @@ fill(const char *cmd) return sb.len; } +// --- the console key -------------------------------------------------------- +// +// Ctrl-G runs the debugger with no filesystem, no shell and no client program, +// which is what makes it available *during boot* rather than once userspace is +// up. docs/plan/debugger.md calls this level 0, and it is the level that +// matters when the thing you are debugging is the boot. +// +// It cannot do the work in the interrupt handler, though, and the reason is +// worth stating because the neighbouring Ctrl-P does exactly that. procdump() +// reads the process table with no lock held, deliberately: a lock is a way to +// hang while reporting a hang. The views cannot follow that rule -- they read +// free lists and socket tables that are meaningless torn -- so they take locks, +// and taking a lock from an interrupt handler deadlocks against whatever was +// interrupted while holding it. +// +// So the key *requests* and a kernel thread *performs*. The cost is that a +// scheduler too wedged to run a thread will not answer; Ctrl-P still does, and +// that division is the honest one. +static int dbg_wanted; + +void +dbg_request_dump(void) +{ + dbg_wanted = 1; + wakeup(&dbg_wanted); +} + +static void +dbg_thread(void) +{ + for (;;) { + acquire(&dbglock); + while (!dbg_wanted) + sleep(&dbg_wanted, &dbglock); + dbg_wanted = 0; + release(&dbglock); + + cprintf("\n--- debug views, requested from the console ---\n"); + dbg_dump_all(); + } +} + +void +dbg_thread_start(void) +{ + if (!dbginit) { initlock(&dbglock, "dbg"); dbginit = 1; } + if (kthread_create(dbg_thread, "dbgd") < 0) + cprintf("dbg: no kernel thread; Ctrl-G will not work\n"); +} + void dbg_dump_all(void) {