From mboxrd@z Thu Jan 1 00:00:00 1970 From: Carsten Otte Subject: Re: (no subject) Date: Tue, 14 Aug 2007 12:38:14 +0200 Message-ID: <46C18616.5000005@de.ibm.com> References: <64F9B87B6B770947A9F8391472E032160CBECF40@ehost011-8.exch011.intermedia.net> Reply-To: carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Izik Eidus Return-path: In-Reply-To: <64F9B87B6B770947A9F8391472E032160CBECF40-yEcIvxbTEBqsx+V+t5oei8rau4O3wl8o3fe8/T/H7NteoWH0uzbU5w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org 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(¤t->mm->mmap_sem); + pages_num = get_user_pages(current, current->mm, guest_host_addr, + npages, 1, 0, new.phys_mem, NULL); + up_read(¤t->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/