Manually spin_unlock_irqsave and a cleaner destroy
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 13 Nov 2009 00:18:57 +0000 (16:18 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 13 Nov 2009 00:18:57 +0000 (16:18 -0800)
proc_destroy is no longer ghetto when it comes to decrefing, which will
help in some future commits regarding refcnts and atomic operations on
those refcnts.

kern/include/atomic.h
kern/src/process.c

index e8205cc..440e7ef 100644 (file)
@@ -10,6 +10,7 @@ static inline void
 (SLOCK(0) spin_lock_irqsave)(spinlock_t RACY*SAFE lock);
 static inline void
 (SUNLOCK(0) spin_unlock_irqsave)(spinlock_t RACY*SAFE lock);
+static inline bool spin_lock_irq_enabled(spinlock_t *SAFE lock);
 
 /*********************** Checklist stuff **********************/
 typedef struct checklist_mask {
@@ -69,6 +70,9 @@ void init_barrier(barrier_t*COUNT(1) barrier, uint32_t count);
 void reset_barrier(barrier_t* barrier);
 void waiton_barrier(barrier_t* barrier);
 
+/* Spinlock bit flags */
+#define SPINLOCK_IRQ_EN                        0x80000000
+
 // If ints are enabled, disable them and note it in the top bit of the lock
 // There is an assumption about releasing locks in order here...
 static inline void spin_lock_irqsave(spinlock_t *SAFE lock)
@@ -78,18 +82,25 @@ static inline void spin_lock_irqsave(spinlock_t *SAFE lock)
        disable_irq();
        spin_lock(lock);
        if (irq_en)
-               lock->rlock |= 0x80000000;
+               lock->rlock |= SPINLOCK_IRQ_EN;
 }
 
 // if the high bit of the lock is set, then re-enable interrupts
 // (note from asw: you're lucky this works, you little-endian jerks)
 static inline void spin_unlock_irqsave(spinlock_t *SAFE lock)
 {
-       if (lock->rlock & 0x80000000) {
+       if (spin_lock_irq_enabled(lock)) {
                spin_unlock(lock);
                enable_irq();
        } else
                spin_unlock(lock);
 }
 
+/* Returns whether or not unlocking this lock should enable interrupts or not.
+ * Is meaningless on locks that weren't locked with irqsave. */
+static inline bool spin_lock_irq_enabled(spinlock_t *SAFE lock)
+{
+       return lock->rlock & SPINLOCK_IRQ_EN;
+}
+
 #endif /* !ROS_KERN_ATOMIC_H */
index a514ebb..7d792a7 100644 (file)
@@ -583,22 +583,20 @@ void proc_destroy(struct proc *p)
        }
        __proc_set_state(p, PROC_DYING);
 
-       /* TODO: (REF) dirty hack.  decref currently uses a lock, but needs to use
-        * CAS instead (another lock would be slightly less ghetto).  but we need to
-        * decref before releasing the lock, since that could enable interrupts,
-        * which would have us receive the DEATH IPI if this was called locally by
-        * the target process. */
-       //proc_decref(p); // this decref is for the process in general
-       p->env_refcnt--;
-       size_t refcnt = p->env_refcnt; // need to copy this in so it's not reloaded
-
-       /* After unlocking, we can receive a DEATH IPI and clean up */
-       spin_unlock_irqsave(&p->proc_lock);
+       /* Leave interrupts turned off, but unlock.  Store whether or not we should
+        * reenable interrupts down below.  We're manually doing the unlock_irqsave,
+        * since we need to make sure we finish this function before turning
+        * interrupts back on (in case a death/abandon_core comes in). */
+       bool irq_en = spin_lock_irq_enabled(&p->proc_lock);
+       spin_unlock(&p->proc_lock);
 
-       // coupled with the refcnt-- above, from decref.  if this happened,
-       // proc_destroy was called "remotely", and with no one else refcnting
-       if (!refcnt)
-               __proc_free(p);
+       proc_decref(p); // this decref is for the process in general
+       
+       /* Reenable interrupts if we should have from the spin_unlock_irqsave.  We
+        * should almost always be reenabling.  Once we reenable, we can receive a
+        * death IPI (if applicable) and never return. */
+       if (irq_en)
+               enable_irq();
 
        /* if our core is part of process p, then check/wait for the death IPI. */
        check_for_local_death(p);