[kvm-devel] FW: (no subject)

[Izik Eidus]

hey Carsten,
about the dirty pages, i think you are right, i will look at this!.
about the api, as far as i can tell yes, we are going to keep the api to both 
options.
(when i wrote it, i wrote it with thought about how not breaking the old api.)

thanks for the comments! :)

-----Original Message-----
From: Carsten Otte [mailto:[EMAIL PROTECTED]
Sent: Tue 8/14/2007 3:38 AM
To: Izik Eidus
Cc: kvm-devel@lists.sourceforge.net
Subject: Re: [kvm-devel] (no subject)
 
Izik Eidus wrote:
> we are working on swapping support for the guests in kvm.
> we want to allow management of the memory swapping of the guests from kvm.
This is excellent, thank you!

> this is request for comment, so any idea you have please write to me.
I ran into a few while reading the code:

+static void kvm_free_userspace_physmem(struct kvm_memory_slot *free)
+{
+       int i;
+       
+       for (i = 0; i < free->npages; ++i) {
+               if (free->phys_mem[i]) {
+                       if (!PageReserved(free->phys_mem[i]))
+                               SetPageDirty(free->phys_mem[i]);
+                       page_cache_release(free->phys_mem[i]);
+               }
+       }
+}
I don't see why we would want to dirty a page we release in general. 
We do only need to dirty it, if the corresponding page table entry 
indicates so (dirty bit). Did I miss something?



@@ -670,7 +692,8 @@ EXPORT_SYMBOL_GPL(fx_init);
   * Discontiguous memory is allowed, mostly for framebuffers.
   */
  static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
-                                         struct kvm_memory_region *mem)
+                                         struct kvm_memory_region *mem,
+                                               unsigned long guest_host_addr)
  {
        int r;
        gfn_t base_gfn;
@@ -748,12 +771,26 @@ raced:
                        goto out_free;

                memset(new.phys_mem, 0, npages * sizeof(struct page *));
-               for (i = 0; i < npages; ++i) {
-                       new.phys_mem[i] = alloc_page(GFP_HIGHUSER
-                                                    | __GFP_ZERO);
-                       if (!new.phys_mem[i])
+               
+               if (guest_host_addr) {
+                       unsigned long pages_num;
+                       
+                       new.user_alloc = 1;
+                       down_read(&current->mm->mmap_sem);
+                       pages_num = get_user_pages(current, current->mm, 
guest_host_addr,
+                                                               npages, 1, 0, 
new.phys_mem, NULL);
+                       up_read(&current->mm->mmap_sem);
+                       if (pages_num != npages)
                                goto out_free;
-                       set_page_private(new.phys_mem[i],0);
+               } else {
+                       new.user_alloc = 0;
+                       for (i = 0; i < npages; ++i) {
+                               new.phys_mem[i] = alloc_page(GFP_HIGHUSER
+                                                            | __GFP_ZERO);
+                               if (!new.phys_mem[i])
+                                       goto out_free;
+                               set_page_private(new.phys_mem[i],0);
+                       }
                }
        }
Do we intend to maintain both pathes in the long run, or wait until we 
don't care about ancient userland anymore that does'nt do the 
allocation on its own?




-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel