Re: [PATCH 2/3] kvm: commonize allocation of the new memory slots
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
>
> Signed-off-by: Paolo Bonzini
> ---
> 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;
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kvm: commonize allocation of the new memory slots
On Fri, 14 Nov 2014 12:12:01 +0100
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.
>
> Signed-off-by: Paolo Bonzini
> ---
> 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;
Reviewed-by: Igor Mammedov
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
