Re: [PATCH] xen: don't compile pv-specific parts if XEN_PV isn't configured

From: Juergen Gross
Date: Thu Sep 14 2017 - 10:03:10 EST


On 14/09/17 16:00, Boris Ostrovsky wrote:
> 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 <jgross@xxxxxxxx>
>> ---
>> 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.

I moved the functions to require only one #ifdef (plus 1 for setting
the pv variants).


Juergen