From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adin Scannell Subject: [PATCH 2/3] Handle GNTST_eagain in kernel drivers Date: Fri, 16 Dec 2011 22:22:20 -0500 Message-ID: <1324092141-9730-3-git-send-email-adin@scannell.ca> References: <1324092141-9730-1-git-send-email-adin@scannell.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1324092141-9730-1-git-send-email-adin@scannell.ca> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: olaf@aepfle.de, Adin Scannell , andres@gridcentric.ca, JBeulich@suse.com, konrad@darnok.org, adin@gridcentric.com List-Id: xen-devel@lists.xenproject.org Handle GNTST_eagain status from GNTTABOP_map_grant_ref and GNTTABOP_copy operations properly to allow usage of xenpaging without causing crashes or data corruption. Replace all relevant HYPERVISOR_grant_table_op() calls with a retry loop. This loop is implemented as a macro to allow different GNTTABOP_* args. It will sleep up to 33 seconds and wait for the page to be paged in again. All ->status checks were updated to use the GNTST_* namespace. All return values are converted from GNTST_* namespace to 0/-EINVAL, since all callers did not use the actual return value. Signed-off-by: Olaf Hering Acked-by: Patrick Colp This is a port to the mainline Linux tree. This required dropping many backend drivers which have not yet made it in. Additionally, several of the referenced functions have moved and/or been refactored. I attempted to minimize changes while keeping the same semantics. Signed-off-by: Adin Scannell Index: linux/drivers/block/xen-blkback/blkback.c =================================================================== --- drivers/block/xen-blkback/blkback.c | 4 ++- drivers/xen/grant-table.c | 7 ++++- drivers/xen/xenbus/xenbus_client.c | 4 +++ include/xen/grant_table.h | 39 +++++++++++++++++++++++++++++++++++ include/xen/interface/grant_table.h | 6 ++++- 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index 15ec4db..d3fb290 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -390,7 +390,9 @@ static int xen_blkbk_map(struct blkif_request *req, * the page from the other domain. */ for (i = 0; i < nseg; i++) { - if (unlikely(map[i].status != 0)) { + if (unlikely(map[i].status == GNTST_eagain)) + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map[i]); + if (unlikely(map[i].status != GNTST_okay)) { pr_debug(DRV_PFX "invalid buffer -- could not remap it\n"); map[i].handle = BLKBACK_INVALID_HANDLE; ret |= 1; diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c index bf1c094..48826c3 100644 --- a/drivers/xen/grant-table.c +++ b/drivers/xen/grant-table.c @@ -464,9 +464,12 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops, for (i = 0; i < count; i++) { /* Do not add to override if the map failed. */ - if (map_ops[i].status) + if (map_ops[i].status != GNTST_okay && map_ops[i].status != GNTST_eagain) continue; + if (map_ops[i].status == GNTST_eagain) + return -EAGAIN; + if (map_ops[i].flags & GNTMAP_contains_pte) { pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) + (map_ops[i].host_addr & ~PAGE_MASK)); @@ -565,7 +568,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx) return -ENOSYS; } - BUG_ON(rc || setup.status); + BUG_ON(rc || (setup.status != GNTST_okay)); rc = arch_gnttab_map_shared(frames, nr_gframes, gnttab_max_grant_frames(), &shared); diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c index 1906125..d123c78 100644 --- a/drivers/xen/xenbus/xenbus_client.c +++ b/drivers/xen/xenbus/xenbus_client.c @@ -455,6 +455,8 @@ int xenbus_map_ring_valloc(struct xenbus_device *dev, int gnt_ref, void **vaddr) if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) BUG(); + if (op.status == GNTST_eagain) + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, (&op)); if (op.status != GNTST_okay) { free_vm_area(area); xenbus_dev_fatal(dev, op.status, @@ -499,6 +501,8 @@ int xenbus_map_ring(struct xenbus_device *dev, int gnt_ref, if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) BUG(); + if (op.status == GNTST_eagain) + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, (&op)); if (op.status != GNTST_okay) { xenbus_dev_fatal(dev, op.status, "mapping in shared page %d from domain %d", diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h index 11e2dfc..46fac85 100644 --- a/include/xen/grant_table.h +++ b/include/xen/grant_table.h @@ -37,6 +37,8 @@ #ifndef __ASM_GNTTAB_H__ #define __ASM_GNTTAB_H__ +#include + #include #include @@ -160,4 +162,41 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops, int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops, struct page **pages, unsigned int count); +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) \ +do { \ + u8 __hc_delay = 1; \ + int __ret; \ + while (unlikely((__HCarg_p)->status == GNTST_eagain && __hc_delay)) { \ + msleep(__hc_delay++); \ + __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1); \ + BUG_ON(__ret); \ + } \ + if (__hc_delay == 0) { \ + printk(KERN_ERR "%s: gnt busy\n", __func__,); \ + (__HCarg_p)->status = GNTST_bad_page; \ + } \ + if ((__HCarg_p)->status != GNTST_okay) \ + printk(KERN_ERR "%s: gnt status %x\n", \ + __func__, (__HCarg_p)->status); \ +} while(0) + +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p) \ +do { \ + u8 __hc_delay = 1; \ + int __ret; \ + do { \ + __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1); \ + BUG_ON(__ret); \ + if ((__HCarg_p)->status == GNTST_eagain) \ + msleep(__hc_delay++); \ + } while ((__HCarg_p)->status == GNTST_eagain && __hc_delay); \ + if (__hc_delay == 0) { \ + printk(KERN_ERR "%s: gnt busy\n", __func__); \ + (__HCarg_p)->status = GNTST_bad_page; \ + } \ + if ((__HCarg_p)->status != GNTST_okay) \ + printk(KERN_ERR "%s: gnt status %x\n", \ + __func__, (__HCarg_p)->status); \ +} while(0) + #endif /* __ASM_GNTTAB_H__ */ diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h index 39e5717..ba04080 100644 --- a/include/xen/interface/grant_table.h +++ b/include/xen/interface/grant_table.h @@ -363,6 +363,8 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_query_size); #define GNTST_permission_denied (-8) /* Not enough privilege for operation. */ #define GNTST_bad_page (-9) /* Specified page was invalid for op. */ #define GNTST_bad_copy_arg (-10) /* copy arguments cross page boundary */ +#define GNTST_address_too_big (-11) /* transfer page address too large. */ +#define GNTST_eagain (-12) /* Could not map at the moment. Retry. */ #define GNTTABOP_error_msgs { \ "okay", \ @@ -375,7 +377,9 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_query_size); "no spare translation slot in the I/O MMU", \ "permission denied", \ "bad page", \ - "copy arguments cross page boundary" \ + "copy arguments cross page boundary", \ + "page address size too large", \ + "could not map at the moment, retry" \ } #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */ -- 1.6.2.5