On 26/06/2015 18:09, Frederic Konrad wrote: >>> >>> +void async_run_safe_work_on_cpu(CPUState *cpu, void (*func)(void >>> *data), >>> + void *data) >>> +{ >> Do you need a mutex to protect this data structure? I would use one >> even if not strictly necessary, to avoid introducing new BQL-protected >> structures. > > For the moment it's called by tb_invalidate and tb_flush_safe the second > lacks a > tb_lock/unlock which should be added. I don't need an other mutex expect > if this is > used elsewhere?
In any case, the locking policy should be documented. At which point you realize that protecting a CPU's queued_safe_work_{first,next} fields with the tb_lock is a bit weird. :) I would add a mutex inside CPUState, and then later we could also use it for regular run_on_cpu/async_run_on_cpu. Paolo