kth: Remove irq_state from sem_.*irqsave's interface 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>
diff --git a/kern/include/kthread.h b/kern/include/kthread.h index 3a4bdb5..115b552 100644 --- a/kern/include/kthread.h +++ b/kern/include/kthread.h
@@ -147,13 +147,11 @@ 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);
diff --git a/kern/src/kthread.c b/kern/src/kthread.c index f407f1d..c5be337 100644 --- a/kern/src/kthread.c +++ b/kern/src/kthread.c
@@ -529,41 +529,44 @@ 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; }