Re: [patch 06/11] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/23/2009 01:38 PM, Marcelo Tosatti wrote:
Use two steps for memslot deletion: mark the slot invalid (which stops
instantiation of new shadow pages for that slot, but allows destruction),
then instantiate the new empty slot.

Also simplifies kvm_handle_hva locking.


  	r = kvm_arch_prepare_memory_region(kvm,&new, old, user_alloc);
  	if (r)
  		goto out_free;

r == 0


-	spin_lock(&kvm->mmu_lock);
-	if (mem->slot>= kvm->memslots->nmemslots)
-		kvm->memslots->nmemslots = mem->slot + 1;
+#ifdef CONFIG_DMAR
+	/* map the pages in iommu page table */
+	if (npages)
+		r = kvm_iommu_map_pages(kvm,&new);
+		if (r)
+			goto out_free;

Bad indentation.

(still r == 0)

+#endif

-	*memslot = new;
-	spin_unlock(&kvm->mmu_lock);
+	slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
+	if (!slots)
+		goto out_free;

goto out_free with r == 0.

Best to assign r = -ENOMEM each time, to avoid these headaches.


+int memslot_id(struct kvm *kvm, gfn_t gfn)

Should be either static or kvm_memslot_id(). But the source is already like that, so leave it.

@@ -807,23 +808,18 @@ static int kvm_handle_hva(struct kvm *kv
  			  int (*handler)(struct kvm *kvm, unsigned long *rmapp,
  					 unsigned long data))
  {
-	int i, j;
+	int i, j, idx;
  	int retval = 0;
-	struct kvm_memslots *slots = kvm->memslots;
+	struct kvm_memslots *slots;
+
+	idx = srcu_read_lock(&kvm->srcu);

Maybe a better place is in the mmu notifiers, so the code is shared (once other archs start to use mmu notifiers).


@@ -3020,16 +3017,20 @@ nomem:
   */
  unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm)
  {
-	int i;
+	int i, idx;
  	unsigned int nr_mmu_pages;
  	unsigned int  nr_pages = 0;
+	struct kvm_memslots *slots;

-	for (i = 0; i<  kvm->memslots->nmemslots; i++)
-		nr_pages += kvm->memslots->memslots[i].npages;
+	idx = srcu_read_lock(&kvm->srcu);
+	slots = rcu_dereference(kvm->memslots);
+	for (i = 0; i<  slots->nmemslots; i++)
+		nr_pages += slots->memslots[i].npages;

  	nr_mmu_pages = nr_pages * KVM_PERMILLE_MMU_PAGES / 1000;
  	nr_mmu_pages = max(nr_mmu_pages,
  			(unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+	srcu_read_unlock(&kvm->srcu, idx);


Again, would like to move the srcu_locking to an outer scope.

@@ -1503,10 +1504,18 @@ static void enter_pmode(struct kvm_vcpu
  static gva_t rmode_tss_base(struct kvm *kvm)
  {
  	if (!kvm->arch.tss_addr) {
-		gfn_t base_gfn = kvm->memslots->memslots[0].base_gfn +
-				 kvm->memslots->memslots[0].npages - 3;
+		struct kvm_memslots *slots;
+		gfn_t base_gfn;
+		int idx;
+
+		idx = srcu_read_lock(&kvm->srcu);
+		slots = rcu_dereference(kvm->memslots);
+ 		base_gfn = slots->memslots[0].base_gfn +
+				 slots->memslots[0].npages - 3;
+		srcu_read_unlock(&kvm->srcu, idx);
  		return base_gfn<<  PAGE_SHIFT;
  	}
+
  	return kvm->arch.tss_addr;
  }

Shouldn't we already hold the srcu_lock as part of normal guest exit (similar to down_read() we do today)?

--
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux