Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On 02/10/2012 07:16 PM, Marcelo Tosatti wrote: > On Thu, Feb 09, 2012 at 04:25:36PM +0200, Avi Kivity wrote: > > On 02/08/2012 08:45 PM, Marcelo Tosatti wrote: > > > > BTW do we really need fast slot creation/destruction? > > > > > > At the moment yes. Boot a RHEL/Fedora installation disk (or any other > > > guest which uses SYSLINUX splash screen) and you will see. > > > > Another workload that suffers is Windows XP clearing the screen during boot. > > > > > That > > > particular case is a limitation of cirrus in QEMU, ideally it should be > > > optimized there. > > > > Why do you say that? > > There is no fundamental need to create/destroy the 0xa VGA memory > slot repeatedly. If the guest writes to it, then the need exists. > But you are right that the aim should be decent performance > nevertheless. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On 02/10/2012 03:25 PM, Takuya Yoshikawa wrote: > Avi Kivity wrote: > > > > 2. When we create(and shift?) a memory slot, we call > > > kvm_arch_flush_shadow() > > > to clear all mmio sptes, again not restricted to that slot. > > > > > > /* > > >* If the new memory slot is created, we need to clear all > > >* mmio sptes. > > >*/ > > > if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > > > kvm_arch_flush_shadow(kvm); > > > > This is pretty rare outside the previous scenario (memory/pci hotplug). > > Is this condition correct? > > When npages != 0 and old.npages == 0, the slot is being newly created, do we > really need to flush shadow pages? > > This should be > if (npages && old.npages && (old.base_gfn != base_gfn)) > Your condition is more correct, but in practice there's no difference. If old.npages == 0, then old.base_gfn will be 0, and the condition will fail, except for the first slot created (when the shadow cache is empty anyway). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On Fri, Feb 10, 2012 at 10:08:12PM +0900, Takuya Yoshikawa wrote: > Avi Kivity wrote: > > > On 02/09/2012 04:23 PM, Avi Kivity wrote: > > > > BTW do we really need fast slot creation/destruction? > > > > > > Not really, but it's good to have infrastructure that copes with > > > different workloads. If the patches keep the code simple I think it's a > > > good thing to have. > > My patch is pretty simple. > > > To qualify - taking several tens of milliseconds is out of the question > > as some workloads grind to a halt. But it doesn't need to be incredibly > > fast. > > I think the problem is not how long it takes to create/invalidate a slot > but following faults and shadow pages reconstruction. The problem which is immediatelly visible is kvm_set_mem performance. > Similar to mmu shrinker problem we discussed before. > > E.g. users will not expect sudden latency induced by pci hotplug. > > Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On Thu, Feb 09, 2012 at 04:25:36PM +0200, Avi Kivity wrote: > On 02/08/2012 08:45 PM, Marcelo Tosatti wrote: > > > BTW do we really need fast slot creation/destruction? > > > > At the moment yes. Boot a RHEL/Fedora installation disk (or any other > > guest which uses SYSLINUX splash screen) and you will see. > > Another workload that suffers is Windows XP clearing the screen during boot. > > > That > > particular case is a limitation of cirrus in QEMU, ideally it should be > > optimized there. > > Why do you say that? There is no fundamental need to create/destroy the 0xa VGA memory slot repeatedly. But you are right that the aim should be decent performance nevertheless. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
Avi Kivity wrote: > > 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow() > > to clear all mmio sptes, again not restricted to that slot. > > > > /* > > * If the new memory slot is created, we need to clear all > > * mmio sptes. > > */ > > if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > > kvm_arch_flush_shadow(kvm); > > This is pretty rare outside the previous scenario (memory/pci hotplug). Is this condition correct? When npages != 0 and old.npages == 0, the slot is being newly created, do we really need to flush shadow pages? This should be if (npages && old.npages && (old.base_gfn != base_gfn)) No? Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
Avi Kivity wrote: > On 02/09/2012 04:23 PM, Avi Kivity wrote: > > > BTW do we really need fast slot creation/destruction? > > > > Not really, but it's good to have infrastructure that copes with > > different workloads. If the patches keep the code simple I think it's a > > good thing to have. My patch is pretty simple. > To qualify - taking several tens of milliseconds is out of the question > as some workloads grind to a halt. But it doesn't need to be incredibly > fast. I think the problem is not how long it takes to create/invalidate a slot but following faults and shadow pages reconstruction. Similar to mmu shrinker problem we discussed before. E.g. users will not expect sudden latency induced by pci hotplug. Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On 02/08/2012 08:45 PM, Marcelo Tosatti wrote: > > BTW do we really need fast slot creation/destruction? > > At the moment yes. Boot a RHEL/Fedora installation disk (or any other > guest which uses SYSLINUX splash screen) and you will see. Another workload that suffers is Windows XP clearing the screen during boot. > That > particular case is a limitation of cirrus in QEMU, ideally it should be > optimized there. Why do you say that? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On 02/09/2012 04:23 PM, Avi Kivity wrote: > > BTW do we really need fast slot creation/destruction? > > Not really, but it's good to have infrastructure that copes with > different workloads. If the patches keep the code simple I think it's a > good thing to have. To qualify - taking several tens of milliseconds is out of the question as some workloads grind to a halt. But it doesn't need to be incredibly fast. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On 02/08/2012 05:43 PM, Takuya Yoshikawa wrote: > [Dropped non-kvm members from cc] > > Marcelo Tosatti wrote: > > > VGABIOS mode constantly destroys and creates 0xa slot, so > > performance is required for KVM_SET_MEM too (it can probably be fixed in > > qemu, but older qemu's must be supported). > > Apart from srcu, I see some problems concerning slot creation/destruction: > heavy shadow flush (and extra write protection). > > > Look at __kvm_set_memory_region(). > > 1. When we invalidate a memory slot, we call kvm_arch_flush_shadow() and > zap all shadow pages, not restricted to that slot. > > /* From this point no new shadow pages pointing to a deleted >* memslot will be created. >* >* validation of sp->gfn happens in: >* - gfn_to_hva (kvm_read_guest, gfn_to_pfn) >* - kvm_is_visible_gfn (mmu_check_roots) >*/ > kvm_arch_flush_shadow(kvm); The workloads that exercise slot removal heavily usually do so in a tight loop, so flushing all shadow is not too bad (not much to flush). > > 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow() > to clear all mmio sptes, again not restricted to that slot. > > /* >* If the new memory slot is created, we need to clear all >* mmio sptes. >*/ > if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > kvm_arch_flush_shadow(kvm); This is pretty rare outside the previous scenario (memory/pci hotplug). > > 3. In addition to these, we do write-protect all pages in that slot in > kvm_arch_commit_memory_region() every time. > > > For 1: I made a patch to restrict the flush to that slot by using > sp->slot_bitmap. > (seems working here) > > For 2: I think we can do the same: not 100% sure yet because I am still > struggling with the "mmio spte optimization" code. (really hacky ...) > > For 3: I think doing both "write protection" and "shadow flush" is > unnecessary. Maybe it only needs to be done if the only change is enabling dirty logging. > BTW do we really need fast slot creation/destruction? Not really, but it's good to have infrastructure that copes with different workloads. If the patches keep the code simple I think it's a good thing to have. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On Wed, 8 Feb 2012 16:45:31 -0200 Marcelo Tosatti wrote: > > For 3: I think doing both "write protection" and "shadow flush" is > > unnecessary. > > If you enable dirty logging on a slot, certainly you have to write > protect? When we enable dirty logging, yes. > > > BTW do we really need fast slot creation/destruction? > > At the moment yes. Boot a RHEL/Fedora installation disk (or any other > guest which uses SYSLINUX splash screen) and you will see. That > particular case is a limitation of cirrus in QEMU, ideally it should be > optimized there. > I've checked; limiting flush to that slot seems to be good. I will send a patch soon. Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
On Thu, Feb 09, 2012 at 12:43:20AM +0900, Takuya Yoshikawa wrote: > [Dropped non-kvm members from cc] > > Marcelo Tosatti wrote: > > > VGABIOS mode constantly destroys and creates 0xa slot, so > > performance is required for KVM_SET_MEM too (it can probably be fixed in > > qemu, but older qemu's must be supported). > > Apart from srcu, I see some problems concerning slot creation/destruction: > heavy shadow flush (and extra write protection). > > > Look at __kvm_set_memory_region(). > > 1. When we invalidate a memory slot, we call kvm_arch_flush_shadow() and > zap all shadow pages, not restricted to that slot. > > /* From this point no new shadow pages pointing to a deleted >* memslot will be created. >* >* validation of sp->gfn happens in: >* - gfn_to_hva (kvm_read_guest, gfn_to_pfn) >* - kvm_is_visible_gfn (mmu_check_roots) >*/ > kvm_arch_flush_shadow(kvm); > > 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow() > to clear all mmio sptes, again not restricted to that slot. > > /* >* If the new memory slot is created, we need to clear all >* mmio sptes. >*/ > if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > kvm_arch_flush_shadow(kvm); > > 3. In addition to these, we do write-protect all pages in that slot in > kvm_arch_commit_memory_region() every time. > > > For 1: I made a patch to restrict the flush to that slot by using > sp->slot_bitmap. > (seems working here) > > For 2: I think we can do the same: not 100% sure yet because I am still > struggling with the "mmio spte optimization" code. (really hacky ...) Yes, you can. The flush is there because a new slot invalidates any mmio sptes in that region, but in the region covered by the new slot only. > For 3: I think doing both "write protection" and "shadow flush" is > unnecessary. If you enable dirty logging on a slot, certainly you have to write protect? > BTW do we really need fast slot creation/destruction? At the moment yes. Boot a RHEL/Fedora installation disk (or any other guest which uses SYSLINUX splash screen) and you will see. That particular case is a limitation of cirrus in QEMU, ideally it should be optimized there. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()
[Dropped non-kvm members from cc] Marcelo Tosatti wrote: > VGABIOS mode constantly destroys and creates 0xa slot, so > performance is required for KVM_SET_MEM too (it can probably be fixed in > qemu, but older qemu's must be supported). Apart from srcu, I see some problems concerning slot creation/destruction: heavy shadow flush (and extra write protection). Look at __kvm_set_memory_region(). 1. When we invalidate a memory slot, we call kvm_arch_flush_shadow() and zap all shadow pages, not restricted to that slot. /* From this point no new shadow pages pointing to a deleted * memslot will be created. * * validation of sp->gfn happens in: * - gfn_to_hva (kvm_read_guest, gfn_to_pfn) * - kvm_is_visible_gfn (mmu_check_roots) */ kvm_arch_flush_shadow(kvm); 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow() to clear all mmio sptes, again not restricted to that slot. /* * If the new memory slot is created, we need to clear all * mmio sptes. */ if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) kvm_arch_flush_shadow(kvm); 3. In addition to these, we do write-protect all pages in that slot in kvm_arch_commit_memory_region() every time. For 1: I made a patch to restrict the flush to that slot by using sp->slot_bitmap. (seems working here) For 2: I think we can do the same: not 100% sure yet because I am still struggling with the "mmio spte optimization" code. (really hacky ...) For 3: I think doing both "write protection" and "shadow flush" is unnecessary. BTW do we really need fast slot creation/destruction? Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
