EC: Add priority levels for deferred routines? |
||
Issue descriptionI was wondering if this was a good idea or not. What are your thoughts on adding priority levels to deferred routines? If multiple deferred routines are ready to run when the HOOK task checks for them, I don't think the order in which they're processed is deterministic. It's solely dependent upon the order that the linker places them in and I think that comes from the order that the linker happens to see them in. I imagine this also changes when link-time optimizations are enabled. My proposal is pretty simple. Assign a default priority, but introduce a new macro which can assign a custom priority to a deferred function. (all existing deferred functions use the default priority) Sort the deferred functions by priority in the linker script. Then if multiple deferred functions of different priorities are ready to run, the highest priority is executed first. Usually this isn't a problem as deferred functions don't (shouldn't?) care about what order they're run in with respect to each other. Some background: I was debugging a deadlock in https://chromium-review.googlesource.com/c/445804/1/board/cr50/wp.c#199 where a task held a lock, a deferred function (A) wanted to acquire that lock, but there was also a deferred function (B) that would cause the task to release the lock. Basically, I wanted B to run before A. I could do it with time, that is make A wait longer to run than B, but adding priorities seemed more robust. Thoughts?
,
Feb 24 2017
It wouldn't be hard to add priorities. We already have them for hooks.
But it seems like the real problem is the deferred function will deadlock, and that we only have mutex_lock() and not mutex_try_lock().
Then the deferred function could do:
if (mutex_try_lock(&m) != 0) {
hook_call_deferred(me, 10*MSEC);
return;
}
,
Feb 24 2017
Other alternatives are restructuring the locking so it doesn't get held across idle-level tasks, or some way of having a mutex call a deferred function when it unlocks (though yecch, what if multiple deferred functions are waiting on the same mutex?)
,
Feb 24 2017
Sure, a function like that function would help in general. But there should be no cases in the code when one task locks the mutex and another one unlocks it, unless there is a damn good reason for that.
,
Feb 24 2017
Yeah, totally agree. This was just something that came to mind when I was trying to work around it.
,
Feb 25 2017
Aseda, I finally followed your analysis of the problem, it looks like making sure that this is the tpm task who releases the lock is not going to help much, as the tpm task would need an event to indicate that it's time to release the lock, and this event is supposed to come from the hooks task which would be blocked waiting for the lock to be released. I guess the proper way to fix this would be to have a common api to send the tpm task the event triggering releasing the lock. It could be sent from both hook and console tasks.
,
Mar 30 2017
It's not hard to add, but these aren't needed.
,
Apr 20 2017
For posterity, here was my attempt: https://chromium-review.googlesource.com/c/446785/ |
||
►
Sign in to add a comment |
||
Comment 1 by vbendeb@google.com
, Feb 24 2017