Adds some event/uthread debugging code
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 2 Oct 2012 00:14:20 +0000 (17:14 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 2 Oct 2012 00:14:20 +0000 (17:14 -0700)
And a better rassert.  When I have weird low-level bugs, I plan to
include rassert.h instead of using the glibc assert.  Might make it the
default for all of parlib at some point.

kern/src/event.c
user/parlib/debug.c
user/parlib/event.c
user/parlib/include/rassert.h
user/parlib/mcs.c

index c89bc2d..5e1ab3c 100644 (file)
@@ -353,6 +353,13 @@ void send_event(struct proc *p, struct event_queue *ev_q, struct event_msg *msg,
                printk("[kernel] Illegal addr for ev_q\n");
                return;
        }
+       /* This should be caught by "future technology" that can tell when the
+        * kernel PFs on the user's behalf.  For now, we catch common userspace bugs
+        * (had this happen a few times). */
+       if (!PTE_ADDR(ev_q)) {
+               printk("[kernel] Bad addr %08p for ev_q\n", ev_q);
+               return;
+       }
        /* ev_q is a user pointer, so we need to make sure we're in the right
         * address space */
        old_proc = switch_to(p);
index eb3b8bd..354589c 100644 (file)
@@ -56,4 +56,3 @@ int ros_debug(const char *fmt, ...)
 
        return cnt;
 }
-
index 8e4acf6..9bcf094 100644 (file)
@@ -178,6 +178,8 @@ static int handle_mbox_msgs(struct event_mbox *ev_mbox)
        struct event_msg local_msg;
        unsigned int ev_type;
        uint32_t vcoreid = vcore_id();
+       /* Some stack-smashing bugs cause this to fail */
+       assert(ev_mbox);
        /* Try to dequeue, dispatch whatever you get. */
        while (!get_ucq_msg(&ev_mbox->ev_msgs, &local_msg)) {
                ev_type = local_msg.ev_type;
@@ -305,6 +307,8 @@ void handle_event_q(struct event_queue *ev_q)
                return;
        }
        printd("[event] handling ev_q %08p on vcore %d\n", ev_q, vcore_id());
+       /* Raw ev_qs that haven't been connected to an mbox, user bug: */
+       assert(ev_q->ev_mbox);
        handle_mbox(ev_q->ev_mbox);
 }
 
index 7e249d9..72e231c 100644 (file)
@@ -5,6 +5,7 @@
 
 #include <assert.h>
 #include <vcore.h>
+#include <ros_debug.h>
 #undef assert
 
 void _warn(const char*, int, const char*, ...);
@@ -13,8 +14,15 @@ void _panic(const char*, int, const char*, ...) __attribute__((noreturn));
 #define warn(...) _warn(__FILE__, __LINE__, __VA_ARGS__)
 #define panic(...) _panic(__FILE__, __LINE__, __VA_ARGS__)
 
-#define assert(x)              \
-       do { if (!(x)) panic("assertion failed - vcore %d: %s", vcore_id(), #x); } while (0)
+#define assert(x)                                                                 \
+       do {                                                                       \
+               if (!(x)) {                                                            \
+                       ros_debug("[user] %s:%d, vcore %d, Assertion failed: %s\n",        \
+                                 __FILE__, __LINE__, vcore_id(), #x);                     \
+                       breakpoint();                                                      \
+                       abort();                                                           \
+               }                                                                      \
+       } while (0)
 
 // static_assert(x) will generate a compile-time error if 'x' is false.
 #define static_assert(x)       switch (x) case 0: case (x):
index 95a5083..03c8dca 100644 (file)
@@ -172,6 +172,7 @@ void mcs_pdr_init(struct mcs_pdr_lock *lock)
        lock->lock_holder = 0;
        lock->vc_qnodes = malloc(sizeof(struct mcs_pdr_qnode) * max_vcores());
        assert(lock->vc_qnodes);
+       memset(lock->vc_qnodes, 0, sizeof(struct mcs_pdr_qnode) * max_vcores());
        for (int i = 0; i < max_vcores(); i++)
                lock->vc_qnodes[i].vcoreid = i;
 }
@@ -192,6 +193,9 @@ void mcs_pdr_fini(struct mcs_pdr_lock *lock)
  * wastefully spin, we're okay. */
 static void __ensure_qnode_runs(struct mcs_pdr_qnode *qnode)
 {
+       /* This assert is somewhat useless.  __lock will compile it out, since it is
+        * impossible.  If you have a PF around here in __lock, odds are your stack
+        * is getting gently clobbered (syscall finished twice?). */
        assert(qnode);
        ensure_vcore_runs(qnode->vcoreid);
 }
@@ -239,7 +243,7 @@ void __mcs_pdr_unlock(struct mcs_pdr_lock *lock, struct mcs_pdr_qnode *qnode)
                cmb();  /* no need for CPU mbs, since there's an atomic_cas() */
                /* If we're still the lock, just swap it with 0 (unlock) and return */
                if (atomic_cas_ptr((void**)&lock->lock, qnode, 0))
-                       return;
+                       goto out;
                cmb();  /* need to read a fresh tail.  the CAS was a CPU mb */
                /* Read in the tail (or someone who recent was the tail, but could now
                 * be farther up the chain), in prep for our spinning. */
@@ -266,6 +270,9 @@ void __mcs_pdr_unlock(struct mcs_pdr_lock *lock, struct mcs_pdr_qnode *qnode)
                /* simply unlock whoever is next */
                qnode->next->locked = 0;
        }
+out:
+       /* ease of debugging */
+       qnode->next = 0;
 }
 
 /* Actual MCS-PDR locks.  These use memory in the lock for their qnodes, though