On 11/04/2012 10:30 AM, Jan Kiszka wrote: > From: Jan Kiszka <jan.kis...@siemens.com> > > Cirrus is triggering this, e.g. during Win2k boot: Changes only on > disabled regions require no topology update when transaction depth drops > to 0 again.
817dcc5368988b0 (pci: give each device its own address space) mad this much worse by multiplying the number of address spaces. Each change is now evaluated N+2 times, where N is the number of PCI devices. It also causes a corresponding expansion in memory usage. I want to address this by caching AddressSpaceDispatch trees with the key being the contents of the FlatView for that address space. This will drop the number of distinct trees to 2-4 (3 if some devices have PCI_COMMAND_MASTER disabled, 4 if the PCI address space is different from the cpu memory address space) but will fail if we make each address space different (for example filtering out the device's own BARs). If this change also improves cpu usage sufficiently, then it will be better than your patch, which doesn't recognize changes in an enabled region inside a disabled or hidden region. In other words, your patch fits the problem at hand but isn't general. On the other hand my approach doesn't eliminate render_memory_region(), just the exec.c stuff and listener updates. So we need to understand where the slowness comes from. > > @@ -1543,6 +1554,7 @@ void address_space_init(AddressSpace *as, MemoryRegion > *root) > flatview_init(as->current_map); > QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link); > as->name = NULL; > + memory_region_update_pending = true; > memory_region_transaction_commit(); > address_space_init_dispatch(as); > } > @@ -1552,6 +1564,7 @@ void address_space_destroy(AddressSpace *as) > /* Flush out anything from MemoryListeners listening in on this */ > memory_region_transaction_begin(); > as->root = NULL; > + memory_region_update_pending = true; > memory_region_transaction_commit(); > QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); > address_space_destroy_dispatch(as); init and destroy cannot happen to a region that is mapped (and cannot happen within a transaction), so these two are redundant. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.