On Thu, May 9, 2013 at 10:50 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > Il 09/05/2013 02:53, liu ping fan ha scritto: >> On Wed, May 8, 2013 at 11:44 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: >>> Il 08/05/2013 08:20, liu ping fan ha scritto: >>>> On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: >>>>> Hi, >>>>> >>>>> this is an alternative approach to refactoring of dataplane's HostMem >>>>> code. Here, I take Ping Fan's idea of RCU-style updating of the >>>>> region list and apply it to the AddressSpace's FlatView. With this >>>> >>>> In fact, I am worrying about the priority of MemoryListener, if it is >>>> true, then we should drop RCU-style idea. >>> >>> You mean in hostmem, or in general as in this patch? Note that this >>> patch releases the old FlatView at the end of all MemoryListener operations. >>> >> Both in hostmem and this patch, they all broke the original design of >> the MemoryListener, see notes for priority in code. > > I think both hostmem and this patch are fine. The hypervisor is never > involved, all accesses go through the "old" FlatView and regions cannot > disappear thanks to ref/unref. > Here, we worry about add, not del.
> In fact, we need _more_ RCU-style updates, not less. For BQL-less > dispatch, address space mapping/translation can race against the > MemoryListeners in exec.c. To fix this, phys_sections and > AddressSpaceDispatch need to be reference counted and RCU-ified as well. > Agree, I like RCU too. > Paolo > >> I have set out 2 patches to highlight this issue, and have CC you and >> Stefanha. >> >> Regards, >> Pingfan >> >>> Paolo >>> >>>> Also if it is true, there is >>>> already a bug with hostmem listener. It should use region_del, not >>>> region_nop to reconstruct the local view. But just let me have a deep >>>> thinking. >>>> >>>> Regards, >>>> Pingfan >>>>> change, dataplane can simply use memory_region_find instead of >>>>> hostmem. >>>>> >>>>> This is a somewhat larger change, but I prefer it for two reasons. >>>>> >>>>> 1) it splits the task of adding BQL-less memory dispatch in two parts, >>>>> tacking memory_region_find first (which is simpler because locking >>>>> is left to the caller). >>>>> >>>>> 2) HostMem duplicates a lot of the FlatView logic, and adding the >>>>> RCU-style update in FlatView benefits everyone. >>>>> >>>>> The missing ingredients here are: >>>>> >>>>> 1) remember and unreference the MemoryRegions that are used in >>>>> a vring entry. In order to implement this, it is probably simpler >>>>> to change vring.c to use virtio.c's VirtQueueElement data structure. >>>>> We want something like that anyway in order to support migration. >>>>> >>>>> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions >>>>> for hot-unpluggable devices. In this series, ref/unref are stubs. >>>>> >>>>> For simplicity I based the patches on my IOMMU rebase. I placed the >>>>> tree at git://github.com/bonzini/qemu.git, branch iommu. >>>>> >>>>> Paolo >>>>> >>>>> Paolo Bonzini (8): >>>>> memory: add ref/unref calls >>>>> exec: check MRU in qemu_ram_addr_from_host >>>>> memory: return MemoryRegion from qemu_ram_addr_from_host >>>>> memory: ref/unref memory across address_space_map/unmap >>>>> memory: access FlatView from a local variable >>>>> memory: use a new FlatView pointer on every topology update >>>>> memory: add reference counting to FlatView >>>>> dataplane: replace hostmem with memory_region_find >>>>> >>>>> exec.c | 63 +++++++++--- >>>>> hw/core/loader.c | 1 + >>>>> hw/display/exynos4210_fimd.c | 6 + >>>>> hw/display/framebuffer.c | 10 +- >>>>> hw/i386/kvm/ioapic.c | 2 + >>>>> hw/i386/kvmvapic.c | 1 + >>>>> hw/misc/vfio.c | 2 + >>>>> hw/virtio/dataplane/Makefile.objs | 2 +- >>>>> hw/virtio/dataplane/hostmem.c | 176 >>>>> --------------------------------- >>>>> hw/virtio/dataplane/vring.c | 56 +++++++++-- >>>>> hw/virtio/vhost.c | 2 + >>>>> hw/virtio/virtio-balloon.c | 1 + >>>>> hw/xen/xen_pt.c | 4 + >>>>> include/exec/cpu-common.h | 2 +- >>>>> include/exec/memory.h | 9 ++ >>>>> include/hw/virtio/dataplane/hostmem.h | 57 ----------- >>>>> include/hw/virtio/dataplane/vring.h | 3 +- >>>>> kvm-all.c | 2 + >>>>> memory.c | 142 +++++++++++++++++++++----- >>>>> target-arm/kvm.c | 2 + >>>>> target-i386/kvm.c | 4 +- >>>>> target-sparc/mmu_helper.c | 1 + >>>>> xen-all.c | 2 + >>>>> 23 files changed, 253 insertions(+), 297 deletions(-) >>>>> delete mode 100644 hw/virtio/dataplane/hostmem.c >>>>> delete mode 100644 include/hw/virtio/dataplane/hostmem.h >>>>> >>> >