Re: [PATCH 2/3] kvm: commonize allocation of the new memory slots

[Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>]

On 2014/11/14 20:12, Paolo Bonzini wrote:
> The two kmemdup invocations can be unified.  I find that the new
> placement of the comment makes it easier to see what happens.

A lot easier to follow the logic.

Reviewed-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   virt/kvm/kvm_main.c | 28 +++++++++++-----------------
>   1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index c8ff99cc0ccb..7bfc842b96d7 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -865,11 +865,12 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   			goto out_free;
>   	}
>   
> +	slots = kmemdup(kvm->memslots, sizeof(struct kvm_memslots),
> +			GFP_KERNEL);
> +	if (!slots)
> +		goto out_free;
> +
>   	if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {
> -		slots = kmemdup(kvm->memslots, sizeof(struct kvm_memslots),
> -				GFP_KERNEL);
> -		if (!slots)
> -			goto out_free;
>   		slot = id_to_memslot(slots, mem->slot);
>   		slot->flags |= KVM_MEMSLOT_INVALID;
>   
> @@ -885,6 +886,12 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   		 * 	- kvm_is_visible_gfn (mmu_check_roots)
>   		 */
>   		kvm_arch_flush_shadow_memslot(kvm, slot);
> +
> +		/*
> +		 * We can re-use the old_memslots from above, the only difference
> +		 * from the currently installed memslots is the invalid flag.  This
> +		 * will get overwritten by update_memslots anyway.
> +		 */
>   		slots = old_memslots;
>   	}
>   
> @@ -892,19 +899,6 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   	if (r)
>   		goto out_slots;
>   
> -	r = -ENOMEM;
> -	/*
> -	 * We can re-use the old_memslots from above, the only difference
> -	 * from the currently installed memslots is the invalid flag.  This
> -	 * will get overwritten by update_memslots anyway.
> -	 */
> -	if (!slots) {
> -		slots = kmemdup(kvm->memslots, sizeof(struct kvm_memslots),
> -				GFP_KERNEL);
> -		if (!slots)
> -			goto out_free;
> -	}
> -
>   	/* actual memory is freed via old in kvm_free_physmem_slot below */
>   	if (change == KVM_MR_DELETE) {
>   		new.dirty_bitmap = NULL;
>