Re: [PATCH 1/3 V4] kvm tools: Add memory gap for larger RAM sizes

[Ingo Molnar <mingo@xxxxxxx>]

* levinsasha928@xxxxxxxxx <levinsasha928@xxxxxxxxx> wrote:

> @@ -225,7 +266,18 @@ struct kvm *kvm__init(const char *kvm_dev, unsigned long ram_size)
>  
>  	self->ram_size		= ram_size;
>  
> -	self->ram_start = mmap(NULL, ram_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
> +	if (self->ram_size < KVM_32BIT_GAP_START) {
> +		self->ram_start = mmap(NULL, ram_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
> +	} else {
> +		self->ram_start = mmap(NULL, ram_size + KVM_32BIT_GAP_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
> +		if (self->ram_start != MAP_FAILED) {
> +			/*
> +			 * We mprotect the gap (see kvm__init_ram() for details) PROT_NONE so that
> +			 * if we accidently write to it, we will know.
> +			 */
> +			mprotect(self->ram_start + KVM_32BIT_GAP_START, KVM_32BIT_GAP_SIZE, PROT_NONE);

Nit: the mmaps here wrap off the end of line. It would be a lot more easier to 
read if kvm.h defined two helpers, like:

	#define PROT_RW		(PROT_READ|PROT_WRITE)
	#define MAP_ANON	(MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE)

So the mmaps() would become a bit more readable:

> +	if (self->ram_size < KVM_32BIT_GAP_START) {
> +		self->ram_start = mmap(NULL, ram_size, PROT_RW, MAP_ANON, -1, 0);
> +	} else {
> +		self->ram_start = mmap(NULL, ram_size + KVM_32BIT_GAP_SIZE, PROT_RW, MAP_ANON, -1, 0);
> +		if (self->ram_start != MAP_FAILED) {
> +			/*
> +			 * We mprotect the gap (see kvm__init_ram() for details) PROT_NONE so that
> +			 * if we accidently write to it, we will know.
> +			 */
> +			mprotect(self->ram_start + KVM_32BIT_GAP_START, KVM_32BIT_GAP_SIZE, PROT_NONE);

I'll test your series.

Thanks,

	Ingo
--
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