Re: [PATCH 3/4] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region()

[Avi Kivity <avi@redhat.com>]

On 01/29/2012 02:30 PM, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
>
> Narrow down the controlled text inside the conditional so that it will
> include lpage_info and rmap stuff only.
>
> For this we change the way we check whether the slot is being created
> from "if (npages && !new.rmap)" to "if (npages && !old.npages)".
>  
>  static struct kvm_memory_slot *
>  search_memslots(struct kvm_memslots *slots, gfn_t gfn)
> @@ -828,23 +828,20 @@ int __kvm_set_memory_region(struct kvm *kvm,
>  	r = -ENOMEM;
>  
>  	/* Allocate if a slot is being created */
> +	if (npages && !old.npages) {
> +		new.user_alloc = user_alloc;
> +		if (user_alloc)
> +			new.userspace_addr = mem->userspace_addr;

This if () is new, is it not? ah, I see it was in the S390 path.  Better
to remove it.

>  #ifndef CONFIG_S390
> -	if (npages && !new.rmap) {
> -		new.rmap = vzalloc(npages * sizeof(*new.rmap));
> -
> -		if (!new.rmap)
> +		if (!new.rmap) {

Why this new test?  Can we have a new slot that does have an rmap?

> +			new.rmap = vzalloc(npages * sizeof(*new.rmap));
> +			if (!new.rmap)
> +				goto out_free;
> +		}
> +		if (create_lpage_info(&new, npages))
>  			goto out_free;
> -
> -		new.user_alloc = user_alloc;
> -		new.userspace_addr = mem->userspace_addr;
> +#endif /* not defined CONFIG_S390 */
>  	}
> -	if (!npages)
> -		goto skip_lpage;
> -
> -	if (create_lpage_info(&new, npages))
> -		goto out_free;
> -
> -skip_lpage:
>  
>  	/* Allocate page dirty bitmap if needed */
>  	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
> @@ -852,11 +849,6 @@ skip_lpage:
>  			goto out_free;
>  		/* destroy any largepage mappings for dirty tracking */
>  	}
> -#else  /* not defined CONFIG_S390 */
> -	new.user_alloc = user_alloc;
> -	if (user_alloc)
> -		new.userspace_addr = mem->userspace_addr;
> -#endif /* not defined CONFIG_S390 */
>  
>  	if (!npages) {
>  		struct kvm_memory_slot *slot;


-- 
error compiling committee.c: too many arguments to function