On 2013-05-07 20:00, Jan Kiszka wrote: > On 2013-05-07 16:17, Paolo Bonzini wrote: >> With this change, a FlatView can be used even after a concurrent >> update has replaced it. Because we do not have RCU, we use a >> mutex to protect the small critical sections that read/write the >> as->current_map pointer. Accesses to the FlatView can be done >> outside the mutex. >> >> If a MemoryRegion will be used after the FlatView is unref-ed (or after >> a MemoryListener callback is returned), a reference has to be added to >> that MemoryRegion. For example, memory_region_find adds a reference to >> the MemoryRegion that it returns. > > For my understanding: Every lookup, e.g. triggered by address_space_rw, > will briefly reference the FlatView of that address space and drop that > reference again after referencing the target memory region. > > Provided that is true: If we run that lookup on an address space that > happens to be modified in parallel, the lookup may actually cause the > final deref and, thus, release of the FlatView - even if the target > memory region was totally unrelated to the ongoing change. That could > make a hot-path pay the price of an action a slow path caused. Not > really a beautiful concept. Even if the FlatView finalization is a > simple free() (that is the minimum), we would pull the memory allocator > into code paths that might try hard to keep a safe distance for the sake > of predictability.
A mitigation could be to off-load the finalization to a bottom-half e.g., ie. let the main iothread do the dirty work. Similar to the RCU handler off-loading of the kernel. There is just a theoretical risk that we pile up finalization requests and run out of memory. But all that is pretty complicated and not really elegant. Better avoid the ref/unref over hot-paths for objects of such broad scope like the FlatView. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
