From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/4] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region() Date: Sun, 29 Jan 2012 18:09:50 +0200 Message-ID: <4F256F4E.10300@redhat.com> References: <20120129212630.3986589f898100bbbd312c63@gmail.com> <20120129213007.5ef934bf771bf73bb3355264@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60512 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752797Ab2A2QJx (ORCPT ); Sun, 29 Jan 2012 11:09:53 -0500 In-Reply-To: <20120129213007.5ef934bf771bf73bb3355264@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/29/2012 02:30 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > 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