Wei Liu
2013-May-30 14:06 UTC
[PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
Originally Xen PV drivers only use single-page ring to pass along information. This might limit the throughput between frontend and backend as they can easily run out of ring slots if hardware is fast. The patch extends Xenbus driver to support multi-page ring, which in general should improve throughput. Changes to various frontend / backend to adapt to the new interface are also included. Affected Xen drivers: * blkfront/back * netfront/back * pcifront/back The interface is documented, as before, in xenbus_client.c. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- drivers/block/xen-blkfront.c | 5 +- drivers/net/xen-netback/netback.c | 4 +- drivers/net/xen-netfront.c | 9 +- drivers/pci/xen-pcifront.c | 5 +- drivers/xen/xen-pciback/xenbus.c | 2 +- drivers/xen/xenbus/xenbus_client.c | 360 ++++++++++++++++++++++++++---------- include/xen/xenbus.h | 19 +- 7 files changed, 290 insertions(+), 114 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index d89ef86..cf20fd3 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1023,6 +1023,7 @@ static int setup_blkring(struct xenbus_device *dev, { struct blkif_sring *sring; int err; + grant_ref_t gref; info->ring_ref = GRANT_INVALID_REF; @@ -1042,13 +1043,13 @@ static int setup_blkring(struct xenbus_device *dev, if (err) goto fail; - err = xenbus_grant_ring(dev, virt_to_mfn(info->ring.sring)); + err = xenbus_grant_ring(dev, info->ring.sring, 1, &gref); if (err < 0) { free_page((unsigned long)sring); info->ring.sring = NULL; goto fail; } - info->ring_ref = err; + info->ring_ref = gref; err = xenbus_alloc_evtchn(dev, &info->evtchn); if (err) diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 09f32f2..7561ca4 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1572,7 +1572,7 @@ int xenvif_map_frontend_rings(struct xenvif *vif, int err = -ENOMEM; err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif), - tx_ring_ref, &addr); + &tx_ring_ref, 1, &addr); if (err) goto err; @@ -1580,7 +1580,7 @@ int xenvif_map_frontend_rings(struct xenvif *vif, BACK_RING_INIT(&vif->tx, txs, PAGE_SIZE); err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif), - rx_ring_ref, &addr); + &rx_ring_ref, 1, &addr); if (err) goto err; diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 62238a0..91a1a80 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1604,6 +1604,7 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) int err; struct net_device *netdev = info->netdev; unsigned int feature_split_evtchn; + grant_ref_t gref; info->tx_ring_ref = GRANT_INVALID_REF; info->rx_ring_ref = GRANT_INVALID_REF; @@ -1632,11 +1633,11 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) SHARED_RING_INIT(txs); FRONT_RING_INIT(&info->tx, txs, PAGE_SIZE); - err = xenbus_grant_ring(dev, virt_to_mfn(txs)); + err = xenbus_grant_ring(dev, txs, 1, &gref); if (err < 0) goto grant_tx_ring_fail; - info->tx_ring_ref = err; + info->tx_ring_ref = gref; rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH); if (!rxs) { err = -ENOMEM; @@ -1646,10 +1647,10 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) SHARED_RING_INIT(rxs); FRONT_RING_INIT(&info->rx, rxs, PAGE_SIZE); - err = xenbus_grant_ring(dev, virt_to_mfn(rxs)); + err = xenbus_grant_ring(dev, rxs, 1, &gref); if (err < 0) goto grant_rx_ring_fail; - info->rx_ring_ref = err; + info->rx_ring_ref = gref; if (feature_split_evtchn) err = setup_netfront_split(info); diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index 966abc6..a2c3427 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -772,12 +772,13 @@ static int pcifront_publish_info(struct pcifront_device *pdev) { int err = 0; struct xenbus_transaction trans; + grant_ref_t gref; - err = xenbus_grant_ring(pdev->xdev, virt_to_mfn(pdev->sh_info)); + err = xenbus_grant_ring(pdev->xdev, pdev->sh_info, 1, &gref); if (err < 0) goto out; - pdev->gnt_ref = err; + pdev->gnt_ref = gref; err = xenbus_alloc_evtchn(pdev->xdev, &pdev->evtchn); if (err) diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index 64b11f9..e0834cd 100644 --- a/drivers/xen/xen-pciback/xenbus.c +++ b/drivers/xen/xen-pciback/xenbus.c @@ -108,7 +108,7 @@ static int xen_pcibk_do_attach(struct xen_pcibk_device *pdev, int gnt_ref, "Attaching to frontend resources - gnt_ref=%d evtchn=%d\n", gnt_ref, remote_evtchn); - err = xenbus_map_ring_valloc(pdev->xdev, gnt_ref, &vaddr); + err = xenbus_map_ring_valloc(pdev->xdev, &gnt_ref, 1, &vaddr); if (err < 0) { xenbus_dev_fatal(pdev->xdev, err, "Error mapping other domain page in ours."); diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c index ec097d6..6930679 100644 --- a/drivers/xen/xenbus/xenbus_client.c +++ b/drivers/xen/xenbus/xenbus_client.c @@ -51,17 +51,25 @@ struct xenbus_map_node { struct list_head next; union { - struct vm_struct *area; /* PV */ - struct page *page; /* HVM */ + struct { + struct vm_struct *area; + } pv; + struct { + struct page *page[XENBUS_MAX_RING_PAGES]; + void *addr; + } hvm; }; - grant_handle_t handle; + grant_handle_t handles[XENBUS_MAX_RING_PAGES]; + unsigned int nr_handles; }; static DEFINE_SPINLOCK(xenbus_valloc_lock); static LIST_HEAD(xenbus_valloc_pages); struct xenbus_ring_ops { - int (*map)(struct xenbus_device *dev, int gnt, void **vaddr); + int (*map)(struct xenbus_device *dev, + grant_ref_t *gnt_refs, unsigned int nr_grefs, + void **vaddr); int (*unmap)(struct xenbus_device *dev, void *vaddr); }; @@ -357,17 +365,39 @@ static void xenbus_switch_fatal(struct xenbus_device *dev, int depth, int err, /** * xenbus_grant_ring * @dev: xenbus device - * @ring_mfn: mfn of ring to grant - - * Grant access to the given @ring_mfn to the peer of the given device. Return - * 0 on success, or -errno on error. On error, the device will switch to + * @vaddr: starting virtual address of the ring + * @nr_pages: number of pages to be granted + * @grefs: grant reference array to be filled in + * + * Grant access to the given @vaddr to the peer of the given device. + * Then fill in @grefs with grant references. Return 0 on success, or + * -errno on error. On error, the device will switch to * XenbusStateClosing, and the error will be saved in the store. */ -int xenbus_grant_ring(struct xenbus_device *dev, unsigned long ring_mfn) +int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr, + unsigned int nr_pages, grant_ref_t *grefs) { - int err = gnttab_grant_foreign_access(dev->otherend_id, ring_mfn, 0); - if (err < 0) - xenbus_dev_fatal(dev, err, "granting access to ring page"); + int err; + int i; + + for (i = 0; i < nr_pages; i++) { + unsigned long addr = (unsigned long)vaddr + + (PAGE_SIZE * i); + err = gnttab_grant_foreign_access(dev->otherend_id, + virt_to_mfn(addr), 0); + if (err < 0) { + xenbus_dev_fatal(dev, err, + "granting access to ring page"); + goto fail; + } + grefs[i] = err; + } + + return 0; + +fail: + for ( ; i >= 0; i--) + gnttab_end_foreign_access_ref(grefs[i], 0); return err; } EXPORT_SYMBOL_GPL(xenbus_grant_ring); @@ -448,62 +478,74 @@ EXPORT_SYMBOL_GPL(xenbus_free_evtchn); /** * xenbus_map_ring_valloc * @dev: xenbus device - * @gnt_ref: grant reference + * @gnt_refs: grant reference array + * @nr_grefs: number of grant references * @vaddr: pointer to address to be filled out by mapping * - * Based on Rusty Russell''s skeleton driver''s map_page. - * Map a page of memory into this domain from another domain''s grant table. - * xenbus_map_ring_valloc allocates a page of virtual address space, maps the - * page to that address, and sets *vaddr to that address. - * Returns 0 on success, and GNTST_* (see xen/include/interface/grant_table.h) - * or -ENOMEM on error. If an error is returned, device will switch to + * Map @nr_grefs pages of memory into this domain from another + * domain''s grant table. xenbus_map_ring_valloc allocates @nr_grefs + * pages of virtual address space, maps the pages to that address, and + * sets *vaddr to that address. Returns 0 on success, and GNTST_* + * (see xen/include/interface/grant_table.h) or -ENOMEM / -EINVAL on + * error. If an error is returned, device will switch to * XenbusStateClosing and the error message will be saved in XenStore. */ -int xenbus_map_ring_valloc(struct xenbus_device *dev, int gnt_ref, void **vaddr) +int xenbus_map_ring_valloc(struct xenbus_device *dev, grant_ref_t *gnt_refs, + unsigned int nr_grefs, void **vaddr) { - return ring_ops->map(dev, gnt_ref, vaddr); + return ring_ops->map(dev, gnt_refs, nr_grefs, vaddr); } EXPORT_SYMBOL_GPL(xenbus_map_ring_valloc); static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev, - int gnt_ref, void **vaddr) + grant_ref_t *gnt_refs, + unsigned int nr_grefs, + void **vaddr) { - struct gnttab_map_grant_ref op = { - .flags = GNTMAP_host_map | GNTMAP_contains_pte, - .ref = gnt_ref, - .dom = dev->otherend_id, - }; + struct gnttab_map_grant_ref map[XENBUS_MAX_RING_PAGES]; + struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_PAGES]; struct xenbus_map_node *node; struct vm_struct *area; - pte_t *pte; + pte_t *pte[XENBUS_MAX_RING_PAGES]; + int i, j; + int err = GNTST_okay; + bool vma_leaked; *vaddr = NULL; + if (nr_grefs > XENBUS_MAX_RING_PAGES) + return -EINVAL; + node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) return -ENOMEM; - area = alloc_vm_area(PAGE_SIZE, &pte); + area = alloc_vm_area(PAGE_SIZE * nr_grefs, pte); if (!area) { kfree(node); return -ENOMEM; } - op.host_addr = arbitrary_virt_to_machine(pte).maddr; + for (i = 0; i < nr_grefs; i++) { + map[i].flags = GNTMAP_host_map | GNTMAP_contains_pte; + map[i].ref = gnt_refs[i]; + map[i].dom = dev->otherend_id; + map[i].host_addr = arbitrary_virt_to_machine(pte[i]).maddr; + node->handles[i] = INVALID_GRANT_HANDLE; + } - gnttab_batch_map(&op, 1); + gnttab_batch_map(map, i); - if (op.status != GNTST_okay) { - free_vm_area(area); - kfree(node); - xenbus_dev_fatal(dev, op.status, - "mapping in shared page %d from domain %d", - gnt_ref, dev->otherend_id); - return op.status; + for (i = 0; i < nr_grefs; i++) { + if (map[i].status != GNTST_okay) { + err = map[i].status; + goto failed; + } else + node->handles[i] = map[i].handle; } - node->handle = op.handle; - node->area = area; + node->nr_handles = nr_grefs; + node->pv.area = area; spin_lock(&xenbus_valloc_lock); list_add(&node->next, &xenbus_valloc_pages); @@ -511,14 +553,53 @@ static int xenbus_map_ring_valloc_pv(struct xenbus_device *dev, *vaddr = area->addr; return 0; + +failed: + for (i = j = 0; i < nr_grefs; i++) { + if (node->handles[i] != INVALID_GRANT_HANDLE) { + unmap[j].dev_bus_addr = 0; + unmap[j].host_addr = arbitrary_virt_to_machine(pte[i]).maddr; + unmap[j].handle = node->handles[i]; + node->handles[i] = INVALID_GRANT_HANDLE; + j++; + } + } + + if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, j)) + BUG(); + + vma_leaked = false; + for (i = 0; i < j; i++) { + if (unmap[i].status != GNTST_okay) { + vma_leaked = true; + break; + } + } + + if (!vma_leaked) + free_vm_area(area); + else + pr_alert("leaking VM area %p size %d page(s)", area, nr_grefs); + + kfree(node); + xenbus_dev_fatal(dev, err, + "mapping in shared page %d from domain %d", + gnt_refs[i], dev->otherend_id); + return err; } static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev, - int gnt_ref, void **vaddr) + grant_ref_t *gnt_ref, + unsigned int nr_grefs, + void **vaddr) { struct xenbus_map_node *node; int err; void *addr; + bool leaked = false; + + if (nr_grefs > XENBUS_MAX_RING_PAGES) + return -EINVAL; *vaddr = NULL; @@ -526,15 +607,22 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev, if (!node) return -ENOMEM; - err = alloc_xenballooned_pages(1, &node->page, false /* lowmem */); + err = alloc_xenballooned_pages(nr_grefs, node->hvm.page, + false /* lowmem */); if (err) goto out_err; - addr = pfn_to_kaddr(page_to_pfn(node->page)); + addr = vmap(node->hvm.page, nr_grefs, VM_MAP, PAGE_KERNEL); + if (!addr) + goto out_err_free_ballooned_pages; + + node->hvm.addr = addr; - err = xenbus_map_ring(dev, gnt_ref, &node->handle, addr); + err = xenbus_map_ring(dev, gnt_ref, nr_grefs, node->handles, addr, &leaked); if (err) - goto out_err_free_ballooned_pages; + goto out_err_vunmap; + + node->nr_handles = nr_grefs; spin_lock(&xenbus_valloc_lock); list_add(&node->next, &xenbus_valloc_pages); @@ -543,8 +631,11 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev, *vaddr = addr; return 0; + out_err_vunmap: + if (!leaked) + vunmap(addr); out_err_free_ballooned_pages: - free_xenballooned_pages(1, &node->page); + free_xenballooned_pages(nr_grefs, node->hvm.page); out_err: kfree(node); return err; @@ -554,35 +645,80 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev, /** * xenbus_map_ring * @dev: xenbus device - * @gnt_ref: grant reference - * @handle: pointer to grant handle to be filled + * @gnt_refs: grant reference array + * @nr_grefs: number of grant reference + * @handles: pointer to grant handle to be filled * @vaddr: address to be mapped to + * @leaked: fail to clean up a failed map, caller should not free vaddr * - * Map a page of memory into this domain from another domain''s grant table. + * Map pages of memory into this domain from another domain''s grant table. * xenbus_map_ring does not allocate the virtual address space (you must do - * this yourself!). It only maps in the page to the specified address. + * this yourself!). It only maps in the pages to the specified address. * Returns 0 on success, and GNTST_* (see xen/include/interface/grant_table.h) - * or -ENOMEM on error. If an error is returned, device will switch to - * XenbusStateClosing and the error message will be saved in XenStore. + * or -ENOMEM / -EINVAL on error. If an error is returned, device will switch to + * XenbusStateClosing and the first error message will be saved in XenStore. + * Further more if we fail to map the ring, caller should check @leaked. + * If @leaked is not zero it means xenbus_map_ring fails to clean up, caller + * should not free the address space of @vaddr. */ -int xenbus_map_ring(struct xenbus_device *dev, int gnt_ref, - grant_handle_t *handle, void *vaddr) +int xenbus_map_ring(struct xenbus_device *dev, grant_ref_t *gnt_refs, + unsigned int nr_grefs, grant_handle_t *handles, void *vaddr, + bool *leaked) { - struct gnttab_map_grant_ref op; + struct gnttab_map_grant_ref map[XENBUS_MAX_RING_PAGES]; + struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_PAGES]; + int i, j; + int err = GNTST_okay; + + if (nr_grefs > XENBUS_MAX_RING_PAGES) + return -EINVAL; + + for (i = 0; i < nr_grefs; i++) { + unsigned long addr = (unsigned long)vaddr + (PAGE_SIZE * i); + gnttab_set_map_op(&map[i], addr, GNTMAP_host_map, gnt_refs[i], + dev->otherend_id); + handles[i] = INVALID_GRANT_HANDLE; + } - gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map, gnt_ref, - dev->otherend_id); + gnttab_batch_map(map, i); + + for (i = 0; i < nr_grefs; i++) { + if (map[i].status != GNTST_okay) { + err = map[i].status; + xenbus_dev_fatal(dev, map[i].status, + "mapping in shared page %d from domain %d", + gnt_refs[i], dev->otherend_id); + goto fail; + } else + handles[i] = map[i].handle; + } - gnttab_batch_map(&op, 1); + return GNTST_okay; - if (op.status != GNTST_okay) { - xenbus_dev_fatal(dev, op.status, - "mapping in shared page %d from domain %d", - gnt_ref, dev->otherend_id); - } else - *handle = op.handle; + fail: + for (i = j = 0; i < nr_grefs; i++) { + if (handles[i] != INVALID_GRANT_HANDLE) { + unsigned long addr = (unsigned long)vaddr + + (PAGE_SIZE * i); + gnttab_set_unmap_op(&unmap[j], (phys_addr_t)addr, + GNTMAP_host_map, handles[i]); + j++; + } + } + + if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, j)) + BUG(); + + *leaked = false; + for (i = 0; i < j; i++) { + if (unmap[i].status != GNTST_okay) { + *leaked = true; + break; + } + } + + return err; - return op.status; } EXPORT_SYMBOL_GPL(xenbus_map_ring); @@ -592,8 +728,7 @@ EXPORT_SYMBOL_GPL(xenbus_map_ring); * @dev: xenbus device * @vaddr: addr to unmap * - * Based on Rusty Russell''s skeleton driver''s unmap_page. - * Unmap a page of memory in this domain that was imported from another domain. + * Unmap memory in this domain that was imported from another domain. * Use xenbus_unmap_ring_vfree if you mapped in your memory with * xenbus_map_ring_valloc (it will free the virtual address space). * Returns 0 on success and returns GNTST_* on error @@ -608,14 +743,15 @@ EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree); static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr) { struct xenbus_map_node *node; - struct gnttab_unmap_grant_ref op = { - .host_addr = (unsigned long)vaddr, - }; + struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_PAGES]; unsigned int level; + int i; + bool vma_leaked = false; + int err; spin_lock(&xenbus_valloc_lock); list_for_each_entry(node, &xenbus_valloc_pages, next) { - if (node->area->addr == vaddr) { + if (node->pv.area->addr == vaddr) { list_del(&node->next); goto found; } @@ -630,22 +766,36 @@ static int xenbus_unmap_ring_vfree_pv(struct xenbus_device *dev, void *vaddr) return GNTST_bad_virt_addr; } - op.handle = node->handle; - op.host_addr = arbitrary_virt_to_machine( - lookup_address((unsigned long)vaddr, &level)).maddr; + for (i = 0; i < node->nr_handles; i++) { + unsigned long addr; + + addr = (unsigned long)vaddr + (PAGE_SIZE * i); + unmap[i].host_addr = arbitrary_virt_to_machine( + lookup_address(addr, &level)).maddr; + unmap[i].handle = node->handles[i]; + } - if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)) + if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, i)) BUG(); - if (op.status == GNTST_okay) - free_vm_area(node->area); - else - xenbus_dev_error(dev, op.status, - "unmapping page at handle %d error %d", - node->handle, op.status); + err = GNTST_okay; + vma_leaked = false; + for (i = 0; i < node->nr_handles; i++) { + if (unmap[i].status != GNTST_okay) { + vma_leaked = true; + xenbus_dev_error(dev, unmap[i].status, + "unmapping page at handle %d error %d", + node->handles[i], unmap[i].status); + err = unmap[i].status; + break; + } + } + + if (!vma_leaked) + free_vm_area(node->pv.area); kfree(node); - return op.status; + return err; } static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr) @@ -656,7 +806,7 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr) spin_lock(&xenbus_valloc_lock); list_for_each_entry(node, &xenbus_valloc_pages, next) { - addr = pfn_to_kaddr(page_to_pfn(node->page)); + addr = node->hvm.addr; if (addr == vaddr) { list_del(&node->next); goto found; @@ -672,12 +822,13 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr) return GNTST_bad_virt_addr; } - rv = xenbus_unmap_ring(dev, node->handle, addr); + rv = xenbus_unmap_ring(dev, node->handles, node->nr_handles, addr); if (!rv) - free_xenballooned_pages(1, &node->page); + free_xenballooned_pages(node->nr_handles, node->hvm.page); else - WARN(1, "Leaking %p\n", vaddr); + WARN(1, "Leaking %p, size %d page(s)\n", vaddr, + node->nr_handles); kfree(node); return rv; @@ -686,29 +837,44 @@ static int xenbus_unmap_ring_vfree_hvm(struct xenbus_device *dev, void *vaddr) /** * xenbus_unmap_ring * @dev: xenbus device - * @handle: grant handle + * @handles: grant handle array + * @nr_handles: number of handles in the array * @vaddr: addr to unmap * - * Unmap a page of memory in this domain that was imported from another domain. + * Unmap memory in this domain that was imported from another domain. * Returns 0 on success and returns GNTST_* on error * (see xen/include/interface/grant_table.h). */ int xenbus_unmap_ring(struct xenbus_device *dev, - grant_handle_t handle, void *vaddr) + grant_handle_t *handles, unsigned int nr_handles, + void *vaddr) { - struct gnttab_unmap_grant_ref op; + struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_PAGES]; + int i; + int err; - gnttab_set_unmap_op(&op, (unsigned long)vaddr, GNTMAP_host_map, handle); + for (i = 0; i < nr_handles; i++) { + unsigned long addr; + + addr = (unsigned long)vaddr + (PAGE_SIZE * i); + gnttab_set_unmap_op(&unmap[i], addr, GNTMAP_host_map, handles[i]); + } - if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)) + if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, i)) BUG(); - if (op.status != GNTST_okay) - xenbus_dev_error(dev, op.status, - "unmapping page at handle %d error %d", - handle, op.status); + err = GNTST_okay; + for (i = 0; i < nr_handles; i++) { + if (unmap[i].status != GNTST_okay) { + xenbus_dev_error(dev, unmap[i].status, + "unmapping page at handle %d error %d", + handles[i], unmap[i].status); + err = unmap[i].status; + break; + } + } - return op.status; + return err; } EXPORT_SYMBOL_GPL(xenbus_unmap_ring); diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h index 0a7515c..98a8048 100644 --- a/include/xen/xenbus.h +++ b/include/xen/xenbus.h @@ -46,6 +46,10 @@ #include <xen/interface/io/xenbus.h> #include <xen/interface/io/xs_wire.h> +#define XENBUS_MAX_RING_PAGE_ORDER 4 +#define XENBUS_MAX_RING_PAGES (1U << XENBUS_MAX_RING_PAGE_ORDER) +#define INVALID_GRANT_HANDLE (~0U) + /* Register callback to watch this node. */ struct xenbus_watch { @@ -195,15 +199,18 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch, const char *pathfmt, ...); int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state); -int xenbus_grant_ring(struct xenbus_device *dev, unsigned long ring_mfn); -int xenbus_map_ring_valloc(struct xenbus_device *dev, - int gnt_ref, void **vaddr); -int xenbus_map_ring(struct xenbus_device *dev, int gnt_ref, - grant_handle_t *handle, void *vaddr); +int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr, + unsigned int nr_pages, grant_ref_t *grefs); +int xenbus_map_ring_valloc(struct xenbus_device *dev, grant_ref_t *gnt_refs, + unsigned int nr_grefs, void **vaddr); +int xenbus_map_ring(struct xenbus_device *dev, + grant_ref_t *gnt_refs, unsigned int nr_grefs, + grant_handle_t *handles, void *vaddr, bool *leaked); int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr); int xenbus_unmap_ring(struct xenbus_device *dev, - grant_handle_t handle, void *vaddr); + grant_handle_t *handles, unsigned int nr_handles, + void *vaddr); int xenbus_alloc_evtchn(struct xenbus_device *dev, int *port); int xenbus_bind_evtchn(struct xenbus_device *dev, int remote_port, int *port); -- 1.7.10.4
Wei Liu
2013-May-30 14:09 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
On Thu, May 30, 2013 at 03:06:00PM +0100, Wei Liu wrote:> Originally Xen PV drivers only use single-page ring to pass along > information. This might limit the throughput between frontend and > backend as they can easily run out of ring slots if hardware is fast. > > The patch extends Xenbus driver to support multi-page ring, which in > general should improve throughput. Changes to various frontend / backend > to adapt to the new interface are also included. > > Affected Xen drivers: > * blkfront/back > * netfront/back > * pcifront/back > > The interface is documented, as before, in xenbus_client.c. >Please note that this patch won''t apply cleanly on upstream kernel as it relies on some patches that are not in tree. I mainly want to collect views on the interface. Also in the HVM case I use vmap, I would like to know if I get it right. Thanks Wei.
Konrad Rzeszutek Wilk
2013-Aug-20 17:11 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
On Thu, May 30, 2013 at 03:09:12PM +0100, Wei Liu wrote:> On Thu, May 30, 2013 at 03:06:00PM +0100, Wei Liu wrote: > > Originally Xen PV drivers only use single-page ring to pass along > > information. This might limit the throughput between frontend and > > backend as they can easily run out of ring slots if hardware is fast. > > > > The patch extends Xenbus driver to support multi-page ring, which in > > general should improve throughput. Changes to various frontend / backend > > to adapt to the new interface are also included. > > > > Affected Xen drivers: > > * blkfront/back > > * netfront/back > > * pcifront/back > > > > The interface is documented, as before, in xenbus_client.c. > > > > Please note that this patch won''t apply cleanly on upstream kernel as it > relies on some patches that are not in tree.OK.> > I mainly want to collect views on the interface. Also in the HVM case I > use vmap, I would like to know if I get it right.This patches looks familiar that I had seen some time ago (maybe I even backported it for PV?). Anyhow the PV part looks OK. The HVM - I don''t see why we cannot do vmap. The nature of the ring is that it stays mapped for a very long time - so using vmap is OK. That is, the issues with vmap is that it takes a bit to setup and teardown - and that is OK with us. If you would like to rebase it on top of v3.11-rc5 and repost it I don''t see why we can''t have it for v3.12. David, Boris?> > > Thanks > Wei. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Wei Liu
2013-Aug-20 17:41 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
On Tue, Aug 20, 2013 at 6:11 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:> On Thu, May 30, 2013 at 03:09:12PM +0100, Wei Liu wrote: >> On Thu, May 30, 2013 at 03:06:00PM +0100, Wei Liu wrote: >> > Originally Xen PV drivers only use single-page ring to pass along >> > information. This might limit the throughput between frontend and >> > backend as they can easily run out of ring slots if hardware is fast. >> > >> > The patch extends Xenbus driver to support multi-page ring, which in >> > general should improve throughput. Changes to various frontend / backend >> > to adapt to the new interface are also included. >> > >> > Affected Xen drivers: >> > * blkfront/back >> > * netfront/back >> > * pcifront/back >> > >> > The interface is documented, as before, in xenbus_client.c. >> > >> >> Please note that this patch won''t apply cleanly on upstream kernel as it >> relies on some patches that are not in tree. > > OK. >> >> I mainly want to collect views on the interface. Also in the HVM case I >> use vmap, I would like to know if I get it right. > > This patches looks familiar that I had seen some time ago (maybe I even > backported it for PV?). Anyhow the PV part looks OK. > > The HVM - I don''t see why we cannot do vmap. The nature of the ring is that > it stays mapped for a very long time - so using vmap is OK. That is, the > issues with vmap is that it takes a bit to setup and teardown - and that > is OK with us. > > If you would like to rebase it on top of v3.11-rc5 and repost it I don''t > see why we can''t have it for v3.12. David, Boris? >I think the above implementation has a problem in the HVM case, but I cannot recall what. :-( If you''re generally happy with the idea, I can polish this patch and target it for 3.12. Wei.>> >> >> Thanks >> Wei. >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2013-Aug-20 20:07 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
On Tue, Aug 20, 2013 at 06:41:29PM +0100, Wei Liu wrote:> On Tue, Aug 20, 2013 at 6:11 PM, Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com> wrote: > > On Thu, May 30, 2013 at 03:09:12PM +0100, Wei Liu wrote: > >> On Thu, May 30, 2013 at 03:06:00PM +0100, Wei Liu wrote: > >> > Originally Xen PV drivers only use single-page ring to pass along > >> > information. This might limit the throughput between frontend and > >> > backend as they can easily run out of ring slots if hardware is fast. > >> > > >> > The patch extends Xenbus driver to support multi-page ring, which in > >> > general should improve throughput. Changes to various frontend / backend > >> > to adapt to the new interface are also included. > >> > > >> > Affected Xen drivers: > >> > * blkfront/back > >> > * netfront/back > >> > * pcifront/back > >> > > >> > The interface is documented, as before, in xenbus_client.c. > >> > > >> > >> Please note that this patch won''t apply cleanly on upstream kernel as it > >> relies on some patches that are not in tree. > > > > OK. > >> > >> I mainly want to collect views on the interface. Also in the HVM case I > >> use vmap, I would like to know if I get it right. > > > > This patches looks familiar that I had seen some time ago (maybe I even > > backported it for PV?). Anyhow the PV part looks OK. > > > > The HVM - I don''t see why we cannot do vmap. The nature of the ring is that > > it stays mapped for a very long time - so using vmap is OK. That is, the > > issues with vmap is that it takes a bit to setup and teardown - and that > > is OK with us. > > > > If you would like to rebase it on top of v3.11-rc5 and repost it I don''t > > see why we can''t have it for v3.12. David, Boris? > > > > I think the above implementation has a problem in the HVM case, but I > cannot recall what. :-(Heh. Well does it work?> > If you''re generally happy with the idea, I can polish this patch and > target it for 3.12.Yes sure. We still have some time before I need to clamp down.> > Wei. > > >> > >> > >> Thanks > >> Wei. > >> > >> _______________________________________________ > >> Xen-devel mailing list > >> Xen-devel@lists.xen.org > >> http://lists.xen.org/xen-devel > >> > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel
Wei Liu
2013-Aug-21 03:37 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
On Tue, Aug 20, 2013 at 04:07:31PM -0400, Konrad Rzeszutek Wilk wrote: [...]> > > > > > > I think the above implementation has a problem in the HVM case, but I > > cannot recall what. :-( > > Heh. Well does it work?I don''t remember. :-)> > > > If you''re generally happy with the idea, I can polish this patch and > > target it for 3.12. > > Yes sure. We still have some time before I need to clamp down.How much time do we have? Will probably post patch later this week or early next week. Wei.> > > > Wei. > > > > >> > > >> > > >> Thanks > > >> Wei. > > >> > > >> _______________________________________________ > > >> Xen-devel mailing list > > >> Xen-devel@lists.xen.org > > >> http://lists.xen.org/xen-devel > > >> > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xen.org > > > http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2013-Aug-21 11:00 UTC
Re: [PATCH RFC] xenbus_client: extend interface to suppurt multi-page ring
Wei Liu <wei.liu2@citrix.com> wrote:>On Tue, Aug 20, 2013 at 04:07:31PM -0400, Konrad Rzeszutek Wilk wrote: >[...] >> > > >> > >> > I think the above implementation has a problem in the HVM case, but >I >> > cannot recall what. :-( >> >> Heh. Well does it work? > >I don''t remember. :-) > >> > >> > If you''re generally happy with the idea, I can polish this patch >and >> > target it for 3.12. >> >> Yes sure. We still have some time before I need to clamp down. > >How much time do we have? Will probably post patch later this week or >early next week.That sounds double. We have two to three weeks until the merge window opens. Next week is when we will be clamping down and asking only for bug fixes. Or wait till the next merge window.> >Wei. > >> > >> > Wei. >> > >> > >> >> > >> >> > >> Thanks >> > >> Wei. >> > >> >> > >> _______________________________________________ >> > >> Xen-devel mailing list >> > >> Xen-devel@lists.xen.org >> > >> http://lists.xen.org/xen-devel >> > >> >> > > >> > > _______________________________________________ >> > > Xen-devel mailing list >> > > Xen-devel@lists.xen.org >> > > http://lists.xen.org/xen-devel