alarm: Force unset_alarm to grab the CV lock
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 16 Oct 2018 14:20:40 +0000 (10:20 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 22 Oct 2018 23:35:51 +0000 (19:35 -0400)
commitc62a28c7897607e839d87620da21410a96615f39
tree7d412f820d873761b843df42918f88bc074d232f
parentce5ff2deabf58b64c1d72f028083f464fcb0370c
alarm: Force unset_alarm to grab the CV lock

The issue here is that unset_alarm() is now used to mean "is the alarm
service done with the alarm."  Previously, it was "has the function run
yet."

This all is overly fucked up, due to a bunch of things:
- The alarm callbacks allegedly can block.  Not sure if any of them
  actually need that.
- Unset blocks until a CB is done, meaning it might sleep.
- So we need the CV and flags, and need to do things *after* the
  function pointer runs.
- That means the caller/user needs to be aware of the alarm service, and
  can't reuse/free the alarm right away.
- Alarms can re-arm from within their callbacks, and the service doesn't
  know about it.  Hell, what if they put themselves on another timer
chain?

Even with this change, we still have some issues.  Say you set_alarm()
from within a callback.  Then that alarm fires and runs before the alarm
service finishes it's CV work?  This could easily happen with a global
tchain (like parlib provides).  It could also happen with a per-core
timer chain, if the callback calls set_alarm() and then blocks.  Anyway,
under this situation, we could have *two* contexts trying to finish the
same alarm at the same time, where one of them clears is_running while
the other is still running.

Oh, and if an alarm CB blocks, under the current implementation, it'll
block the other unrelated alarms after it.  Not ideal.

The longer term fix might be to not allow alarm CBs to block (in
userspace, this means no uthread mutexes, which is actually already the
case since they run in vcore context).  This actually makes it a lot
more like Linux's timer stuff: they run in softirq context only (not
hard irq), though their del_timer_sync is a bit nasty.  They allow
mod_timer() to be called from within the CB, though maybe a little
discipline could help out with some of the corner cases.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/alarm.c
user/parlib/alarm.c