On 06/14/2018 01:49 PM, Andrea Parri wrote:
[...]
/** wake_up_process() paired with set_current_state() inserts* sufficient barriers to make sure @owner either sees it's* wounded or has a wakeup pending to re-read the wounded* state.IIUC, "sufficient barriers" = full memory barriers (here). (You may want to be more specific.)
Thanks for reviewing! OK. What about if someone relaxes that in the future?
This is actually one of my main concerns ;-) as, IIUC, those barriers are not only sufficient but also necessary: anything "less than a full barrier" (in either wake_up_process() or set_current_state()) would _not_ guarantee the "condition" above unless I'm misunderstanding it.
But am I misunderstanding it? Which barriers/guarantee do you _need_ from the above mentioned pairing? (hence my comment...)
Andrea
No you are probably not misunderstanding me at all. My comment originated from the reading of the kerneldoc of set_current_state()
* set_current_state() includes a barrier so that the write of current->state * is correctly serialised wrt the caller's subsequent test of whether to * actually sleep: * * for (;;) { * set_current_state(TASK_UNINTERRUPTIBLE); * if (!need_sleep) * break; * * schedule(); * } * __set_current_state(TASK_RUNNING); * * If the caller does not need such serialisation (because, for instance, the * condition test and condition change and wakeup are under the same lock) then * use __set_current_state(). * * The above is typically ordered against the wakeup, which does: * * need_sleep = false; * wake_up_state(p, TASK_UNINTERRUPTIBLE); * * Where wake_up_state() (and all other wakeup primitives) imply enough * barriers to order the store of the variable against wakeup. And with ctx->wounded := !need_sleep this exactly matches what's happening in my code. So what I was trying to say in the comment was that this above contract is sufficient to guarantee the "condition" above, whitout me actually knowing exactly what barriers are required. Thanks, Thomas