kth: Remove irq_state from sem_.*irqsave's interface
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 17 Dec 2018 16:04:57 +0000 (11:04 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 18 Dec 2018 20:30:28 +0000 (15:30 -0500)
If disable_irqsave() and enable_irqsave() are in the same function, you
don't need to have the state passed in.

I stumbled on this while looking into whether or not CVs could drop the
irqstate as well.  The short version is 'no'.

Our kernel CVs just use spinlocks.  The particular lock and use case of
the CVs is up to our caller, so whether or not that lock is used from
IRQ context is beyond our control.

It would be nice to be able to use the functionality of
spin_lock_irqsave() to track whether or not we need to restore IRQs when
we return.  However, we unlock and relock that spinlock during the
process of waiting and restarting.  At that point, any information about
whether or not we should reenable IRQs is gone.  Thus we need to track
it with irqstate.

Note that the cv_wait code tracks whether or not IRQs were enabled *at
the moment*, but we also need to know if IRQs were enabled at the point
of the cv_lock_irqsave() and to carry that information over to
cv_unlock_irqsave().

Also, none of the CV callers are from IRQ ctx anymore.  At most, all are
from RKMs.  When analyzing CVs, we only care about the wakeup side too
- you're not allowed to sleep from IRQ context.  And no one even calls
sem_.*irqsave().

I considered removing the CV irqsave functions (and sems), but given the
flux in alarm code, there might come a time where we kick CVs from IRQ
context again, to include immediate kernel messages.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/kthread.h
kern/src/kthread.c

index 3a4bdb5..115b552 100644 (file)
@@ -147,13 +147,11 @@ bool sem_trydown(struct semaphore *sem);
 void sem_down_bulk(struct semaphore *sem, int nr_signals);
 void sem_down(struct semaphore *sem);
 bool sem_up(struct semaphore *sem);
-bool sem_trydown_bulk_irqsave(struct semaphore *sem, int nr_signals,
-                              int8_t *irq_state);
-bool sem_trydown_irqsave(struct semaphore *sem, int8_t *irq_state);
-void sem_down_bulk_irqsave(struct semaphore *sem, int nr_signals,
-                           int8_t *irq_state);
-void sem_down_irqsave(struct semaphore *sem, int8_t *irq_state);
-bool sem_up_irqsave(struct semaphore *sem, int8_t *irq_state);
+bool sem_trydown_bulk_irqsave(struct semaphore *sem, int nr_signals);
+bool sem_trydown_irqsave(struct semaphore *sem);
+void sem_down_bulk_irqsave(struct semaphore *sem, int nr_signals);
+void sem_down_irqsave(struct semaphore *sem);
+bool sem_up_irqsave(struct semaphore *sem);
 void print_db_blk_info(pid_t pid);
 
 void cv_init(struct cond_var *cv);
index f407f1d..c5be337 100644 (file)
@@ -529,41 +529,44 @@ bool sem_up(struct semaphore *sem)
        return FALSE;
 }
 
-bool sem_trydown_bulk_irqsave(struct semaphore *sem, int nr_signals,
-                              int8_t *irq_state)
+bool sem_trydown_bulk_irqsave(struct semaphore *sem, int nr_signals)
 {
        bool ret;
+       int8_t irq_state = 0;
 
-       disable_irqsave(irq_state);
+       disable_irqsave(&irq_state);
        ret = sem_trydown_bulk(sem, nr_signals);
-       enable_irqsave(irq_state);
+       enable_irqsave(&irq_state);
        return ret;
 }
 
-bool sem_trydown_irqsave(struct semaphore *sem, int8_t *irq_state)
+bool sem_trydown_irqsave(struct semaphore *sem)
 {
-       return sem_trydown_bulk_irqsave(sem, 1, irq_state);
+       return sem_trydown_bulk_irqsave(sem, 1);
 }
 
-void sem_down_bulk_irqsave(struct semaphore *sem, int nr_signals,
-                           int8_t *irq_state)
+void sem_down_bulk_irqsave(struct semaphore *sem, int nr_signals)
 {
-       disable_irqsave(irq_state);
+       int8_t irq_state = 0;
+
+       disable_irqsave(&irq_state);
        sem_down_bulk(sem, nr_signals);
-       enable_irqsave(irq_state);
+       enable_irqsave(&irq_state);
 }
 
-void sem_down_irqsave(struct semaphore *sem, int8_t *irq_state)
+void sem_down_irqsave(struct semaphore *sem)
 {
-       sem_down_bulk_irqsave(sem, 1, irq_state);
+       sem_down_bulk_irqsave(sem, 1);
 }
 
-bool sem_up_irqsave(struct semaphore *sem, int8_t *irq_state)
+bool sem_up_irqsave(struct semaphore *sem)
 {
        bool retval;
-       disable_irqsave(irq_state);
+       int8_t irq_state = 0;
+
+       disable_irqsave(&irq_state);
        retval = sem_up(sem);
-       enable_irqsave(irq_state);
+       enable_irqsave(&irq_state);
        return retval;
 }