From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH][RESEND] Allocate userspace memory for older userspace (v2) Date: Thu, 18 Oct 2007 10:02:16 -0500 Message-ID: <47177578.8010807@us.ibm.com> References: <1192716331182-git-send-email-aliguori@us.ibm.com> <47176F67.9000200@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Avi Kivity Return-path: In-Reply-To: <47176F67.9000200-atKUWr5tajBWk0Htik3J/w@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 Avi Kivity wrote: > dAnthony Liguori wrote: > >> Allocate a userspace buffer for older userspaces. Also eliminate phys_mem >> buffer. The memset() in kvmctl really kills initial memory usage but swapping >> does even with old userspaces. >> >> Since v1, fixed a bug in slot creation. >> >> I send the previous patch in response to a mail instead of top level so I'm >> resending properly. >> >> >> > > > This is really nice, especially the diffstat. > Time to update patchbomb script... >> Signed-off-by: Anthony Liguori >> @@ -745,29 +727,27 @@ static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm, >> r = -ENOMEM; >> >> /* Allocate if a slot is being created */ >> - if (npages && !new.phys_mem) { >> - new.phys_mem = vmalloc(npages * sizeof(struct page *)); >> - >> - if (!new.phys_mem) >> - goto out_unlock; >> - >> + if (npages && !new.rmap) { >> new.rmap = vmalloc(npages * sizeof(struct page *)); >> >> if (!new.rmap) >> goto out_unlock; >> >> - memset(new.phys_mem, 0, npages * sizeof(struct page *)); >> memset(new.rmap, 0, npages * sizeof(*new.rmap)); >> - if (user_alloc) { >> - new.user_alloc = 1; >> + >> + if (user_alloc) >> new.userspace_addr = mem->userspace_addr; >> - } else { >> - for (i = 0; i < npages; ++i) { >> - new.phys_mem[i] = alloc_page(GFP_HIGHUSER >> - | __GFP_ZERO); >> - if (!new.phys_mem[i]) >> - goto out_unlock; >> - } >> + else { >> + down_write(¤t->mm->mmap_sem); >> + new.userspace_addr = do_mmap(NULL, 0, >> + npages * PAGE_SIZE, >> + PROT_READ | PROT_WRITE, >> + MAP_SHARED | MAP_ANONYMOUS, >> + 0); >> + up_write(¤t->mm->mmap_sem); >> + >> + if (new.userspace_addr > -1024UL) >> >> > > Surely there's some macro to test for this type of error return? Also > we may want to return the do_mmap() error code here (not sure). > I guess I used the wrong code as an example, sent out a new version using IS_ERR() although its a little ugly since we have to cast to a pointer. I looked at the error returns by do_mmap(), I don't think we'll get anything more meaningful from do_mmap() than -ENOMEM so I don't think there's much value to returning it's error code. I don't feel strongly about it though. Regards, Anthony Liguori >> + goto out_unlock; >> } >> } >> >> > > > ------------------------------------------------------------------------- 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/