Re: [PATCH][RESEND] Allocate userspace memory for older userspace (v2)

[Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.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 <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> @@ -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(&current->mm->mmap_sem);
>> +			new.userspace_addr = do_mmap(NULL, 0,
>> +						     npages * PAGE_SIZE,
>> +						     PROT_READ | PROT_WRITE,
>> +						     MAP_SHARED | MAP_ANONYMOUS,
>> +						     0);
>> +			up_write(&current->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/