On 9/9/22 12:44, Emanuele Giuseppe Esposito wrote:
> And make kvm_set_memory_region static, since it is not used outside
> kvm_main.c
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> include/linux/kvm_host.h | 2 --
> virt/kvm/kvm_main.c | 11 +++++------
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 3b40f8d68fbb..1c5b7b2e35dd 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1108,8 +1108,6 @@ enum kvm_mr_change {
> KVM_MR_FLAGS_ONLY,
> };
>
> -int kvm_set_memory_region(struct kvm *kvm,
> - const struct kvm_userspace_memory_region *mem);
> int __kvm_set_memory_region(struct kvm *kvm,
> const struct kvm_userspace_memory_region *mem);
> void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *slot);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index da263c370d00..339de0ed4557 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2007,24 +2007,23 @@ int __kvm_set_memory_region(struct kvm *kvm,
> }
> EXPORT_SYMBOL_GPL(__kvm_set_memory_region);
>
> -int kvm_set_memory_region(struct kvm *kvm,
> - const struct kvm_userspace_memory_region *mem)
> +static int kvm_set_memory_region(struct kvm *kvm,
> + const struct kvm_userspace_memory_region *mem)
> {
> int r;
>
> + if ((u16)mem->slot >= KVM_USER_MEM_SLOTS)
> + return -EINVAL;
> +
> mutex_lock(&kvm->slots_lock);
> r = __kvm_set_memory_region(kvm, mem);
> mutex_unlock(&kvm->slots_lock);
> return r;
> }
> -EXPORT_SYMBOL_GPL(kvm_set_memory_region);
>
> static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
> struct kvm_userspace_memory_region *mem)
> {
> - if ((u16)mem->slot >= KVM_USER_MEM_SLOTS)
> - return -EINVAL;
> -
> return kvm_set_memory_region(kvm, mem);
> }
>
The idea here was that kvm_set_memory_region could be used to set
private memory slots while not taking kvm->slots_lock.
So, I would instead:
1) rename __kvm_set_memory_region to kvm_set_memory_region;
2) inline the old kvm_set_memory_region into kvm_vm_ioctl_set_memory_region.
3) replace the comment "Must be called holding kvm->slots_lock for
write." with a proper lockdep_assert_held() now that the function
doesn't have the __ warning sign in front of it.
Paolo