Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751652AbdINOBN (ORCPT ); Thu, 14 Sep 2017 10:01:13 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:19868 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751277AbdINOBM (ORCPT ); Thu, 14 Sep 2017 10:01:12 -0400 Subject: Re: [PATCH] xen: don't compile pv-specific parts if XEN_PV isn't configured To: Juergen Gross , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org References: <20170914123858.1167-1-jgross@suse.com> Cc: julien.grall@arm.com, tycho@docker.com From: Boris Ostrovsky Message-ID: Date: Thu, 14 Sep 2017 10:00:51 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170914123858.1167-1-jgross@suse.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3892 Lines: 149 On 09/14/2017 08:38 AM, Juergen Gross wrote: > xenbus_client.c contains some functions specific for pv guests. > Enclose them with #ifdef CONFIG_XEN_PV to avoid compiling them when > they are not needed (e.g. on ARM). > > Signed-off-by: Juergen Gross > --- > drivers/xen/xenbus/xenbus_client.c | 130 +++++++++++++++++++------------------ > 1 file changed, 67 insertions(+), 63 deletions(-) > > diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c > index 82a8866758ee..a1c17000129b 100644 > --- a/drivers/xen/xenbus/xenbus_client.c > +++ b/drivers/xen/xenbus/xenbus_client.c > @@ -519,64 +519,6 @@ static int __xenbus_map_ring(struct xenbus_device *dev, > return err; > } > > -static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev, > - grant_ref_t *gnt_refs, > - unsigned int nr_grefs, > - void **vaddr) > -{ > - struct xenbus_map_node *node; > - struct vm_struct *area; > - pte_t *ptes[XENBUS_MAX_RING_GRANTS]; > - phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS]; > - int err = GNTST_okay; > - int i; > - bool leaked; > - > - *vaddr = NULL; > - > - if (nr_grefs > XENBUS_MAX_RING_GRANTS) > - return -EINVAL; > - > - node = kzalloc(sizeof(*node), GFP_KERNEL); > - if (!node) > - return -ENOMEM; > - > - area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes); > - if (!area) { > - kfree(node); > - return -ENOMEM; > - } > - > - for (i = 0; i < nr_grefs; i++) > - phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr; > - > - err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles, > - phys_addrs, > - GNTMAP_host_map | GNTMAP_contains_pte, > - &leaked); > - if (err) > - goto failed; > - > - node->nr_handles = nr_grefs; > - node->pv.area = area; > - > - spin_lock(&xenbus_valloc_lock); > - list_add(&node->next, &xenbus_valloc_pages); > - spin_unlock(&xenbus_valloc_lock); > - > - *vaddr = area->addr; > - return 0; > - > -failed: > - if (!leaked) > - free_vm_area(area); > - else > - pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs); > - > - kfree(node); > - return err; > -} > - > struct map_ring_valloc_hvm > { > unsigned int idx; > @@ -725,6 +667,65 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr) > } > EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree); > > +#ifdef CONFIG_XEN_PV > +static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev, > + grant_ref_t *gnt_refs, > + unsigned int nr_grefs, > + void **vaddr) > +{ > + struct xenbus_map_node *node; > + struct vm_struct *area; > + pte_t *ptes[XENBUS_MAX_RING_GRANTS]; > + phys_addr_t phys_addrs[XENBUS_MAX_RING_GRANTS]; > + int err = GNTST_okay; > + int i; > + bool leaked; > + > + *vaddr = NULL; > + > + if (nr_grefs > XENBUS_MAX_RING_GRANTS) > + return -EINVAL; > + > + node = kzalloc(sizeof(*node), GFP_KERNEL); > + if (!node) > + return -ENOMEM; > + > + area = alloc_vm_area(XEN_PAGE_SIZE * nr_grefs, ptes); > + if (!area) { > + kfree(node); > + return -ENOMEM; > + } > + > + for (i = 0; i < nr_grefs; i++) > + phys_addrs[i] = arbitrary_virt_to_machine(ptes[i]).maddr; > + > + err = __xenbus_map_ring(dev, gnt_refs, nr_grefs, node->handles, > + phys_addrs, > + GNTMAP_host_map | GNTMAP_contains_pte, > + &leaked); > + if (err) > + goto failed; > + > + node->nr_handles = nr_grefs; > + node->pv.area = area; > + > + spin_lock(&xenbus_valloc_lock); > + list_add(&node->next, &xenbus_valloc_pages); > + spin_unlock(&xenbus_valloc_lock); > + > + *vaddr = area->addr; > + return 0; > + > +failed: > + if (!leaked) > + free_vm_area(area); > + else > + pr_alert("leaking VM area %p size %u page(s)", area, nr_grefs); > + > + kfree(node); > + return err; > +} > + Did you make any changes in xenbus_map_ring_valloc_pv()? I don't see any but the diff looks pretty big --- I'd expect only the preprocessor directives to show up. -boris