Add memory clobber to RISC-V set_stack_pointer
[akaros.git] / user / parlib / mcs.c
index ef0cb15..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;
 }
@@ -190,19 +191,13 @@ void mcs_pdr_fini(struct mcs_pdr_lock *lock)
  * Even if they are no longer the lockholder, they will be checking preemption
  * messages and will help break out of the deadlock.  So long as we don't
  * wastefully spin, we're okay. */
-void __ensure_qnode_runs(struct mcs_pdr_qnode *qnode)
+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);
-       if (vcore_is_preempted(qnode->vcoreid)) {
-               printd("MCS-PDR: VC %d changing to VC %d\n", vcore_id(), qnode->vcoreid);
-               /* Note that at this moment, the vcore could still be mapped (we're
-                * racing with __preempt.  If that happens, we'll just fail the
-                * sys_change_vcore(), and next time __ensure runs we'll get it. */
-               /* We want to recover them from preemption.  Since we know they have
-                * notifs disabled, they will need to be directly restarted, so we can
-                * skip the other logic and cut straight to the sys_change_vcore() */
-               sys_change_vcore(qnode->vcoreid, FALSE);
-       }
+       ensure_vcore_runs(qnode->vcoreid);
 }
 
 /* Internal version of the locking function, doesn't care about storage of qnode
@@ -248,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. */
@@ -275,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
@@ -292,20 +290,16 @@ void mcs_pdr_lock(struct mcs_pdr_lock *lock)
        __mcs_pdr_lock(lock, qnode);
 }
 
-void mcs_pdr_unlock(struct mcs_pdr_lock *lock)
-{
-       struct mcs_pdr_qnode *qnode = &lock->vc_qnodes[vcore_id()];
-       assert(qnode->vcoreid == vcore_id());   /* sanity */
-       __mcs_pdr_unlock(lock, qnode);
-       /* Enable notifs, if we're an _M uthread */
-       uth_enable_notifs();
-}
-
-#if 0
-/* We don't actually use this.  To use this, you'll need the unlock code to save
- * pred to a specific field in the qnode and check both its initial pred as well
- * as its run time pred (who could be an usurper).  It's all possible, but a
- * little more difficult to follow. */
+/* CAS-less unlock, not quite as efficient and will make sure every vcore runs
+ * (since we don't have a convenient way to make sure our qnode->next runs
+ * yet, other than making sure everyone runs).
+ *
+ * To use this without ensuring all vcores run, you'll need the unlock code to
+ * save pred to a specific field in the qnode and check both its initial pred
+ * as well as its run time pred (who could be an usurper).  It's all possible,
+ * but a little more difficult to follow.  Also, I'm adjusting this comment
+ * months after writing it originally, so it is probably not sufficient, but
+ * necessary. */
 void __mcs_pdr_unlock_no_cas(struct mcs_pdr_lock *lock,
                              struct mcs_pdr_qnode *qnode)
 {
@@ -325,8 +319,10 @@ void __mcs_pdr_unlock_no_cas(struct mcs_pdr_lock *lock,
                /* since someone else was waiting, they should have made themselves our
                 * next.  spin (very briefly!) til it happens. */
                while (qnode->next == 0) {
-                       /* make sure old_tail isn't preempted */
-
+                       /* make sure old_tail isn't preempted.  best we can do for now is
+                        * to make sure all vcores run, and thereby get our next. */
+                       for (int i = 0; i < max_vcores(); i++)
+                               ensure_vcore_runs(i);
                        cpu_relax();
                }
                if (usurper) {
@@ -337,8 +333,12 @@ void __mcs_pdr_unlock_no_cas(struct mcs_pdr_lock *lock,
                         * First, we need to change our next's pred.  There's a slight race
                         * here, so our next will need to make sure both us and pred are
                         * done */
-                       qnode->next->pred = usurper;
-                       wmb();
+                       /* I was trying to do something so we didn't need to ensure all
+                        * vcores run, using more space in the qnode to figure out who our
+                        * pred was a lock time (guessing actually, since there's a race,
+                        * etc). */
+                       //qnode->next->pred = usurper;
+                       //wmb();
                        usurper->next = qnode->next;
                        /* could imagine another wmb() and a flag so our next knows to no
                         * longer check us too. */
@@ -355,4 +355,16 @@ void __mcs_pdr_unlock_no_cas(struct mcs_pdr_lock *lock,
                qnode->next->locked = 0;
        }
 }
+
+void mcs_pdr_unlock(struct mcs_pdr_lock *lock)
+{
+       struct mcs_pdr_qnode *qnode = &lock->vc_qnodes[vcore_id()];
+       assert(qnode->vcoreid == vcore_id());   /* sanity */
+#ifndef __riscv__
+       __mcs_pdr_unlock(lock, qnode);
+#else
+       __mcs_pdr_unlock_no_cas(lock, qnode);
 #endif
+       /* Enable notifs, if we're an _M uthread */
+       uth_enable_notifs();
+}