Remove CONFIG_KTHREAD_POISON
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 28 Nov 2016 18:58:10 +0000 (13:58 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 29 Nov 2016 16:27:40 +0000 (11:27 -0500)
The poisoning was originally put in to help catch bugs where we run on
kernel stacks that are running somewhere else.  We haven't had a bug like
that in a long time.

Over time, the poisoning was shoe-horned to occasionally detect after we
run off the end of the stack.  It was marginal at best.  Now that we have
stack guard pages, we don't need this at all.

If we ever need this for temporary debugging, people can revert this commit
on their local branches, but I'd rather not keep this cruft around in the
main tree.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
Kconfig
config-default
kern/arch/x86/trap.c
kern/include/kthread.h
kern/src/kthread.c
kern/src/printfmt.c
kern/src/smp.c

diff --git a/Kconfig b/Kconfig
index 3635aef..cd953ca 100644 (file)
--- a/Kconfig
+++ b/Kconfig
@@ -177,14 +177,6 @@ config LARGE_KSTACKS
                higher performance, and mention this setting if you have any weird
                crashes or panics.
 
-config KTHREAD_POISON
-       bool "Kthread Poison Canary"
-       default n
-       help
-               Kthreads will use a bit of storage at the bottom of the stack to track
-               the state of the kthread.  This is useful to catch various bugs with
-               kthreading, such as launching the same kthread twice concurrently.
-
 config DISABLE_SMT
        bool "Disables symmetric multithreading"
        default n
index f69b692..ff5c2ee 100644 (file)
@@ -40,7 +40,6 @@ CONFIG_KFS_CPIO_BIN=""
 #
 # CONFIG_SPINLOCK_DEBUG is not set
 # CONFIG_SEQLOCK_DEBUG is not set
-# CONFIG_KTHREAD_POISON is not set
 # CONFIG_DISABLE_SMT is not set
 # CONFIG_PRINTK_NO_BACKSPACE is not set
 
index 2a6134c..2780b4f 100644 (file)
@@ -162,10 +162,6 @@ void idt_init(void)
        x86_sysenter_init();
        set_stack_top((uintptr_t)bootstacktop);
 
-#ifdef CONFIG_KTHREAD_POISON
-       *kstack_bottom_addr((uintptr_t)bootstacktop) = 0xdeadbeef;
-#endif /* CONFIG_KTHREAD_POISON */
-
        /* Initialize the TSS field of the gdt.  The size of the TSS desc differs
         * between 64 and 32 bit, hence the pointer acrobatics */
        syssegdesc_t *ts_slot = (syssegdesc_t*)&gdt[GD_TSS >> 3];
index 863a983..ed0e43d 100644 (file)
@@ -113,9 +113,6 @@ static inline bool is_ktask(struct kthread *kthread)
        return kthread->flags & KTH_IS_KTASK;
 }
 
-/* Debugging */
-void check_poison(char *msg);
-
 void sem_init(struct semaphore *sem, int signals);
 void sem_init_irqsave(struct semaphore *sem, int signals);
 bool sem_trydown(struct semaphore *sem);
index 0978689..518f37a 100644 (file)
@@ -115,9 +115,6 @@ void __use_real_kstack(void (*f)(void *arg))
        uintptr_t new_stacktop;
 
        new_stacktop = get_kstack();
-#ifdef CONFIG_KTHREAD_POISON
-       *kstack_bottom_addr(new_stacktop) = 0xdeadbeef;
-#endif /* CONFIG_KTHREAD_POISON */
        set_stack_top(new_stacktop);
        __reset_stack_pointer(0, new_stacktop, f);
 }
@@ -149,16 +146,6 @@ void restart_kthread(struct kthread *kthread)
        /* When a kthread runs, its stack is the default kernel stack */
        set_stack_top(kthread->stacktop);
        pcpui->cur_kthread = kthread;
-#ifdef CONFIG_KTHREAD_POISON
-       /* Assert and switch to cur stack not in use, kthr stack in use */
-       uintptr_t *cur_stack_poison, *kth_stack_poison;
-       cur_stack_poison = kstack_bottom_addr(current_stacktop);
-       assert(*cur_stack_poison == 0xdeadbeef);
-       *cur_stack_poison = 0;
-       kth_stack_poison = kstack_bottom_addr(kthread->stacktop);
-       assert(!*kth_stack_poison);
-       *kth_stack_poison = 0xdeadbeef;
-#endif /* CONFIG_KTHREAD_POISON */
        /* Only change current if we need to (the kthread was in process context) */
        if (kthread->proc) {
                if (kthread->proc == pcpui->cur_proc) {
@@ -307,18 +294,6 @@ void ktask(char *name, void (*fn)(void*), void *arg)
                            (long)name, KMSG_ROUTINE);
 }
 
