On 19/10/2017 22:11, Emilio G. Cota wrote: > On Thu, Oct 19, 2017 at 15:05:17 +0200, Paolo Bonzini wrote: >> On 19/10/2017 00:45, Emilio G. Cota wrote: >>> I have just pushed a branch on top of this series that includes >>> 10 patches that further pave the way for the removal of tb_lock: >>> >>> https://github.com/cota/qemu/tree/multi-tcg-v6-plus >> >> I started reviewing those, > > Nice, thanks! > >> I have a few questions: >> >> 1) why is tcg_region_tree separate from tcg_region_state? Would it make >> sense to prepare a linked list of tcg_region_state structs, and reuse >> the region lock for the region tree? > > I think the naming here might be confusing; "tcg_region_state" should be > understood as "tcg_region_global_state". IOW, there is no per-region struct. > > That said, the array of per-region trees could be embedded in this global > struct. I was hesitant to do so because then one could think that > region_state.lock and rt.lock are somehow related; they are not.
Ok, this is clearer now. >> 2) in tb_for_each_tagged_safe, could the "prev" argument instead be >> "next", like > > Is this just to make them closer to the macros in queue.h? > > In this case tracking *prev in the loop (rather than next) is > useful because it makes removing the "current" element very simple: This actually makes a lot of sense. Maybe we should change queue.h the other way. ;) Can you rename "prev" to "pprev" though? Paolo