-void check_poison(char *msg)
-{
-#ifdef CONFIG_KTHREAD_POISON
-       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-       if (pcpui->cur_kthread && pcpui->cur_kthread->stacktop &&
-           (*kstack_bottom_addr(pcpui->cur_kthread->stacktop) != 0xdeadbeef)) {
-               printk("\nBad kthread canary, msg: %s\n", msg);
-               panic("");
-       }
-#endif /* CONFIG_KTHREAD_POISON */
-}
-
 /* Semaphores, using kthreads directly */
 static void debug_downed_sem(struct semaphore *sem);
 static void debug_upped_sem(struct semaphore *sem);
@@ -427,23 +402,10 @@ void sem_down(struct semaphore *sem)
                new_kthread->flags = KTH_DEFAULT_FLAGS;
                new_stacktop = get_kstack();
                new_kthread->stacktop = new_stacktop;
-#ifdef CONFIG_KTHREAD_POISON
-               *kstack_bottom_addr(new_stacktop) = 0;
-#endif /* CONFIG_KTHREAD_POISON */
        }
        /* Set the core's new default stack and kthread */
        set_stack_top(new_stacktop);
        pcpui->cur_kthread = new_kthread;
-#ifdef CONFIG_KTHREAD_POISON
-       /* Mark the new stack as in-use, and unmark the current kthread */
-       uintptr_t *new_stack_poison, *kth_stack_poison;
-       new_stack_poison = kstack_bottom_addr(new_stacktop);
-       assert(!*new_stack_poison);
-       *new_stack_poison = 0xdeadbeef;
-       kth_stack_poison = kstack_bottom_addr(kthread->stacktop);
-       assert(*kth_stack_poison == 0xdeadbeef);
-       *kth_stack_poison = 0;
-#endif /* CONFIG_KTHREAD_POISON */
        /* Kthreads that are ktasks are not related to any process, and do not need
         * to work in a process's address space.  They can operate in any address
         * space that has the kernel mapped (like boot_pgdir, or any pgdir).  Some
@@ -496,11 +458,6 @@ void sem_down(struct semaphore *sem)
        /* Save the allocs as the spare */
        assert(!pcpui->spare);
        pcpui->spare = new_kthread;
-#ifdef CONFIG_KTHREAD_POISON
-       /* switch back to old stack in use, new one not */
-       *new_stack_poison = 0;
-       *kth_stack_poison = 0xdeadbeef;
-#endif /* CONFIG_KTHREAD_POISON */
 block_return_path:
        printd("[kernel] Returning from being 'blocked'! at %llu\n", read_tsc());
        /* restart_kthread and longjmp did not reenable IRQs.  We need to make sure
index 5128b41..eec17e6 100644 (file)
@@ -258,7 +258,6 @@ void printfmt(void (*putch)(int, void**), void **putdat, const char *fmt, ...)
        va_start(ap, fmt);
        vprintfmt(putch, putdat, fmt, ap);
        va_end(ap);
-       check_poison("printfmt");
 }
 
 typedef struct sprintbuf {
@@ -305,7 +304,6 @@ int snprintf(char *buf, int n, const char *fmt, ...)
        rc = vsnprintf(buf, n, fmt, ap);
        va_end(ap);
 
-       check_poison("snprintf");
        return rc;
 }
 
@@ -322,7 +320,6 @@ char *seprintf(char *buf, char *end, const char *fmt, ...)
        va_start(ap, fmt);
        rc = vsnprintf(buf, n, fmt, ap);
        va_end(ap);
-       check_poison("seprintf");
 
        if (rc >= 0)
                return buf + rc;
index bb9233c..ef06842 100644 (file)
@@ -115,9 +115,6 @@ void smp_percpu_init(void)
        STAILQ_INIT(&per_cpu_info[coreid].routine_amsgs);
        /* Initialize the per-core timer chain */
        init_timer_chain(&per_cpu_info[coreid].tchain, set_pcpu_alarm_interrupt);
-#ifdef CONFIG_KTHREAD_POISON
-       *kstack_bottom_addr(kthread->stacktop) = 0xdeadbeef;
-#endif /* CONFIG_KTHREAD_POISON */
        /* Init generic tracing ring */
        trace_buf = kpage_alloc_addr();
        assert(trace_buf);