* xenpaging fixes for kernel and hypervisor
@ 2010-09-15 16:05 Olaf Hering
2010-09-15 16:08 ` [PATCH] xenpaging: handle GNTST_eagain in kernel drivers Olaf Hering
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Olaf Hering @ 2010-09-15 16:05 UTC (permalink / raw)
To: Patrick Colp; +Cc: xen-devel
Patrick,
there following patches fix xenpaging for me.
Granttable handling is incomplete. If a page is gone, a GNTST_eagain
should be returned to the caller to inidcate the hypercall has to be
retried after a while, until the page is available again.
Please review.
Olaf
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH] xenpaging: handle GNTST_eagain in kernel drivers 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering @ 2010-09-15 16:08 ` Olaf Hering 2010-09-16 3:44 ` Patrick Colp 2010-09-15 16:10 ` [PATCH] xenpaging: page-in granttable entries Olaf Hering ` (6 subsequent siblings) 7 siblings, 1 reply; 16+ messages in thread From: Olaf Hering @ 2010-09-15 16:08 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel 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. Remove the existing retry code from net_rx_action(), dispatch_rw_block_io(), net_accel_map_grants_contig() and net_accel_map_iomem_page() and 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 <olaf@aepfle.de> --- Compiled with x86_64 and i386 configs drivers/xen/blkback/blkback.c | 57 ++++++----------------------- drivers/xen/blkback/common.h | 2 - drivers/xen/blkback/interface.c | 22 +++++------ drivers/xen/blktap/blktap.c | 29 ++++++-------- drivers/xen/blktap/interface.c | 22 +++++------ drivers/xen/blktap2/device.c | 34 ++++++++--------- drivers/xen/core/gnttab.c | 4 +- drivers/xen/gntdev/gntdev.c | 26 ++++++------- drivers/xen/netback/interface.c | 26 ++++--------- drivers/xen/netback/netback.c | 52 +++++++------------------- drivers/xen/scsiback/interface.c | 24 +++++------- drivers/xen/scsiback/scsiback.c | 16 ++------ drivers/xen/sfc_netutil/accel_util.c | 41 +++++--------------- drivers/xen/tpmback/interface.c | 22 +++++------ drivers/xen/tpmback/tpmback.c | 22 +++-------- drivers/xen/usbback/interface.c | 25 ++++-------- drivers/xen/usbback/usbback.c | 16 ++------ drivers/xen/xenbus/xenbus_backend_client.c | 27 ++++++------- include/xen/gnttab.h | 38 +++++++++++++++++++ 19 files changed, 204 insertions(+), 301 deletions(-) --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/blkback.c +++ linux-2.6.18-xen.hg/drivers/xen/blkback/blkback.c @@ -107,7 +107,7 @@ static inline unsigned long vaddr(pendin static int do_block_io_op(blkif_t *blkif); -static int dispatch_rw_block_io(blkif_t *blkif, +static void dispatch_rw_block_io(blkif_t *blkif, blkif_request_t *req, pending_req_t *pending_req); static void make_response(blkif_t *blkif, u64 id, @@ -312,13 +312,13 @@ static int do_block_io_op(blkif_t *blkif blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; - int more_to_do = 0, ret; + int more_to_do = 0; rc = blk_rings->common.req_cons; rp = blk_rings->common.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ - while ((rc != rp) || (blkif->is_suspended_req)) { + while ((rc != rp)) { if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) break; @@ -335,14 +335,6 @@ static int do_block_io_op(blkif_t *blkif break; } - /* Handle the suspended request first, if one exists */ - if(blkif->is_suspended_req) - { - memcpy(&req, &blkif->suspended_req, sizeof(req)); - blkif->is_suspended_req = 0; - goto handle_request; - } - switch (blkif->blk_protocol) { case BLKIF_PROTOCOL_NATIVE: memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req)); @@ -361,19 +353,17 @@ static int do_block_io_op(blkif_t *blkif /* Apply all sanity checks to /private copy/ of request. */ barrier(); -handle_request: - ret = 0; switch (req.operation) { case BLKIF_OP_READ: blkif->st_rd_req++; - ret = dispatch_rw_block_io(blkif, &req, pending_req); + dispatch_rw_block_io(blkif, &req, pending_req); break; case BLKIF_OP_WRITE_BARRIER: blkif->st_br_req++; /* fall through */ case BLKIF_OP_WRITE: blkif->st_wr_req++; - ret = dispatch_rw_block_io(blkif, &req, pending_req); + dispatch_rw_block_io(blkif, &req, pending_req); break; default: /* A good sign something is wrong: sleep for a while to @@ -386,17 +376,6 @@ handle_request: free_req(pending_req); break; } - BUG_ON(ret != 0 && ret != -EAGAIN); - /* If we can't handle the request at the moment, save it, and break the - * loop */ - if(ret == -EAGAIN) - { - memcpy(&blkif->suspended_req, &req, sizeof(req)); - blkif->is_suspended_req = 1; - /* Return "no more work pending", restart will be handled 'out of - * band' */ - return 0; - } /* Yield point for this unbounded loop. */ cond_resched(); @@ -405,7 +384,7 @@ handle_request: return more_to_do; } -static int dispatch_rw_block_io(blkif_t *blkif, +static void dispatch_rw_block_io(blkif_t *blkif, blkif_request_t *req, pending_req_t *pending_req) { @@ -474,15 +453,13 @@ static int dispatch_rw_block_io(blkif_t ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg); BUG_ON(ret); -#define GENERAL_ERR (1<<0) -#define EAGAIN_ERR (1<<1) 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)) { DPRINTK("invalid buffer -- could not remap it\n"); map[i].handle = BLKBACK_INVALID_HANDLE; - ret |= GENERAL_ERR; - if(map[i].status == GNTST_eagain) - ret |= EAGAIN_ERR; + ret = 1; } else { blkback_pagemap_set(vaddr_pagenr(pending_req, i), pending_page(pending_req, i), @@ -502,14 +479,6 @@ static int dispatch_rw_block_io(blkif_t (req->seg[i].first_sect << 9); } - /* If any of grant maps failed with GNTST_eagain, suspend and retry later */ - if(ret & EAGAIN_ERR) - { - fast_flush_area(pending_req); - free_req(pending_req); - return -EAGAIN; - } - if (ret) goto fail_flush; @@ -575,7 +544,7 @@ static int dispatch_rw_block_io(blkif_t else if (operation == WRITE || operation == WRITE_BARRIER) blkif->st_wr_sect += preq.nr_sects; - return 0; + return; fail_flush: fast_flush_area(pending_req); @@ -583,7 +552,7 @@ static int dispatch_rw_block_io(blkif_t make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR); free_req(pending_req); msleep(1); /* back off a bit */ - return 0; + return; fail_put_bio: __end_block_io_op(pending_req, -EINVAL); @@ -591,7 +560,7 @@ static int dispatch_rw_block_io(blkif_t bio_put(bio); unplug_queue(blkif); msleep(1); /* back off a bit */ - return 0; + return; } --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/common.h +++ linux-2.6.18-xen.hg/drivers/xen/blkback/common.h @@ -83,8 +83,6 @@ typedef struct blkif_st { struct task_struct *xenblkd; unsigned int waiting_reqs; request_queue_t *plug; - int is_suspended_req; - blkif_request_t suspended_req; /* statistics */ unsigned long st_print; --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/blkback/interface.c @@ -59,25 +59,23 @@ blkif_t *blkif_alloc(domid_t domid) static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) { struct gnttab_map_grant_ref op; + int ret; gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, GNTMAP_host_map, shared_page, blkif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(100); - } while(op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - if (op.status) { - DPRINTK(" Grant table operation failure !\n"); - return op.status; + if (op.status == GNTST_okay) { + blkif->shmem_ref = shared_page; + blkif->shmem_handle = op.handle; + ret = 0; + } else { + DPRINTK(" Grant table operation failure %d!\n", (int)op.status); + ret = -EINVAL; } - blkif->shmem_ref = shared_page; - blkif->shmem_handle = op.handle; - - return 0; + return ret; } static void unmap_frontend_page(blkif_t *blkif) --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/blktap.c +++ linux-2.6.18-xen.hg/drivers/xen/blktap/blktap.c @@ -1526,19 +1526,17 @@ static void dispatch_rw_block_io(blkif_t uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i/2); - if (unlikely(map[i].status != 0)) { - WPRINTK("invalid kernel buffer -- " - "could not remap it\n"); - if(map[i].status == GNTST_eagain) - WPRINTK("grant GNTST_eagain: please use blktap2\n"); - ret |= 1; + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); + + if (unlikely(map[i].status != GNTST_okay)) { + WPRINTK("invalid kernel buffer -- could not remap it\n"); + ret = 1; map[i].handle = INVALID_GRANT_HANDLE; } - if (unlikely(map[i+1].status != 0)) { - WPRINTK("invalid user buffer -- " - "could not remap it\n"); - ret |= 1; + if (unlikely(map[i+1].status != GNTST_okay)) { + WPRINTK("invalid kernel buffer -- could not remap it\n"); + ret = 1; map[i+1].handle = INVALID_GRANT_HANDLE; } @@ -1565,12 +1563,11 @@ static void dispatch_rw_block_io(blkif_t uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i); - if (unlikely(map[i].status != 0)) { - WPRINTK("invalid kernel buffer -- " - "could not remap it\n"); - if(map[i].status == GNTST_eagain) - WPRINTK("grant GNTST_eagain: please use blktap2\n"); - ret |= 1; + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); + + if (unlikely(map[i].status != GNTST_okay)) { + WPRINTK("invalid kernel buffer -- could not remap it\n"); + ret = 1; map[i].handle = INVALID_GRANT_HANDLE; } --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/blktap/interface.c @@ -59,25 +59,23 @@ blkif_t *tap_alloc_blkif(domid_t domid) static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) { struct gnttab_map_grant_ref op; + int ret; gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, GNTMAP_host_map, shared_page, blkif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - if (op.status) { - DPRINTK(" Grant table operation failure !\n"); - return op.status; + if (op.status == GNTST_okay) { + blkif->shmem_ref = shared_page; + blkif->shmem_handle = op.handle; + ret = 0; + } else { + DPRINTK("Grant table operation failure %d!\n", (int)op.status); + ret = -EINVAL; } - blkif->shmem_ref = shared_page; - blkif->shmem_handle = op.handle; - - return 0; + return ret; } static void unmap_frontend_page(blkif_t *blkif) --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap2/device.c +++ linux-2.6.18-xen.hg/drivers/xen/blktap2/device.c @@ -505,10 +505,10 @@ blktap_map_foreign(struct blktap *tap, uvaddr = MMAP_VADDR(ring->user_vstart, usr_idx, i); - if (unlikely(table->grants[grant].status)) { + if (unlikely(table->grants[grant].status != GNTST_okay)) { BTERR("invalid kernel buffer: could not remap it\n"); - /* This should never happen: blkback should handle eagain first */ - BUG_ON(table->grants[grant].status == GNTST_eagain); + /* This should never happen: blkback should handle eagain first */ + BUG_ON(table->grants[grant].status == GNTST_eagain); err |= 1; table->grants[grant].handle = INVALID_GRANT_HANDLE; } @@ -517,19 +517,18 @@ blktap_map_foreign(struct blktap *tap, foreign_mfn = table->grants[grant].dev_bus_addr >> PAGE_SHIFT; grant++; - if (xen_feature(XENFEAT_auto_translated_physmap)) - goto done; - - if (unlikely(table->grants[grant].status)) { - BTERR("invalid user buffer: could not remap it\n"); - err |= 1; + if (!xen_feature(XENFEAT_auto_translated_physmap)) { + if (unlikely(table->grants[grant].status != GNTST_okay)) { + /* This should never happen: blkback should handle eagain first */ + WARN_ON(table->grants[grant].status == GNTST_eagain); + BTERR("invalid user buffer: could not remap it\n"); + err |= 1; + } table->grants[grant].handle = INVALID_GRANT_HANDLE; + request->handles[i].user = table->grants[grant].handle; + grant++; } - request->handles[i].user = table->grants[grant].handle; - grant++; - - done: if (err) continue; @@ -602,11 +601,10 @@ blktap_map(struct blktap *tap, set_page_private(tap_page, page_private(page)); SetPageBlkback(tap_page); - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, - &map, 1); - BUG_ON(err); - /* We are not expecting the grant op to fail */ - BUG_ON(map.status != GNTST_okay); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map); + + /* We are not expecting the grant op to fail */ + BUG_ON(map.status != GNTST_okay); err = vm_insert_page(ring->vma, uvaddr, tap_page); if (err) { --- linux-2.6.18-xen.hg.orig/drivers/xen/core/gnttab.c +++ linux-2.6.18-xen.hg/drivers/xen/core/gnttab.c @@ -487,7 +487,7 @@ static int gnttab_map(unsigned int start return -ENOSYS; } - BUG_ON(rc || setup.status); + BUG_ON(rc || setup.status != GNTST_okay); if (shared == NULL) shared = arch_gnttab_alloc_shared(frames); @@ -571,7 +571,7 @@ int gnttab_copy_grant_page(grant_ref_t r err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace, &unmap, 1); BUG_ON(err); - BUG_ON(unmap.status); + BUG_ON(unmap.status != GNTST_okay); write_sequnlock(&gnttab_dma_lock); --- linux-2.6.18-xen.hg.orig/drivers/xen/gntdev/gntdev.c +++ linux-2.6.18-xen.hg/drivers/xen/gntdev/gntdev.c @@ -578,7 +578,7 @@ static int gntdev_mmap (struct file *fli vma->vm_mm->context.has_foreign_mappings = 1; #endif - exit_ret = -ENOMEM; + exit_ret = -ENOMEM; for (i = 0; i < size; ++i) { flags = GNTMAP_host_map; @@ -599,8 +599,8 @@ static int gntdev_mmap (struct file *fli ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); BUG_ON(ret); - if (op.status) { - if(op.status != GNTST_eagain) + if (op.status != GNTST_okay) { + if (op.status != GNTST_eagain) printk(KERN_ERR "Error mapping the grant reference " "into the kernel (%d). domid = %d; ref = %d\n", op.status, @@ -608,9 +608,9 @@ static int gntdev_mmap (struct file *fli .u.valid.domid, private_data->grants[slot_index+i] .u.valid.ref); - else - /* Propagate eagain instead of trying to fix it up */ - exit_ret = -EAGAIN; + else + /* Propagate eagain instead of trying to fix it up */ + exit_ret = -EAGAIN; goto undo_map_out; } @@ -679,7 +679,7 @@ static int gntdev_mmap (struct file *fli ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); BUG_ON(ret); - if (op.status) { + if (op.status != GNTST_okay) { printk(KERN_ERR "Error mapping the grant " "reference into user space (%d). domid " "= %d; ref = %d\n", op.status, @@ -687,9 +687,9 @@ static int gntdev_mmap (struct file *fli .valid.domid, private_data->grants[slot_index+i].u .valid.ref); - /* This should never happen after we've mapped into - * the kernel space. */ - BUG_ON(op.status == GNTST_eagain); + /* This should never happen after we've mapped into + * the kernel space. */ + BUG_ON(op.status == GNTST_eagain); goto undo_map_out; } @@ -713,7 +713,7 @@ static int gntdev_mmap (struct file *fli } } - exit_ret = 0; + exit_ret = 0; up_write(&private_data->grants_sem); return exit_ret; @@ -785,7 +785,7 @@ static pte_t gntdev_clear_pte(struct vm_ ret = HYPERVISOR_grant_table_op( GNTTABOP_unmap_grant_ref, &op, 1); BUG_ON(ret); - if (op.status) + if (op.status != GNTST_okay) printk("User unmap grant status = %d\n", op.status); } else { @@ -802,7 +802,7 @@ static pte_t gntdev_clear_pte(struct vm_ ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); BUG_ON(ret); - if (op.status) + if (op.status != GNTST_okay) printk("Kernel unmap grant status = %d\n", op.status); --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c @@ -251,15 +251,11 @@ static int map_frontend_pages( gnttab_set_map_op(&op, (unsigned long)netif->tx_comms_area->addr, GNTMAP_host_map, tx_ring_ref, netif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(op.status == GNTST_eagain); - - if (op.status) { - DPRINTK(" Gnttab failure mapping tx_ring_ref!\n"); - return op.status; + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); + + if (op.status != GNTST_okay) { + DPRINTK(" Gnttab failure mapping tx_ring_ref %d!\n", (int)op.status); + return -EINVAL; } netif->tx_shmem_ref = tx_ring_ref; @@ -267,13 +263,9 @@ static int map_frontend_pages( gnttab_set_map_op(&op, (unsigned long)netif->rx_comms_area->addr, GNTMAP_host_map, rx_ring_ref, netif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - if (op.status) { + if (op.status != GNTST_okay) { struct gnttab_unmap_grant_ref unop; gnttab_set_unmap_op(&unop, @@ -281,8 +273,8 @@ static int map_frontend_pages( GNTMAP_host_map, netif->tx_shmem_handle); VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop, 1)); - DPRINTK(" Gnttab failure mapping rx_ring_ref!\n"); - return op.status; + DPRINTK(" Gnttab failure mapping rx_ring_ref %d!\n", (int)op.status); + return -EINVAL; } netif->rx_shmem_ref = rx_ring_ref; --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/netback.c +++ linux-2.6.18-xen.hg/drivers/xen/netback/netback.c @@ -494,8 +494,7 @@ static inline void netbk_free_pages(int used to set up the operations on the top of netrx_pending_operations, which have since been done. Check that they didn't give any errors and advance over them. */ -static int netbk_check_gop(int nr_frags, domid_t domid, - struct netrx_pending_operations *npo, int *eagain) +static int netbk_check_gop(int nr_frags, domid_t domid, struct netrx_pending_operations *npo) { multicall_entry_t *mcl; gnttab_transfer_t *gop; @@ -503,17 +502,15 @@ static int netbk_check_gop(int nr_frags, int status = NETIF_RSP_OKAY; int i; - *eagain = 0; - for (i = 0; i <= nr_frags; i++) { if (npo->meta[npo->meta_cons + i].copy) { copy_op = npo->copy + npo->copy_cons++; - if (copy_op->status != GNTST_okay) { + if (unlikely(copy_op->status == GNTST_eagain)) + gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op); + if (unlikely(copy_op->status != GNTST_okay)) { DPRINTK("Bad status %d from copy to DOM%d.\n", copy_op->status, domid); status = NETIF_RSP_ERROR; - if(copy_op->status == GNTST_eagain) - *eagain = 1; } } else { if (!xen_feature(XENFEAT_auto_translated_physmap)) { @@ -524,7 +521,7 @@ static int netbk_check_gop(int nr_frags, gop = npo->trans + npo->trans_cons++; /* Check the reassignment error code. */ - if (gop->status != 0) { + if (unlikely(gop->status != GNTST_okay)) { DPRINTK("Bad status %d from grant transfer to DOM%u\n", gop->status, domid); /* @@ -533,8 +530,6 @@ static int netbk_check_gop(int nr_frags, * a fatal error anyway. */ BUG_ON(gop->status == GNTST_bad_page); - if(gop->status == GNTST_eagain) - *eagain = 1; status = NETIF_RSP_ERROR; } } @@ -576,7 +571,6 @@ static void net_rx_action(unsigned long int nr_frags; int count; unsigned long offset; - int eagain; /* * Putting hundreds of bytes on the stack is considered rude. @@ -680,7 +674,7 @@ static void net_rx_action(unsigned long netif = netdev_priv(skb->dev); - status = netbk_check_gop(nr_frags, netif->domid, &npo, &eagain); + status = netbk_check_gop(nr_frags, netif->domid, &npo); /* We can't rely on skb_release_data to release the pages used by fragments for us, since it tries to @@ -691,22 +685,14 @@ static void net_rx_action(unsigned long /* (Freeing the fragments is safe since we copy non-linear skbs destined for flipping interfaces) */ if (!netif->copying_receiver) { - /* - * Cannot handle failed grant transfers at the moment (because - * mmu_updates likely completed) - */ - BUG_ON(eagain); atomic_set(&(skb_shinfo(skb)->dataref), 1); skb_shinfo(skb)->frag_list = NULL; skb_shinfo(skb)->nr_frags = 0; netbk_free_pages(nr_frags, meta + npo.meta_cons + 1); } - if(!eagain) - { - netif->stats.tx_bytes += skb->len; - netif->stats.tx_packets++; - } + netif->stats.tx_bytes += skb->len; + netif->stats.tx_packets++; id = meta[npo.meta_cons].id; flags = nr_frags ? NETRXF_more_data : 0; @@ -756,18 +742,8 @@ static void net_rx_action(unsigned long !netbk_queue_full(netif)) netif_wake_queue(netif->dev); - if(!eagain || netbk_queue_full(netif)) - { - netif_put(netif); - dev_kfree_skb(skb); - netif->stats.tx_dropped += !!eagain; - } - else - { - netif->rx_req_cons_peek += skb_shinfo(skb)->nr_frags + 1 + - !!skb_shinfo(skb)->gso_size; - skb_queue_head(&rx_queue, skb); - } + netif_put(netif); + dev_kfree_skb(skb); npo.meta_cons += nr_frags + 1; } @@ -1107,7 +1083,7 @@ static int netbk_tx_check_mop(struct sk_ /* Check status of header. */ err = mop->status; - if (unlikely(err)) { + if (unlikely(err != GNTST_okay)) { txp = &pending_tx_info[pending_idx].req; make_tx_response(netif, txp, NETIF_RSP_ERROR); pending_ring[MASK_PEND_IDX(pending_prod++)] = pending_idx; @@ -1128,12 +1104,12 @@ static int netbk_tx_check_mop(struct sk_ /* Check error status: if okay then remember grant handle. */ newerr = (++mop)->status; - if (likely(!newerr)) { + if (likely(newerr == GNTST_okay)) { set_phys_to_machine(idx_to_pfn(pending_idx), FOREIGN_FRAME(mop->dev_bus_addr>>PAGE_SHIFT)); grant_tx_handle[pending_idx] = mop->handle; /* Had a previous error? Invalidate this fragment. */ - if (unlikely(err)) + if (unlikely(err != GNTST_okay)) netif_idx_release(pending_idx); continue; } @@ -1145,7 +1121,7 @@ static int netbk_tx_check_mop(struct sk_ netif_put(netif); /* Not the first error? Preceding frags already invalidated. */ - if (err) + if (err != GNTST_okay) continue; /* First error: invalidate header and preceding fragments. */ --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/interface.c @@ -64,27 +64,23 @@ static int map_frontend_page( struct vsc unsigned long ring_ref) { struct gnttab_map_grant_ref op; - int err; + int ret; gnttab_set_map_op(&op, (unsigned long)info->ring_area->addr, GNTMAP_host_map, ring_ref, info->domid); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - do { - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); - BUG_ON(err); - msleep(10); - } while(op.status == GNTST_eagain); - - if (op.status) { - printk(KERN_ERR "scsiback: Grant table operation failure !\n"); - return op.status; + if (op.status != GNTST_okay) { + printk(KERN_ERR "scsiback: Grant table operation failure %d!\n", (int)op.status); + ret = -EINVAL; + } else { + info->shmem_ref = ring_ref; + info->shmem_handle = op.handle; + ret = 0; } - info->shmem_ref = ring_ref; - info->shmem_handle = op.handle; - - return (GNTST_okay); + return ret; } static void unmap_frontend_page(struct vscsibk_info *info) --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/scsiback.c +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/scsiback.c @@ -279,22 +279,14 @@ static int scsiback_gnttab_data_map(vscs err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nr_segments); BUG_ON(err); - /* Retry maps with GNTST_eagain */ - for(i=0; i < nr_segments; i++) { - while(unlikely(map[i].status == GNTST_eagain)) - { - msleep(10); - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, - &map[i], - 1); - BUG_ON(err); - } - } for (i = 0; i < nr_segments; i++) { struct page *pg; - if (unlikely(map[i].status != 0)) { + /* Retry maps with GNTST_eagain */ + if (unlikely(map[i].status == GNTST_eagain)) + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); + if (unlikely(map[i].status != GNTST_okay)) { printk(KERN_ERR "scsiback: invalid buffer -- could not remap it\n"); map[i].handle = SCSIBACK_INVALID_HANDLE; err |= 1; --- linux-2.6.18-xen.hg.orig/drivers/xen/sfc_netutil/accel_util.c +++ linux-2.6.18-xen.hg/drivers/xen/sfc_netutil/accel_util.c @@ -71,24 +71,27 @@ static int net_accel_map_grant(struct xe u64 *dev_bus_addr, unsigned flags) { struct gnttab_map_grant_ref op; + int ret; gnttab_set_map_op(&op, (unsigned long)vaddr, flags, gnt_ref, dev->otherend_id); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); if (op.status != GNTST_okay) { xenbus_dev_error (dev, op.status, "failed mapping in shared page %d from domain %d\n", gnt_ref, dev->otherend_id); + ret = -EINVAL; } else { *handle = op.handle; if (dev_bus_addr) *dev_bus_addr = op.dev_bus_addr; + ret = 0; } - return op.status; + return ret; } @@ -112,7 +115,7 @@ static int net_accel_unmap_grant(struct "failed unmapping page at handle %d error %d\n", handle, op.status); - return op.status; + return op.status == GNTST_okay ? 0 : -EINVAL; } @@ -144,7 +147,7 @@ struct net_accel_valloc_grant_mapping { /* Map a series of grants into a contiguous virtual area */ static void *net_accel_map_grants_valloc(struct xenbus_device *dev, unsigned *grants, int npages, - unsigned flags, void **priv, int *errno) + unsigned flags, void **priv) { struct net_accel_valloc_grant_mapping *map; struct vm_struct *vm; @@ -172,16 +175,12 @@ static void *net_accel_map_grants_valloc /* Do the actual mapping */ addr = vm->addr; - if(errno != NULL) *errno = 0; + for (i = 0; i < npages; i++) { rc = net_accel_map_grant(dev, grants[i], map->grant_handles + i, addr, NULL, flags); - if (rc != 0) - { - if(errno != NULL) - *errno = (rc == GNTST_eagain ? -EAGAIN : -EINVAL); + if (rc < 0) goto undo; - } addr = (void*)((unsigned long)addr + PAGE_SIZE); } @@ -230,16 +229,7 @@ void *net_accel_map_grants_contig(struct unsigned *grants, int npages, void **priv) { - int errno; - void *ret; - - do { - ret = net_accel_map_grants_valloc(dev, grants, npages, - GNTMAP_host_map, priv, &errno); - if(errno) msleep(10); - } while(errno == -EAGAIN); - - return ret; + return net_accel_map_grants_valloc(dev, grants, npages, GNTMAP_host_map, priv); } EXPORT_SYMBOL(net_accel_map_grants_contig); @@ -255,16 +245,7 @@ EXPORT_SYMBOL(net_accel_unmap_grants_con void *net_accel_map_iomem_page(struct xenbus_device *dev, int gnt_ref, void **priv) { - int errno; - void *ret; - - do { - ret = net_accel_map_grants_valloc(dev, &gnt_ref, 1, - GNTMAP_host_map, priv, &errno); - if(errno) msleep(10); - } while(errno == -EAGAIN); - - return ret; + return net_accel_map_grants_valloc(dev, &gnt_ref, 1, GNTMAP_host_map, priv); } EXPORT_SYMBOL(net_accel_map_iomem_page); --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/interface.c @@ -81,25 +81,23 @@ tpmif_t *tpmif_find(domid_t domid, struc static int map_frontend_page(tpmif_t *tpmif, unsigned long shared_page) { struct gnttab_map_grant_ref op; + int ret; gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr, GNTMAP_host_map, shared_page, tpmif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - if (op.status) { - DPRINTK(" Grant table operation failure !\n"); - return op.status; + if (op.status != GNTST_okay) { + DPRINTK(" Grant table operation failure %d!\n", (int)op.status); + ret = -EINVAL; + } else { + tpmif->shmem_ref = shared_page; + tpmif->shmem_handle = op.handle; + ret = 0; } - tpmif->shmem_ref = shared_page; - tpmif->shmem_handle = op.handle; - - return 0; + return ret; } static void unmap_frontend_page(tpmif_t *tpmif) --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/tpmback.c +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/tpmback.c @@ -257,20 +257,15 @@ int _packet_write(struct packet *pak, gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), GNTMAP_host_map, tx->ref, tpmif->domid); - do { - if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, - &map_op, 1))) - BUG(); - if(map_op.status) msleep(10); - } while(map_op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op); - handle = map_op.handle; - - if (map_op.status) { + if (map_op.status != GNTST_okay) { DPRINTK(" Grant table operation failure !\n"); return 0; } + handle = map_op.handle; + tocopy = min_t(size_t, size - offset, PAGE_SIZE); if (copy_from_buffer((void *)(idx_to_kaddr(tpmif, i) | @@ -397,14 +392,9 @@ static int packet_read_shmem(struct pack gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), GNTMAP_host_map, tx->ref, tpmif->domid); - do { - if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, - &map_op, 1))) - BUG(); - if(map_op.status) msleep(10); - } while(map_op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op); - if (map_op.status) { + if (map_op.status != GNTST_okay) { DPRINTK(" Grant table operation failure !\n"); return -EFAULT; } --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/interface.c +++ linux-2.6.18-xen.hg/drivers/xen/usbback/interface.c @@ -110,16 +110,11 @@ static int map_frontend_pages(usbif_t *u gnttab_set_map_op(&op, (unsigned long)usbif->urb_ring_area->addr, GNTMAP_host_map, urb_ring_ref, usbif->domid); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while (op.status == GNTST_eagain); - - if (op.status) { - printk(KERN_ERR "grant table failure mapping urb_ring_ref\n"); - return op.status; + if (op.status != GNTST_okay) { + printk(KERN_ERR "grant table failure mapping urb_ring_ref %d\n", (int)op.status); + return -EINVAL; } usbif->urb_shmem_ref = urb_ring_ref; @@ -128,21 +123,17 @@ static int map_frontend_pages(usbif_t *u gnttab_set_map_op(&op, (unsigned long)usbif->conn_ring_area->addr, GNTMAP_host_map, conn_ring_ref, usbif->domid); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while (op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); - if (op.status) { + if (op.status != GNTST_okay) { struct gnttab_unmap_grant_ref unop; gnttab_set_unmap_op(&unop, (unsigned long) usbif->urb_ring_area->addr, GNTMAP_host_map, usbif->urb_shmem_handle); VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop, 1)); - printk(KERN_ERR "grant table failure mapping conn_ring_ref\n"); - return op.status; + printk(KERN_ERR "grant table failure mapping conn_ring_ref %d\n", (int)op.status); + return -EINVAL; } usbif->conn_shmem_ref = conn_ring_ref; --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/usbback.c +++ linux-2.6.18-xen.hg/drivers/xen/usbback/usbback.c @@ -392,19 +392,13 @@ static int usbbk_gnttab_map(usbif_t *usb ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nr_segs); BUG_ON(ret); - /* Make sure than none of the map ops failed with GNTST_eagain */ - for( i = 0; i < nr_segs; i++) { - while(map[i].status == GNTST_eagain) { - msleep(10); - ret = HYPERVISOR_grant_table_op( - GNTTABOP_map_grant_ref, - &map[i], 1); - BUG_ON(ret); - } - } for (i = 0; i < nr_segs; i++) { - if (unlikely(map[i].status != 0)) { + /* Make sure than none of the map ops failed with GNTST_eagain */ + if (unlikely(map[i].status == GNTST_eagain)) + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); + + if (unlikely(map[i].status != GNTST_okay)) { printk(KERN_ERR "usbback: invalid buffer -- could not remap it\n"); map[i].handle = USBBACK_INVALID_HANDLE; ret |= 1; --- linux-2.6.18-xen.hg.orig/drivers/xen/xenbus/xenbus_backend_client.c +++ linux-2.6.18-xen.hg/drivers/xen/xenbus/xenbus_backend_client.c @@ -49,11 +49,7 @@ struct vm_struct *xenbus_map_ring_valloc gnttab_set_map_op(&op, (unsigned long)area->addr, GNTMAP_host_map, gnt_ref, dev->otherend_id); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(op.status == GNTST_eagain); + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); if (op.status != GNTST_okay) { free_vm_area(area); @@ -61,7 +57,7 @@ struct vm_struct *xenbus_map_ring_valloc "mapping in shared page %d from domain %d", gnt_ref, dev->otherend_id); BUG_ON(!IS_ERR(ERR_PTR(op.status))); - return ERR_PTR(op.status); + return ERR_PTR(-EINVAL); } /* Stuff the handle in an unused field */ @@ -76,23 +72,24 @@ int xenbus_map_ring(struct xenbus_device grant_handle_t *handle, void *vaddr) { struct gnttab_map_grant_ref op; + int ret; gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map, gnt_ref, dev->otherend_id); - do { - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) - BUG(); - msleep(10); - } while(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", gnt_ref, dev->otherend_id); - } else + ret = -EINVAL; + } else { *handle = op.handle; + ret = 0; + } - return op.status; + return ret; } EXPORT_SYMBOL_GPL(xenbus_map_ring); @@ -115,7 +112,7 @@ int xenbus_unmap_ring_vfree(struct xenbu "unmapping page at handle %d error %d", (int16_t)area->phys_addr, op.status); - return op.status; + return op.status == GNTST_okay ? 0 : -EINVAL; } EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree); @@ -135,7 +132,7 @@ int xenbus_unmap_ring(struct xenbus_devi "unmapping page at handle %d error %d", handle, op.status); - return op.status; + return op.status == GNTST_okay ? 0 : -EINVAL; } EXPORT_SYMBOL_GPL(xenbus_unmap_ring); --- linux-2.6.18-xen.hg.orig/include/xen/gnttab.h +++ linux-2.6.18-xen.hg/include/xen/gnttab.h @@ -40,6 +40,7 @@ #include <asm/hypervisor.h> #include <asm/maddr.h> /* maddr_t */ #include <linux/mm.h> +#include <linux/delay.h> #include <xen/interface/grant_table.h> #include <xen/features.h> @@ -161,4 +162,41 @@ gnttab_set_replace_op(struct gnttab_unma unmap->handle = handle; } +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) \ +{ \ + 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: %s gnt busy\n", __func__, current->comm); \ + (__HCarg_p)->status = GNTST_bad_page; \ + } \ + if ((__HCarg_p)->status != GNTST_okay) \ + printk(KERN_ERR "%s: %s gnt status %x\n", \ + __func__, current->comm, (__HCarg_p)->status); \ +} + +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p) \ +{ \ + 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: %s gnt busy\n", __func__, current->comm); \ + (__HCarg_p)->status = GNTST_bad_page; \ + } \ + if ((__HCarg_p)->status != GNTST_okay) \ + printk(KERN_ERR "%s: %s gnt status %x\n", \ + __func__, current->comm, (__HCarg_p)->status); \ +} + #endif /* __ASM_GNTTAB_H__ */ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] xenpaging: handle GNTST_eagain in kernel drivers 2010-09-15 16:08 ` [PATCH] xenpaging: handle GNTST_eagain in kernel drivers Olaf Hering @ 2010-09-16 3:44 ` Patrick Colp 2010-09-16 9:05 ` Keir Fraser 2010-09-16 13:33 ` Olaf Hering 0 siblings, 2 replies; 16+ messages in thread From: Patrick Colp @ 2010-09-16 3:44 UTC (permalink / raw) To: Olaf Hering; +Cc: xen-devel [-- Attachment #1: Type: text/plain, Size: 57732 bytes --] Is there any particular reason for 33 seconds? This number seems a bit high to me. Did you run into situations where that amount of time was needed? Otherwise I'm happy with everything. Thanks for all the hard work! Acked-by: Patrick Colp <pjcolp@cs.ubc.ca> Patrick On 15 September 2010 09:08, Olaf Hering <olaf@aepfle.de> wrote: > > 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. > > Remove the existing retry code from net_rx_action(), dispatch_rw_block_io(), > net_accel_map_grants_contig() and net_accel_map_iomem_page() and 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 <olaf@aepfle.de> > > --- > Compiled with x86_64 and i386 configs > > drivers/xen/blkback/blkback.c | 57 ++++++----------------------- > drivers/xen/blkback/common.h | 2 - > drivers/xen/blkback/interface.c | 22 +++++------ > drivers/xen/blktap/blktap.c | 29 ++++++-------- > drivers/xen/blktap/interface.c | 22 +++++------ > drivers/xen/blktap2/device.c | 34 ++++++++--------- > drivers/xen/core/gnttab.c | 4 +- > drivers/xen/gntdev/gntdev.c | 26 ++++++------- > drivers/xen/netback/interface.c | 26 ++++--------- > drivers/xen/netback/netback.c | 52 +++++++------------------- > drivers/xen/scsiback/interface.c | 24 +++++------- > drivers/xen/scsiback/scsiback.c | 16 ++------ > drivers/xen/sfc_netutil/accel_util.c | 41 +++++--------------- > drivers/xen/tpmback/interface.c | 22 +++++------ > drivers/xen/tpmback/tpmback.c | 22 +++-------- > drivers/xen/usbback/interface.c | 25 ++++-------- > drivers/xen/usbback/usbback.c | 16 ++------ > drivers/xen/xenbus/xenbus_backend_client.c | 27 ++++++------- > include/xen/gnttab.h | 38 +++++++++++++++++++ > 19 files changed, 204 insertions(+), 301 deletions(-) > > --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/blkback.c > +++ linux-2.6.18-xen.hg/drivers/xen/blkback/blkback.c > @@ -107,7 +107,7 @@ static inline unsigned long vaddr(pendin > > > static int do_block_io_op(blkif_t *blkif); > -static int dispatch_rw_block_io(blkif_t *blkif, > +static void dispatch_rw_block_io(blkif_t *blkif, > blkif_request_t *req, > pending_req_t *pending_req); > static void make_response(blkif_t *blkif, u64 id, > @@ -312,13 +312,13 @@ static int do_block_io_op(blkif_t *blkif > blkif_request_t req; > pending_req_t *pending_req; > RING_IDX rc, rp; > - int more_to_do = 0, ret; > + int more_to_do = 0; > > rc = blk_rings->common.req_cons; > rp = blk_rings->common.sring->req_prod; > rmb(); /* Ensure we see queued requests up to 'rp'. */ > > - while ((rc != rp) || (blkif->is_suspended_req)) { > + while ((rc != rp)) { > > if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) > break; > @@ -335,14 +335,6 @@ static int do_block_io_op(blkif_t *blkif > break; > } > > - /* Handle the suspended request first, if one exists */ > - if(blkif->is_suspended_req) > - { > - memcpy(&req, &blkif->suspended_req, sizeof(req)); > - blkif->is_suspended_req = 0; > - goto handle_request; > - } > - > switch (blkif->blk_protocol) { > case BLKIF_PROTOCOL_NATIVE: > memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req)); > @@ -361,19 +353,17 @@ static int do_block_io_op(blkif_t *blkif > /* Apply all sanity checks to /private copy/ of request. */ > barrier(); > > -handle_request: > - ret = 0; > switch (req.operation) { > case BLKIF_OP_READ: > blkif->st_rd_req++; > - ret = dispatch_rw_block_io(blkif, &req, pending_req); > + dispatch_rw_block_io(blkif, &req, pending_req); > break; > case BLKIF_OP_WRITE_BARRIER: > blkif->st_br_req++; > /* fall through */ > case BLKIF_OP_WRITE: > blkif->st_wr_req++; > - ret = dispatch_rw_block_io(blkif, &req, pending_req); > + dispatch_rw_block_io(blkif, &req, pending_req); > break; > default: > /* A good sign something is wrong: sleep for a while to > @@ -386,17 +376,6 @@ handle_request: > free_req(pending_req); > break; > } > - BUG_ON(ret != 0 && ret != -EAGAIN); > - /* If we can't handle the request at the moment, save it, and break the > - * loop */ > - if(ret == -EAGAIN) > - { > - memcpy(&blkif->suspended_req, &req, sizeof(req)); > - blkif->is_suspended_req = 1; > - /* Return "no more work pending", restart will be handled 'out of > - * band' */ > - return 0; > - } > > /* Yield point for this unbounded loop. */ > cond_resched(); > @@ -405,7 +384,7 @@ handle_request: > return more_to_do; > } > > -static int dispatch_rw_block_io(blkif_t *blkif, > +static void dispatch_rw_block_io(blkif_t *blkif, > blkif_request_t *req, > pending_req_t *pending_req) > { > @@ -474,15 +453,13 @@ static int dispatch_rw_block_io(blkif_t > ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg); > BUG_ON(ret); > > -#define GENERAL_ERR (1<<0) > -#define EAGAIN_ERR (1<<1) > 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)) { > DPRINTK("invalid buffer -- could not remap it\n"); > map[i].handle = BLKBACK_INVALID_HANDLE; > - ret |= GENERAL_ERR; > - if(map[i].status == GNTST_eagain) > - ret |= EAGAIN_ERR; > + ret = 1; > } else { > blkback_pagemap_set(vaddr_pagenr(pending_req, i), > pending_page(pending_req, i), > @@ -502,14 +479,6 @@ static int dispatch_rw_block_io(blkif_t > (req->seg[i].first_sect << 9); > } > > - /* If any of grant maps failed with GNTST_eagain, suspend and retry later */ > - if(ret & EAGAIN_ERR) > - { > - fast_flush_area(pending_req); > - free_req(pending_req); > - return -EAGAIN; > - } > - > if (ret) > goto fail_flush; > > @@ -575,7 +544,7 @@ static int dispatch_rw_block_io(blkif_t > else if (operation == WRITE || operation == WRITE_BARRIER) > blkif->st_wr_sect += preq.nr_sects; > > - return 0; > + return; > > fail_flush: > fast_flush_area(pending_req); > @@ -583,7 +552,7 @@ static int dispatch_rw_block_io(blkif_t > make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR); > free_req(pending_req); > msleep(1); /* back off a bit */ > - return 0; > + return; > > fail_put_bio: > __end_block_io_op(pending_req, -EINVAL); > @@ -591,7 +560,7 @@ static int dispatch_rw_block_io(blkif_t > bio_put(bio); > unplug_queue(blkif); > msleep(1); /* back off a bit */ > - return 0; > + return; > } > > > --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/common.h > +++ linux-2.6.18-xen.hg/drivers/xen/blkback/common.h > @@ -83,8 +83,6 @@ typedef struct blkif_st { > struct task_struct *xenblkd; > unsigned int waiting_reqs; > request_queue_t *plug; > - int is_suspended_req; > - blkif_request_t suspended_req; > > /* statistics */ > unsigned long st_print; > --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/blkback/interface.c > @@ -59,25 +59,23 @@ blkif_t *blkif_alloc(domid_t domid) > static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) > { > struct gnttab_map_grant_ref op; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, > GNTMAP_host_map, shared_page, blkif->domid); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(100); > - } while(op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - if (op.status) { > - DPRINTK(" Grant table operation failure !\n"); > - return op.status; > + if (op.status == GNTST_okay) { > + blkif->shmem_ref = shared_page; > + blkif->shmem_handle = op.handle; > + ret = 0; > + } else { > + DPRINTK(" Grant table operation failure %d!\n", (int)op.status); > + ret = -EINVAL; > } > > - blkif->shmem_ref = shared_page; > - blkif->shmem_handle = op.handle; > - > - return 0; > + return ret; > } > > static void unmap_frontend_page(blkif_t *blkif) > --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/blktap.c > +++ linux-2.6.18-xen.hg/drivers/xen/blktap/blktap.c > @@ -1526,19 +1526,17 @@ static void dispatch_rw_block_io(blkif_t > > uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i/2); > > - if (unlikely(map[i].status != 0)) { > - WPRINTK("invalid kernel buffer -- " > - "could not remap it\n"); > - if(map[i].status == GNTST_eagain) > - WPRINTK("grant GNTST_eagain: please use blktap2\n"); > - ret |= 1; > + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); > + > + if (unlikely(map[i].status != GNTST_okay)) { > + WPRINTK("invalid kernel buffer -- could not remap it\n"); > + ret = 1; > map[i].handle = INVALID_GRANT_HANDLE; > } > > - if (unlikely(map[i+1].status != 0)) { > - WPRINTK("invalid user buffer -- " > - "could not remap it\n"); > - ret |= 1; > + if (unlikely(map[i+1].status != GNTST_okay)) { > + WPRINTK("invalid kernel buffer -- could not remap it\n"); > + ret = 1; > map[i+1].handle = INVALID_GRANT_HANDLE; > } > > @@ -1565,12 +1563,11 @@ static void dispatch_rw_block_io(blkif_t > > uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i); > > - if (unlikely(map[i].status != 0)) { > - WPRINTK("invalid kernel buffer -- " > - "could not remap it\n"); > - if(map[i].status == GNTST_eagain) > - WPRINTK("grant GNTST_eagain: please use blktap2\n"); > - ret |= 1; > + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); > + > + if (unlikely(map[i].status != GNTST_okay)) { > + WPRINTK("invalid kernel buffer -- could not remap it\n"); > + ret = 1; > map[i].handle = INVALID_GRANT_HANDLE; > } > > --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/blktap/interface.c > @@ -59,25 +59,23 @@ blkif_t *tap_alloc_blkif(domid_t domid) > static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) > { > struct gnttab_map_grant_ref op; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, > GNTMAP_host_map, shared_page, blkif->domid); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - if (op.status) { > - DPRINTK(" Grant table operation failure !\n"); > - return op.status; > + if (op.status == GNTST_okay) { > + blkif->shmem_ref = shared_page; > + blkif->shmem_handle = op.handle; > + ret = 0; > + } else { > + DPRINTK("Grant table operation failure %d!\n", (int)op.status); > + ret = -EINVAL; > } > > - blkif->shmem_ref = shared_page; > - blkif->shmem_handle = op.handle; > - > - return 0; > + return ret; > } > > static void unmap_frontend_page(blkif_t *blkif) > --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap2/device.c > +++ linux-2.6.18-xen.hg/drivers/xen/blktap2/device.c > @@ -505,10 +505,10 @@ blktap_map_foreign(struct blktap *tap, > > uvaddr = MMAP_VADDR(ring->user_vstart, usr_idx, i); > > - if (unlikely(table->grants[grant].status)) { > + if (unlikely(table->grants[grant].status != GNTST_okay)) { > BTERR("invalid kernel buffer: could not remap it\n"); > - /* This should never happen: blkback should handle eagain first */ > - BUG_ON(table->grants[grant].status == GNTST_eagain); > + /* This should never happen: blkback should handle eagain first */ > + BUG_ON(table->grants[grant].status == GNTST_eagain); > err |= 1; > table->grants[grant].handle = INVALID_GRANT_HANDLE; > } > @@ -517,19 +517,18 @@ blktap_map_foreign(struct blktap *tap, > foreign_mfn = table->grants[grant].dev_bus_addr >> PAGE_SHIFT; > grant++; > > - if (xen_feature(XENFEAT_auto_translated_physmap)) > - goto done; > - > - if (unlikely(table->grants[grant].status)) { > - BTERR("invalid user buffer: could not remap it\n"); > - err |= 1; > + if (!xen_feature(XENFEAT_auto_translated_physmap)) { > + if (unlikely(table->grants[grant].status != GNTST_okay)) { > + /* This should never happen: blkback should handle eagain first */ > + WARN_ON(table->grants[grant].status == GNTST_eagain); > + BTERR("invalid user buffer: could not remap it\n"); > + err |= 1; > + } > table->grants[grant].handle = INVALID_GRANT_HANDLE; > + request->handles[i].user = table->grants[grant].handle; > + grant++; > } > > - request->handles[i].user = table->grants[grant].handle; > - grant++; > - > - done: > if (err) > continue; > > @@ -602,11 +601,10 @@ blktap_map(struct blktap *tap, > set_page_private(tap_page, page_private(page)); > SetPageBlkback(tap_page); > > - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > - &map, 1); > - BUG_ON(err); > - /* We are not expecting the grant op to fail */ > - BUG_ON(map.status != GNTST_okay); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map); > + > + /* We are not expecting the grant op to fail */ > + BUG_ON(map.status != GNTST_okay); > > err = vm_insert_page(ring->vma, uvaddr, tap_page); > if (err) { > --- linux-2.6.18-xen.hg.orig/drivers/xen/core/gnttab.c > +++ linux-2.6.18-xen.hg/drivers/xen/core/gnttab.c > @@ -487,7 +487,7 @@ static int gnttab_map(unsigned int start > return -ENOSYS; > } > > - BUG_ON(rc || setup.status); > + BUG_ON(rc || setup.status != GNTST_okay); > > if (shared == NULL) > shared = arch_gnttab_alloc_shared(frames); > @@ -571,7 +571,7 @@ int gnttab_copy_grant_page(grant_ref_t r > err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace, > &unmap, 1); > BUG_ON(err); > - BUG_ON(unmap.status); > + BUG_ON(unmap.status != GNTST_okay); > > write_sequnlock(&gnttab_dma_lock); > > --- linux-2.6.18-xen.hg.orig/drivers/xen/gntdev/gntdev.c > +++ linux-2.6.18-xen.hg/drivers/xen/gntdev/gntdev.c > @@ -578,7 +578,7 @@ static int gntdev_mmap (struct file *fli > vma->vm_mm->context.has_foreign_mappings = 1; > #endif > > - exit_ret = -ENOMEM; > + exit_ret = -ENOMEM; > for (i = 0; i < size; ++i) { > > flags = GNTMAP_host_map; > @@ -599,8 +599,8 @@ static int gntdev_mmap (struct file *fli > ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > &op, 1); > BUG_ON(ret); > - if (op.status) { > - if(op.status != GNTST_eagain) > + if (op.status != GNTST_okay) { > + if (op.status != GNTST_eagain) > printk(KERN_ERR "Error mapping the grant reference " > "into the kernel (%d). domid = %d; ref = %d\n", > op.status, > @@ -608,9 +608,9 @@ static int gntdev_mmap (struct file *fli > .u.valid.domid, > private_data->grants[slot_index+i] > .u.valid.ref); > - else > - /* Propagate eagain instead of trying to fix it up */ > - exit_ret = -EAGAIN; > + else > + /* Propagate eagain instead of trying to fix it up */ > + exit_ret = -EAGAIN; > goto undo_map_out; > } > > @@ -679,7 +679,7 @@ static int gntdev_mmap (struct file *fli > ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > &op, 1); > BUG_ON(ret); > - if (op.status) { > + if (op.status != GNTST_okay) { > printk(KERN_ERR "Error mapping the grant " > "reference into user space (%d). domid " > "= %d; ref = %d\n", op.status, > @@ -687,9 +687,9 @@ static int gntdev_mmap (struct file *fli > .valid.domid, > private_data->grants[slot_index+i].u > .valid.ref); > - /* This should never happen after we've mapped into > - * the kernel space. */ > - BUG_ON(op.status == GNTST_eagain); > + /* This should never happen after we've mapped into > + * the kernel space. */ > + BUG_ON(op.status == GNTST_eagain); > goto undo_map_out; > } > > @@ -713,7 +713,7 @@ static int gntdev_mmap (struct file *fli > } > > } > - exit_ret = 0; > + exit_ret = 0; > > up_write(&private_data->grants_sem); > return exit_ret; > @@ -785,7 +785,7 @@ static pte_t gntdev_clear_pte(struct vm_ > ret = HYPERVISOR_grant_table_op( > GNTTABOP_unmap_grant_ref, &op, 1); > BUG_ON(ret); > - if (op.status) > + if (op.status != GNTST_okay) > printk("User unmap grant status = %d\n", > op.status); > } else { > @@ -802,7 +802,7 @@ static pte_t gntdev_clear_pte(struct vm_ > ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, > &op, 1); > BUG_ON(ret); > - if (op.status) > + if (op.status != GNTST_okay) > printk("Kernel unmap grant status = %d\n", op.status); > > > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c > @@ -251,15 +251,11 @@ static int map_frontend_pages( > > gnttab_set_map_op(&op, (unsigned long)netif->tx_comms_area->addr, > GNTMAP_host_map, tx_ring_ref, netif->domid); > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(op.status == GNTST_eagain); > - > - if (op.status) { > - DPRINTK(" Gnttab failure mapping tx_ring_ref!\n"); > - return op.status; > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > + > + if (op.status != GNTST_okay) { > + DPRINTK(" Gnttab failure mapping tx_ring_ref %d!\n", (int)op.status); > + return -EINVAL; > } > > netif->tx_shmem_ref = tx_ring_ref; > @@ -267,13 +263,9 @@ static int map_frontend_pages( > > gnttab_set_map_op(&op, (unsigned long)netif->rx_comms_area->addr, > GNTMAP_host_map, rx_ring_ref, netif->domid); > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - if (op.status) { > + if (op.status != GNTST_okay) { > struct gnttab_unmap_grant_ref unop; > > gnttab_set_unmap_op(&unop, > @@ -281,8 +273,8 @@ static int map_frontend_pages( > GNTMAP_host_map, netif->tx_shmem_handle); > VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, > &unop, 1)); > - DPRINTK(" Gnttab failure mapping rx_ring_ref!\n"); > - return op.status; > + DPRINTK(" Gnttab failure mapping rx_ring_ref %d!\n", (int)op.status); > + return -EINVAL; > } > > netif->rx_shmem_ref = rx_ring_ref; > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/netback.c > +++ linux-2.6.18-xen.hg/drivers/xen/netback/netback.c > @@ -494,8 +494,7 @@ static inline void netbk_free_pages(int > used to set up the operations on the top of > netrx_pending_operations, which have since been done. Check that > they didn't give any errors and advance over them. */ > -static int netbk_check_gop(int nr_frags, domid_t domid, > - struct netrx_pending_operations *npo, int *eagain) > +static int netbk_check_gop(int nr_frags, domid_t domid, struct netrx_pending_operations *npo) > { > multicall_entry_t *mcl; > gnttab_transfer_t *gop; > @@ -503,17 +502,15 @@ static int netbk_check_gop(int nr_frags, > int status = NETIF_RSP_OKAY; > int i; > > - *eagain = 0; > - > for (i = 0; i <= nr_frags; i++) { > if (npo->meta[npo->meta_cons + i].copy) { > copy_op = npo->copy + npo->copy_cons++; > - if (copy_op->status != GNTST_okay) { > + if (unlikely(copy_op->status == GNTST_eagain)) > + gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op); > + if (unlikely(copy_op->status != GNTST_okay)) { > DPRINTK("Bad status %d from copy to DOM%d.\n", > copy_op->status, domid); > status = NETIF_RSP_ERROR; > - if(copy_op->status == GNTST_eagain) > - *eagain = 1; > } > } else { > if (!xen_feature(XENFEAT_auto_translated_physmap)) { > @@ -524,7 +521,7 @@ static int netbk_check_gop(int nr_frags, > > gop = npo->trans + npo->trans_cons++; > /* Check the reassignment error code. */ > - if (gop->status != 0) { > + if (unlikely(gop->status != GNTST_okay)) { > DPRINTK("Bad status %d from grant transfer to DOM%u\n", > gop->status, domid); > /* > @@ -533,8 +530,6 @@ static int netbk_check_gop(int nr_frags, > * a fatal error anyway. > */ > BUG_ON(gop->status == GNTST_bad_page); > - if(gop->status == GNTST_eagain) > - *eagain = 1; > status = NETIF_RSP_ERROR; > } > } > @@ -576,7 +571,6 @@ static void net_rx_action(unsigned long > int nr_frags; > int count; > unsigned long offset; > - int eagain; > > /* > * Putting hundreds of bytes on the stack is considered rude. > @@ -680,7 +674,7 @@ static void net_rx_action(unsigned long > > netif = netdev_priv(skb->dev); > > - status = netbk_check_gop(nr_frags, netif->domid, &npo, &eagain); > + status = netbk_check_gop(nr_frags, netif->domid, &npo); > > /* We can't rely on skb_release_data to release the > pages used by fragments for us, since it tries to > @@ -691,22 +685,14 @@ static void net_rx_action(unsigned long > /* (Freeing the fragments is safe since we copy > non-linear skbs destined for flipping interfaces) */ > if (!netif->copying_receiver) { > - /* > - * Cannot handle failed grant transfers at the moment (because > - * mmu_updates likely completed) > - */ > - BUG_ON(eagain); > atomic_set(&(skb_shinfo(skb)->dataref), 1); > skb_shinfo(skb)->frag_list = NULL; > skb_shinfo(skb)->nr_frags = 0; > netbk_free_pages(nr_frags, meta + npo.meta_cons + 1); > } > > - if(!eagain) > - { > - netif->stats.tx_bytes += skb->len; > - netif->stats.tx_packets++; > - } > + netif->stats.tx_bytes += skb->len; > + netif->stats.tx_packets++; > > id = meta[npo.meta_cons].id; > flags = nr_frags ? NETRXF_more_data : 0; > @@ -756,18 +742,8 @@ static void net_rx_action(unsigned long > !netbk_queue_full(netif)) > netif_wake_queue(netif->dev); > > - if(!eagain || netbk_queue_full(netif)) > - { > - netif_put(netif); > - dev_kfree_skb(skb); > - netif->stats.tx_dropped += !!eagain; > - } > - else > - { > - netif->rx_req_cons_peek += skb_shinfo(skb)->nr_frags + 1 + > - !!skb_shinfo(skb)->gso_size; > - skb_queue_head(&rx_queue, skb); > - } > + netif_put(netif); > + dev_kfree_skb(skb); > > npo.meta_cons += nr_frags + 1; > } > @@ -1107,7 +1083,7 @@ static int netbk_tx_check_mop(struct sk_ > > /* Check status of header. */ > err = mop->status; > - if (unlikely(err)) { > + if (unlikely(err != GNTST_okay)) { > txp = &pending_tx_info[pending_idx].req; > make_tx_response(netif, txp, NETIF_RSP_ERROR); > pending_ring[MASK_PEND_IDX(pending_prod++)] = pending_idx; > @@ -1128,12 +1104,12 @@ static int netbk_tx_check_mop(struct sk_ > > /* Check error status: if okay then remember grant handle. */ > newerr = (++mop)->status; > - if (likely(!newerr)) { > + if (likely(newerr == GNTST_okay)) { > set_phys_to_machine(idx_to_pfn(pending_idx), > FOREIGN_FRAME(mop->dev_bus_addr>>PAGE_SHIFT)); > grant_tx_handle[pending_idx] = mop->handle; > /* Had a previous error? Invalidate this fragment. */ > - if (unlikely(err)) > + if (unlikely(err != GNTST_okay)) > netif_idx_release(pending_idx); > continue; > } > @@ -1145,7 +1121,7 @@ static int netbk_tx_check_mop(struct sk_ > netif_put(netif); > > /* Not the first error? Preceding frags already invalidated. */ > - if (err) > + if (err != GNTST_okay) > continue; > > /* First error: invalidate header and preceding fragments. */ > --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/interface.c > @@ -64,27 +64,23 @@ static int map_frontend_page( struct vsc > unsigned long ring_ref) > { > struct gnttab_map_grant_ref op; > - int err; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)info->ring_area->addr, > GNTMAP_host_map, ring_ref, > info->domid); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - do { > - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > - BUG_ON(err); > - msleep(10); > - } while(op.status == GNTST_eagain); > - > - if (op.status) { > - printk(KERN_ERR "scsiback: Grant table operation failure !\n"); > - return op.status; > + if (op.status != GNTST_okay) { > + printk(KERN_ERR "scsiback: Grant table operation failure %d!\n", (int)op.status); > + ret = -EINVAL; > + } else { > + info->shmem_ref = ring_ref; > + info->shmem_handle = op.handle; > + ret = 0; > } > > - info->shmem_ref = ring_ref; > - info->shmem_handle = op.handle; > - > - return (GNTST_okay); > + return ret; > } > > static void unmap_frontend_page(struct vscsibk_info *info) > --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/scsiback.c > +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/scsiback.c > @@ -279,22 +279,14 @@ static int scsiback_gnttab_data_map(vscs > > err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nr_segments); > BUG_ON(err); > - /* Retry maps with GNTST_eagain */ > - for(i=0; i < nr_segments; i++) { > - while(unlikely(map[i].status == GNTST_eagain)) > - { > - msleep(10); > - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > - &map[i], > - 1); > - BUG_ON(err); > - } > - } > > for (i = 0; i < nr_segments; i++) { > struct page *pg; > > - if (unlikely(map[i].status != 0)) { > + /* Retry maps with GNTST_eagain */ > + if (unlikely(map[i].status == GNTST_eagain)) > + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); > + if (unlikely(map[i].status != GNTST_okay)) { > printk(KERN_ERR "scsiback: invalid buffer -- could not remap it\n"); > map[i].handle = SCSIBACK_INVALID_HANDLE; > err |= 1; > --- linux-2.6.18-xen.hg.orig/drivers/xen/sfc_netutil/accel_util.c > +++ linux-2.6.18-xen.hg/drivers/xen/sfc_netutil/accel_util.c > @@ -71,24 +71,27 @@ static int net_accel_map_grant(struct xe > u64 *dev_bus_addr, unsigned flags) > { > struct gnttab_map_grant_ref op; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)vaddr, flags, > gnt_ref, dev->otherend_id); > > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > if (op.status != GNTST_okay) { > xenbus_dev_error > (dev, op.status, > "failed mapping in shared page %d from domain %d\n", > gnt_ref, dev->otherend_id); > + ret = -EINVAL; > } else { > *handle = op.handle; > if (dev_bus_addr) > *dev_bus_addr = op.dev_bus_addr; > + ret = 0; > } > > - return op.status; > + return ret; > } > > > @@ -112,7 +115,7 @@ static int net_accel_unmap_grant(struct > "failed unmapping page at handle %d error %d\n", > handle, op.status); > > - return op.status; > + return op.status == GNTST_okay ? 0 : -EINVAL; > } > > > @@ -144,7 +147,7 @@ struct net_accel_valloc_grant_mapping { > /* Map a series of grants into a contiguous virtual area */ > static void *net_accel_map_grants_valloc(struct xenbus_device *dev, > unsigned *grants, int npages, > - unsigned flags, void **priv, int *errno) > + unsigned flags, void **priv) > { > struct net_accel_valloc_grant_mapping *map; > struct vm_struct *vm; > @@ -172,16 +175,12 @@ static void *net_accel_map_grants_valloc > > /* Do the actual mapping */ > addr = vm->addr; > - if(errno != NULL) *errno = 0; > + > for (i = 0; i < npages; i++) { > rc = net_accel_map_grant(dev, grants[i], map->grant_handles + i, > addr, NULL, flags); > - if (rc != 0) > - { > - if(errno != NULL) > - *errno = (rc == GNTST_eagain ? -EAGAIN : -EINVAL); > + if (rc < 0) > goto undo; > - } > addr = (void*)((unsigned long)addr + PAGE_SIZE); > } > > @@ -230,16 +229,7 @@ void *net_accel_map_grants_contig(struct > unsigned *grants, int npages, > void **priv) > { > - int errno; > - void *ret; > - > - do { > - ret = net_accel_map_grants_valloc(dev, grants, npages, > - GNTMAP_host_map, priv, &errno); > - if(errno) msleep(10); > - } while(errno == -EAGAIN); > - > - return ret; > + return net_accel_map_grants_valloc(dev, grants, npages, GNTMAP_host_map, priv); > } > EXPORT_SYMBOL(net_accel_map_grants_contig); > > @@ -255,16 +245,7 @@ EXPORT_SYMBOL(net_accel_unmap_grants_con > void *net_accel_map_iomem_page(struct xenbus_device *dev, int gnt_ref, > void **priv) > { > - int errno; > - void *ret; > - > - do { > - ret = net_accel_map_grants_valloc(dev, &gnt_ref, 1, > - GNTMAP_host_map, priv, &errno); > - if(errno) msleep(10); > - } while(errno == -EAGAIN); > - > - return ret; > + return net_accel_map_grants_valloc(dev, &gnt_ref, 1, GNTMAP_host_map, priv); > } > EXPORT_SYMBOL(net_accel_map_iomem_page); > > --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/interface.c > @@ -81,25 +81,23 @@ tpmif_t *tpmif_find(domid_t domid, struc > static int map_frontend_page(tpmif_t *tpmif, unsigned long shared_page) > { > struct gnttab_map_grant_ref op; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr, > GNTMAP_host_map, shared_page, tpmif->domid); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - if (op.status) { > - DPRINTK(" Grant table operation failure !\n"); > - return op.status; > + if (op.status != GNTST_okay) { > + DPRINTK(" Grant table operation failure %d!\n", (int)op.status); > + ret = -EINVAL; > + } else { > + tpmif->shmem_ref = shared_page; > + tpmif->shmem_handle = op.handle; > + ret = 0; > } > > - tpmif->shmem_ref = shared_page; > - tpmif->shmem_handle = op.handle; > - > - return 0; > + return ret; > } > > static void unmap_frontend_page(tpmif_t *tpmif) > --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/tpmback.c > +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/tpmback.c > @@ -257,20 +257,15 @@ int _packet_write(struct packet *pak, > gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), > GNTMAP_host_map, tx->ref, tpmif->domid); > > - do { > - if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > - &map_op, 1))) > - BUG(); > - if(map_op.status) msleep(10); > - } while(map_op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op); > > - handle = map_op.handle; > - > - if (map_op.status) { > + if (map_op.status != GNTST_okay) { > DPRINTK(" Grant table operation failure !\n"); > return 0; > } > > + handle = map_op.handle; > + > tocopy = min_t(size_t, size - offset, PAGE_SIZE); > > if (copy_from_buffer((void *)(idx_to_kaddr(tpmif, i) | > @@ -397,14 +392,9 @@ static int packet_read_shmem(struct pack > gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), > GNTMAP_host_map, tx->ref, tpmif->domid); > > - do { > - if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > - &map_op, 1))) > - BUG(); > - if(map_op.status) msleep(10); > - } while(map_op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op); > > - if (map_op.status) { > + if (map_op.status != GNTST_okay) { > DPRINTK(" Grant table operation failure !\n"); > return -EFAULT; > } > --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/interface.c > +++ linux-2.6.18-xen.hg/drivers/xen/usbback/interface.c > @@ -110,16 +110,11 @@ static int map_frontend_pages(usbif_t *u > gnttab_set_map_op(&op, (unsigned long)usbif->urb_ring_area->addr, > GNTMAP_host_map, urb_ring_ref, usbif->domid); > > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while (op.status == GNTST_eagain); > - > - if (op.status) { > - printk(KERN_ERR "grant table failure mapping urb_ring_ref\n"); > - return op.status; > + if (op.status != GNTST_okay) { > + printk(KERN_ERR "grant table failure mapping urb_ring_ref %d\n", (int)op.status); > + return -EINVAL; > } > > usbif->urb_shmem_ref = urb_ring_ref; > @@ -128,21 +123,17 @@ static int map_frontend_pages(usbif_t *u > gnttab_set_map_op(&op, (unsigned long)usbif->conn_ring_area->addr, > GNTMAP_host_map, conn_ring_ref, usbif->domid); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while (op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > - if (op.status) { > + if (op.status != GNTST_okay) { > struct gnttab_unmap_grant_ref unop; > gnttab_set_unmap_op(&unop, > (unsigned long) usbif->urb_ring_area->addr, > GNTMAP_host_map, usbif->urb_shmem_handle); > VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop, > 1)); > - printk(KERN_ERR "grant table failure mapping conn_ring_ref\n"); > - return op.status; > + printk(KERN_ERR "grant table failure mapping conn_ring_ref %d\n", (int)op.status); > + return -EINVAL; > } > > usbif->conn_shmem_ref = conn_ring_ref; > --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/usbback.c > +++ linux-2.6.18-xen.hg/drivers/xen/usbback/usbback.c > @@ -392,19 +392,13 @@ static int usbbk_gnttab_map(usbif_t *usb > ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, > map, nr_segs); > BUG_ON(ret); > - /* Make sure than none of the map ops failed with GNTST_eagain */ > - for( i = 0; i < nr_segs; i++) { > - while(map[i].status == GNTST_eagain) { > - msleep(10); > - ret = HYPERVISOR_grant_table_op( > - GNTTABOP_map_grant_ref, > - &map[i], 1); > - BUG_ON(ret); > - } > - } > > for (i = 0; i < nr_segs; i++) { > - if (unlikely(map[i].status != 0)) { > + /* Make sure than none of the map ops failed with GNTST_eagain */ > + if (unlikely(map[i].status == GNTST_eagain)) > + gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); > + > + if (unlikely(map[i].status != GNTST_okay)) { > printk(KERN_ERR "usbback: invalid buffer -- could not remap it\n"); > map[i].handle = USBBACK_INVALID_HANDLE; > ret |= 1; > --- linux-2.6.18-xen.hg.orig/drivers/xen/xenbus/xenbus_backend_client.c > +++ linux-2.6.18-xen.hg/drivers/xen/xenbus/xenbus_backend_client.c > @@ -49,11 +49,7 @@ struct vm_struct *xenbus_map_ring_valloc > gnttab_set_map_op(&op, (unsigned long)area->addr, GNTMAP_host_map, > gnt_ref, dev->otherend_id); > > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(op.status == GNTST_eagain); > + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); > > if (op.status != GNTST_okay) { > free_vm_area(area); > @@ -61,7 +57,7 @@ struct vm_struct *xenbus_map_ring_valloc > "mapping in shared page %d from domain %d", > gnt_ref, dev->otherend_id); > BUG_ON(!IS_ERR(ERR_PTR(op.status))); > - return ERR_PTR(op.status); > + return ERR_PTR(-EINVAL); > } > > /* Stuff the handle in an unused field */ > @@ -76,23 +72,24 @@ int xenbus_map_ring(struct xenbus_device > grant_handle_t *handle, void *vaddr) > { > struct gnttab_map_grant_ref op; > + int ret; > > gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map, > gnt_ref, dev->otherend_id); > - do { > - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) > - BUG(); > - msleep(10); > - } while(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", > gnt_ref, dev->otherend_id); > - } else > + ret = -EINVAL; > + } else { > *handle = op.handle; > + ret = 0; > + } > > - return op.status; > + return ret; > } > EXPORT_SYMBOL_GPL(xenbus_map_ring); > > @@ -115,7 +112,7 @@ int xenbus_unmap_ring_vfree(struct xenbu > "unmapping page at handle %d error %d", > (int16_t)area->phys_addr, op.status); > > - return op.status; > + return op.status == GNTST_okay ? 0 : -EINVAL; > } > EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree); > > @@ -135,7 +132,7 @@ int xenbus_unmap_ring(struct xenbus_devi > "unmapping page at handle %d error %d", > handle, op.status); > > - return op.status; > + return op.status == GNTST_okay ? 0 : -EINVAL; > } > EXPORT_SYMBOL_GPL(xenbus_unmap_ring); > > --- linux-2.6.18-xen.hg.orig/include/xen/gnttab.h > +++ linux-2.6.18-xen.hg/include/xen/gnttab.h > @@ -40,6 +40,7 @@ > #include <asm/hypervisor.h> > #include <asm/maddr.h> /* maddr_t */ > #include <linux/mm.h> > +#include <linux/delay.h> > #include <xen/interface/grant_table.h> > #include <xen/features.h> > > @@ -161,4 +162,41 @@ gnttab_set_replace_op(struct gnttab_unma > unmap->handle = handle; > } > > +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) \ > +{ \ > + 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: %s gnt busy\n", __func__, current->comm); \ > + (__HCarg_p)->status = GNTST_bad_page; \ > + } \ > + if ((__HCarg_p)->status != GNTST_okay) \ > + printk(KERN_ERR "%s: %s gnt status %x\n", \ > + __func__, current->comm, (__HCarg_p)->status); \ > +} > + > +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p) \ > +{ \ > + 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: %s gnt busy\n", __func__, current->comm); \ > + (__HCarg_p)->status = GNTST_bad_page; \ > + } \ > + if ((__HCarg_p)->status != GNTST_okay) \ > + printk(KERN_ERR "%s: %s gnt status %x\n", \ > + __func__, current->comm, (__HCarg_p)->status); \ > +} > + > #endif /* __ASM_GNTTAB_H__ */ > > [-- Attachment #2: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re: [PATCH] xenpaging: handle GNTST_eagain in kernel drivers 2010-09-16 3:44 ` Patrick Colp @ 2010-09-16 9:05 ` Keir Fraser 2010-09-16 13:33 ` Olaf Hering 1 sibling, 0 replies; 16+ messages in thread From: Keir Fraser @ 2010-09-16 9:05 UTC (permalink / raw) To: Patrick Colp, Olaf Hering; +Cc: xen-devel@lists.xensource.com Olaf, The patch doesn't apply for me to the linux-2.6.18-xen tree. I seem to have problems with very large inline patches getting corrupted by our mail server. You might want to try resending as an attachment. -- Keir On 16/09/2010 04:44, "Patrick Colp" <pjcolp@cs.ubc.ca> wrote: > Is there any particular reason for 33 seconds? This number seems a bit > high to me. Did you run into situations where that amount of time was > needed? Otherwise I'm happy with everything. Thanks for all the hard > work! > > Acked-by: Patrick Colp <pjcolp@cs.ubc.ca> > > > Patrick > > > On 15 September 2010 09:08, Olaf Hering <olaf@aepfle.de> wrote: >> >> 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. >> >> Remove the existing retry code from net_rx_action(), dispatch_rw_block_io(), >> net_accel_map_grants_contig() and net_accel_map_iomem_page() and 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 <olaf@aepfle.de> >> >> --- >> Compiled with x86_64 and i386 configs >> >> drivers/xen/blkback/blkback.c | 57 >> ++++++----------------------- >> drivers/xen/blkback/common.h | 2 - >> drivers/xen/blkback/interface.c | 22 +++++------ >> drivers/xen/blktap/blktap.c | 29 ++++++-------- >> drivers/xen/blktap/interface.c | 22 +++++------ >> drivers/xen/blktap2/device.c | 34 ++++++++--------- >> drivers/xen/core/gnttab.c | 4 +- >> drivers/xen/gntdev/gntdev.c | 26 ++++++------- >> drivers/xen/netback/interface.c | 26 ++++--------- >> drivers/xen/netback/netback.c | 52 +++++++------------------- >> drivers/xen/scsiback/interface.c | 24 +++++------- >> drivers/xen/scsiback/scsiback.c | 16 ++------ >> drivers/xen/sfc_netutil/accel_util.c | 41 +++++--------------- >> drivers/xen/tpmback/interface.c | 22 +++++------ >> drivers/xen/tpmback/tpmback.c | 22 +++-------- >> drivers/xen/usbback/interface.c | 25 ++++-------- >> drivers/xen/usbback/usbback.c | 16 ++------ >> drivers/xen/xenbus/xenbus_backend_client.c | 27 ++++++------- >> include/xen/gnttab.h | 38 +++++++++++++++++++ >> 19 files changed, 204 insertions(+), 301 deletions(-) >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/blkback.c >> +++ linux-2.6.18-xen.hg/drivers/xen/blkback/blkback.c >> @@ -107,7 +107,7 @@ static inline unsigned long vaddr(pendin >> >> >> static int do_block_io_op(blkif_t *blkif); >> -static int dispatch_rw_block_io(blkif_t *blkif, >> +static void dispatch_rw_block_io(blkif_t *blkif, >> blkif_request_t *req, >> pending_req_t *pending_req); >> static void make_response(blkif_t *blkif, u64 id, >> @@ -312,13 +312,13 @@ static int do_block_io_op(blkif_t *blkif >> blkif_request_t req; >> pending_req_t *pending_req; >> RING_IDX rc, rp; >> - int more_to_do = 0, ret; >> + int more_to_do = 0; >> >> rc = blk_rings->common.req_cons; >> rp = blk_rings->common.sring->req_prod; >> rmb(); /* Ensure we see queued requests up to 'rp'. */ >> >> - while ((rc != rp) || (blkif->is_suspended_req)) { >> + while ((rc != rp)) { >> >> if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) >> break; >> @@ -335,14 +335,6 @@ static int do_block_io_op(blkif_t *blkif >> break; >> } >> >> - /* Handle the suspended request first, if one exists */ >> - if(blkif->is_suspended_req) >> - { >> - memcpy(&req, &blkif->suspended_req, sizeof(req)); >> - blkif->is_suspended_req = 0; >> - goto handle_request; >> - } >> - >> switch (blkif->blk_protocol) { >> case BLKIF_PROTOCOL_NATIVE: >> memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), >> sizeof(req)); >> @@ -361,19 +353,17 @@ static int do_block_io_op(blkif_t *blkif >> /* Apply all sanity checks to /private copy/ of request. */ >> barrier(); >> >> -handle_request: >> - ret = 0; >> switch (req.operation) { >> case BLKIF_OP_READ: >> blkif->st_rd_req++; >> - ret = dispatch_rw_block_io(blkif, &req, pending_req); >> + dispatch_rw_block_io(blkif, &req, pending_req); >> break; >> case BLKIF_OP_WRITE_BARRIER: >> blkif->st_br_req++; >> /* fall through */ >> case BLKIF_OP_WRITE: >> blkif->st_wr_req++; >> - ret = dispatch_rw_block_io(blkif, &req, pending_req); >> + dispatch_rw_block_io(blkif, &req, pending_req); >> break; >> default: >> /* A good sign something is wrong: sleep for a while >> to >> @@ -386,17 +376,6 @@ handle_request: >> free_req(pending_req); >> break; >> } >> - BUG_ON(ret != 0 && ret != -EAGAIN); >> - /* If we can't handle the request at the moment, save it, and break >> the >> - * loop */ >> - if(ret == -EAGAIN) >> - { >> - memcpy(&blkif->suspended_req, &req, sizeof(req)); >> - blkif->is_suspended_req = 1; >> - /* Return "no more work pending", restart will be handled 'out >> of >> - * band' */ >> - return 0; >> - } >> >> /* Yield point for this unbounded loop. */ >> cond_resched(); >> @@ -405,7 +384,7 @@ handle_request: >> return more_to_do; >> } >> >> -static int dispatch_rw_block_io(blkif_t *blkif, >> +static void dispatch_rw_block_io(blkif_t *blkif, >> blkif_request_t *req, >> pending_req_t *pending_req) >> { >> @@ -474,15 +453,13 @@ static int dispatch_rw_block_io(blkif_t >> ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg); >> BUG_ON(ret); >> >> -#define GENERAL_ERR (1<<0) >> -#define EAGAIN_ERR (1<<1) >> 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)) { >> DPRINTK("invalid buffer -- could not remap it\n"); >> map[i].handle = BLKBACK_INVALID_HANDLE; >> - ret |= GENERAL_ERR; >> - if(map[i].status == GNTST_eagain) >> - ret |= EAGAIN_ERR; >> + ret = 1; >> } else { >> blkback_pagemap_set(vaddr_pagenr(pending_req, i), >> pending_page(pending_req, i), >> @@ -502,14 +479,6 @@ static int dispatch_rw_block_io(blkif_t >> (req->seg[i].first_sect << 9); >> } >> >> - /* If any of grant maps failed with GNTST_eagain, suspend and retry >> later */ >> - if(ret & EAGAIN_ERR) >> - { >> - fast_flush_area(pending_req); >> - free_req(pending_req); >> - return -EAGAIN; >> - } >> - >> if (ret) >> goto fail_flush; >> >> @@ -575,7 +544,7 @@ static int dispatch_rw_block_io(blkif_t >> else if (operation == WRITE || operation == WRITE_BARRIER) >> blkif->st_wr_sect += preq.nr_sects; >> >> - return 0; >> + return; >> >> fail_flush: >> fast_flush_area(pending_req); >> @@ -583,7 +552,7 @@ static int dispatch_rw_block_io(blkif_t >> make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR); >> free_req(pending_req); >> msleep(1); /* back off a bit */ >> - return 0; >> + return; >> >> fail_put_bio: >> __end_block_io_op(pending_req, -EINVAL); >> @@ -591,7 +560,7 @@ static int dispatch_rw_block_io(blkif_t >> bio_put(bio); >> unplug_queue(blkif); >> msleep(1); /* back off a bit */ >> - return 0; >> + return; >> } >> >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/common.h >> +++ linux-2.6.18-xen.hg/drivers/xen/blkback/common.h >> @@ -83,8 +83,6 @@ typedef struct blkif_st { >> struct task_struct *xenblkd; >> unsigned int waiting_reqs; >> request_queue_t *plug; >> - int is_suspended_req; >> - blkif_request_t suspended_req; >> >> /* statistics */ >> unsigned long st_print; >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blkback/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/blkback/interface.c >> @@ -59,25 +59,23 @@ blkif_t *blkif_alloc(domid_t domid) >> static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) >> { >> struct gnttab_map_grant_ref op; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, >> GNTMAP_host_map, shared_page, blkif->domid); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(100); >> - } while(op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - if (op.status) { >> - DPRINTK(" Grant table operation failure !\n"); >> - return op.status; >> + if (op.status == GNTST_okay) { >> + blkif->shmem_ref = shared_page; >> + blkif->shmem_handle = op.handle; >> + ret = 0; >> + } else { >> + DPRINTK(" Grant table operation failure %d!\n", >> (int)op.status); >> + ret = -EINVAL; >> } >> >> - blkif->shmem_ref = shared_page; >> - blkif->shmem_handle = op.handle; >> - >> - return 0; >> + return ret; >> } >> >> static void unmap_frontend_page(blkif_t *blkif) >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/blktap.c >> +++ linux-2.6.18-xen.hg/drivers/xen/blktap/blktap.c >> @@ -1526,19 +1526,17 @@ static void dispatch_rw_block_io(blkif_t >> >> uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i/2); >> >> - if (unlikely(map[i].status != 0)) { >> - WPRINTK("invalid kernel buffer -- " >> - "could not remap it\n"); >> - if(map[i].status == GNTST_eagain) >> - WPRINTK("grant GNTST_eagain: please use blktap2\n"); >> - ret |= 1; >> + >> gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); >> + >> + if (unlikely(map[i].status != GNTST_okay)) { >> + WPRINTK("invalid kernel buffer -- could not >> remap it\n"); >> + ret = 1; >> map[i].handle = INVALID_GRANT_HANDLE; >> } >> >> - if (unlikely(map[i+1].status != 0)) { >> - WPRINTK("invalid user buffer -- " >> - "could not remap it\n"); >> - ret |= 1; >> + if (unlikely(map[i+1].status != GNTST_okay)) { >> + WPRINTK("invalid kernel buffer -- could not >> remap it\n"); >> + ret = 1; >> map[i+1].handle = INVALID_GRANT_HANDLE; >> } >> >> @@ -1565,12 +1563,11 @@ static void dispatch_rw_block_io(blkif_t >> >> uvaddr = MMAP_VADDR(info->user_vstart, usr_idx, i); >> >> - if (unlikely(map[i].status != 0)) { >> - WPRINTK("invalid kernel buffer -- " >> - "could not remap it\n"); >> - if(map[i].status == GNTST_eagain) >> - WPRINTK("grant GNTST_eagain: please use blktap2\n"); >> - ret |= 1; >> + >> gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); >> + >> + if (unlikely(map[i].status != GNTST_okay)) { >> + WPRINTK("invalid kernel buffer -- could not >> remap it\n"); >> + ret = 1; >> map[i].handle = INVALID_GRANT_HANDLE; >> } >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/blktap/interface.c >> @@ -59,25 +59,23 @@ blkif_t *tap_alloc_blkif(domid_t domid) >> static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) >> { >> struct gnttab_map_grant_ref op; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)blkif->blk_ring_area->addr, >> GNTMAP_host_map, shared_page, blkif->domid); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - if (op.status) { >> - DPRINTK(" Grant table operation failure !\n"); >> - return op.status; >> + if (op.status == GNTST_okay) { >> + blkif->shmem_ref = shared_page; >> + blkif->shmem_handle = op.handle; >> + ret = 0; >> + } else { >> + DPRINTK("Grant table operation failure %d!\n", >> (int)op.status); >> + ret = -EINVAL; >> } >> >> - blkif->shmem_ref = shared_page; >> - blkif->shmem_handle = op.handle; >> - >> - return 0; >> + return ret; >> } >> >> static void unmap_frontend_page(blkif_t *blkif) >> --- linux-2.6.18-xen.hg.orig/drivers/xen/blktap2/device.c >> +++ linux-2.6.18-xen.hg/drivers/xen/blktap2/device.c >> @@ -505,10 +505,10 @@ blktap_map_foreign(struct blktap *tap, >> >> uvaddr = MMAP_VADDR(ring->user_vstart, usr_idx, i); >> >> - if (unlikely(table->grants[grant].status)) { >> + if (unlikely(table->grants[grant].status != GNTST_okay)) { >> BTERR("invalid kernel buffer: could not remap it\n"); >> - /* This should never happen: blkback should handle eagain first >> */ >> - BUG_ON(table->grants[grant].status == GNTST_eagain); >> + /* This should never happen: blkback should handle >> eagain first */ >> + BUG_ON(table->grants[grant].status == GNTST_eagain); >> err |= 1; >> table->grants[grant].handle = INVALID_GRANT_HANDLE; >> } >> @@ -517,19 +517,18 @@ blktap_map_foreign(struct blktap *tap, >> foreign_mfn = table->grants[grant].dev_bus_addr >> PAGE_SHIFT; >> grant++; >> >> - if (xen_feature(XENFEAT_auto_translated_physmap)) >> - goto done; >> - >> - if (unlikely(table->grants[grant].status)) { >> - BTERR("invalid user buffer: could not remap it\n"); >> - err |= 1; >> + if (!xen_feature(XENFEAT_auto_translated_physmap)) { >> + if (unlikely(table->grants[grant].status != >> GNTST_okay)) { >> + /* This should never happen: blkback should >> handle eagain first */ >> + WARN_ON(table->grants[grant].status == >> GNTST_eagain); >> + BTERR("invalid user buffer: could not remap >> it\n"); >> + err |= 1; >> + } >> table->grants[grant].handle = INVALID_GRANT_HANDLE; >> + request->handles[i].user = >> table->grants[grant].handle; >> + grant++; >> } >> >> - request->handles[i].user = table->grants[grant].handle; >> - grant++; >> - >> - done: >> if (err) >> continue; >> >> @@ -602,11 +601,10 @@ blktap_map(struct blktap *tap, >> set_page_private(tap_page, page_private(page)); >> SetPageBlkback(tap_page); >> >> - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> - &map, 1); >> - BUG_ON(err); >> - /* We are not expecting the grant op to fail */ >> - BUG_ON(map.status != GNTST_okay); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, >> &map); >> + >> + /* We are not expecting the grant op to fail */ >> + BUG_ON(map.status != GNTST_okay); >> >> err = vm_insert_page(ring->vma, uvaddr, tap_page); >> if (err) { >> --- linux-2.6.18-xen.hg.orig/drivers/xen/core/gnttab.c >> +++ linux-2.6.18-xen.hg/drivers/xen/core/gnttab.c >> @@ -487,7 +487,7 @@ static int gnttab_map(unsigned int start >> return -ENOSYS; >> } >> >> - BUG_ON(rc || setup.status); >> + BUG_ON(rc || setup.status != GNTST_okay); >> >> if (shared == NULL) >> shared = arch_gnttab_alloc_shared(frames); >> @@ -571,7 +571,7 @@ int gnttab_copy_grant_page(grant_ref_t r >> err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace, >> &unmap, 1); >> BUG_ON(err); >> - BUG_ON(unmap.status); >> + BUG_ON(unmap.status != GNTST_okay); >> >> write_sequnlock(&gnttab_dma_lock); >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/gntdev/gntdev.c >> +++ linux-2.6.18-xen.hg/drivers/xen/gntdev/gntdev.c >> @@ -578,7 +578,7 @@ static int gntdev_mmap (struct file *fli >> vma->vm_mm->context.has_foreign_mappings = 1; >> #endif >> >> - exit_ret = -ENOMEM; >> + exit_ret = -ENOMEM; >> for (i = 0; i < size; ++i) { >> >> flags = GNTMAP_host_map; >> @@ -599,8 +599,8 @@ static int gntdev_mmap (struct file *fli >> ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> &op, 1); >> BUG_ON(ret); >> - if (op.status) { >> - if(op.status != GNTST_eagain) >> + if (op.status != GNTST_okay) { >> + if (op.status != GNTST_eagain) >> printk(KERN_ERR "Error mapping the grant >> reference " >> "into the kernel (%d). domid = %d; ref >> = %d\n", >> op.status, >> @@ -608,9 +608,9 @@ static int gntdev_mmap (struct file *fli >> .u.valid.domid, >> private_data->grants[slot_index+i] >> .u.valid.ref); >> - else >> - /* Propagate eagain instead of trying to fix it up */ >> - exit_ret = -EAGAIN; >> + else >> + /* Propagate eagain instead of trying to fix >> it up */ >> + exit_ret = -EAGAIN; >> goto undo_map_out; >> } >> >> @@ -679,7 +679,7 @@ static int gntdev_mmap (struct file *fli >> ret = >> HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> &op, 1); >> BUG_ON(ret); >> - if (op.status) { >> + if (op.status != GNTST_okay) { >> printk(KERN_ERR "Error mapping the grant " >> "reference into user space (%d). domid >> " >> "= %d; ref = %d\n", op.status, >> @@ -687,9 +687,9 @@ static int gntdev_mmap (struct file *fli >> .valid.domid, >> private_data->grants[slot_index+i].u >> .valid.ref); >> - /* This should never happen after we've mapped into >> - * the kernel space. */ >> - BUG_ON(op.status == GNTST_eagain); >> + /* This should never happen after we've >> mapped into >> + * the kernel space. */ >> + BUG_ON(op.status == GNTST_eagain); >> goto undo_map_out; >> } >> >> @@ -713,7 +713,7 @@ static int gntdev_mmap (struct file *fli >> } >> >> } >> - exit_ret = 0; >> + exit_ret = 0; >> >> up_write(&private_data->grants_sem); >> return exit_ret; >> @@ -785,7 +785,7 @@ static pte_t gntdev_clear_pte(struct vm_ >> ret = HYPERVISOR_grant_table_op( >> GNTTABOP_unmap_grant_ref, &op, 1); >> BUG_ON(ret); >> - if (op.status) >> + if (op.status != GNTST_okay) >> printk("User unmap grant status = %d\n", >> op.status); >> } else { >> @@ -802,7 +802,7 @@ static pte_t gntdev_clear_pte(struct vm_ >> ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, >> &op, 1); >> BUG_ON(ret); >> - if (op.status) >> + if (op.status != GNTST_okay) >> printk("Kernel unmap grant status = %d\n", op.status); >> >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c >> @@ -251,15 +251,11 @@ static int map_frontend_pages( >> >> gnttab_set_map_op(&op, (unsigned long)netif->tx_comms_area->addr, >> GNTMAP_host_map, tx_ring_ref, netif->domid); >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, >> 1)) >> - BUG(); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> - >> - if (op.status) { >> - DPRINTK(" Gnttab failure mapping tx_ring_ref!\n"); >> - return op.status; >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> + >> + if (op.status != GNTST_okay) { >> + DPRINTK(" Gnttab failure mapping tx_ring_ref %d!\n", >> (int)op.status); >> + return -EINVAL; >> } >> >> netif->tx_shmem_ref = tx_ring_ref; >> @@ -267,13 +263,9 @@ static int map_frontend_pages( >> >> gnttab_set_map_op(&op, (unsigned long)netif->rx_comms_area->addr, >> GNTMAP_host_map, rx_ring_ref, netif->domid); >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - if (op.status) { >> + if (op.status != GNTST_okay) { >> struct gnttab_unmap_grant_ref unop; >> >> gnttab_set_unmap_op(&unop, >> @@ -281,8 +273,8 @@ static int map_frontend_pages( >> GNTMAP_host_map, netif->tx_shmem_handle); >> VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, >> &unop, 1)); >> - DPRINTK(" Gnttab failure mapping rx_ring_ref!\n"); >> - return op.status; >> + DPRINTK(" Gnttab failure mapping rx_ring_ref %d!\n", >> (int)op.status); >> + return -EINVAL; >> } >> >> netif->rx_shmem_ref = rx_ring_ref; >> --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/netback.c >> +++ linux-2.6.18-xen.hg/drivers/xen/netback/netback.c >> @@ -494,8 +494,7 @@ static inline void netbk_free_pages(int >> used to set up the operations on the top of >> netrx_pending_operations, which have since been done. Check that >> they didn't give any errors and advance over them. */ >> -static int netbk_check_gop(int nr_frags, domid_t domid, >> - struct netrx_pending_operations *npo, int *eagain) >> +static int netbk_check_gop(int nr_frags, domid_t domid, struct >> netrx_pending_operations *npo) >> { >> multicall_entry_t *mcl; >> gnttab_transfer_t *gop; >> @@ -503,17 +502,15 @@ static int netbk_check_gop(int nr_frags, >> int status = NETIF_RSP_OKAY; >> int i; >> >> - *eagain = 0; >> - >> for (i = 0; i <= nr_frags; i++) { >> if (npo->meta[npo->meta_cons + i].copy) { >> copy_op = npo->copy + npo->copy_cons++; >> - if (copy_op->status != GNTST_okay) { >> + if (unlikely(copy_op->status == GNTST_eagain)) >> + >> gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op); >> + if (unlikely(copy_op->status != GNTST_okay)) { >> DPRINTK("Bad status %d from copy to DOM%d.\n", >> copy_op->status, domid); >> status = NETIF_RSP_ERROR; >> - if(copy_op->status == GNTST_eagain) >> - *eagain = 1; >> } >> } else { >> if (!xen_feature(XENFEAT_auto_translated_physmap)) { >> @@ -524,7 +521,7 @@ static int netbk_check_gop(int nr_frags, >> >> gop = npo->trans + npo->trans_cons++; >> /* Check the reassignment error code. */ >> - if (gop->status != 0) { >> + if (unlikely(gop->status != GNTST_okay)) { >> DPRINTK("Bad status %d from grant transfer to >> DOM%u\n", >> gop->status, domid); >> /* >> @@ -533,8 +530,6 @@ static int netbk_check_gop(int nr_frags, >> * a fatal error anyway. >> */ >> BUG_ON(gop->status == GNTST_bad_page); >> - if(gop->status == GNTST_eagain) >> - *eagain = 1; >> status = NETIF_RSP_ERROR; >> } >> } >> @@ -576,7 +571,6 @@ static void net_rx_action(unsigned long >> int nr_frags; >> int count; >> unsigned long offset; >> - int eagain; >> >> /* >> * Putting hundreds of bytes on the stack is considered rude. >> @@ -680,7 +674,7 @@ static void net_rx_action(unsigned long >> >> netif = netdev_priv(skb->dev); >> >> - status = netbk_check_gop(nr_frags, netif->domid, &npo, >> &eagain); >> + status = netbk_check_gop(nr_frags, netif->domid, &npo); >> >> /* We can't rely on skb_release_data to release the >> pages used by fragments for us, since it tries to >> @@ -691,22 +685,14 @@ static void net_rx_action(unsigned long >> /* (Freeing the fragments is safe since we copy >> non-linear skbs destined for flipping interfaces) */ >> if (!netif->copying_receiver) { >> - /* >> - * Cannot handle failed grant transfers at the moment (because >> - * mmu_updates likely completed) >> - */ >> - BUG_ON(eagain); >> atomic_set(&(skb_shinfo(skb)->dataref), 1); >> skb_shinfo(skb)->frag_list = NULL; >> skb_shinfo(skb)->nr_frags = 0; >> netbk_free_pages(nr_frags, meta + npo.meta_cons + 1); >> } >> >> - if(!eagain) >> - { >> - netif->stats.tx_bytes += skb->len; >> - netif->stats.tx_packets++; >> - } >> + netif->stats.tx_bytes += skb->len; >> + netif->stats.tx_packets++; >> >> id = meta[npo.meta_cons].id; >> flags = nr_frags ? NETRXF_more_data : 0; >> @@ -756,18 +742,8 @@ static void net_rx_action(unsigned long >> !netbk_queue_full(netif)) >> netif_wake_queue(netif->dev); >> >> - if(!eagain || netbk_queue_full(netif)) >> - { >> - netif_put(netif); >> - dev_kfree_skb(skb); >> - netif->stats.tx_dropped += !!eagain; >> - } >> - else >> - { >> - netif->rx_req_cons_peek += skb_shinfo(skb)->nr_frags + 1 + >> - !!skb_shinfo(skb)->gso_size; >> - skb_queue_head(&rx_queue, skb); >> - } >> + netif_put(netif); >> + dev_kfree_skb(skb); >> >> npo.meta_cons += nr_frags + 1; >> } >> @@ -1107,7 +1083,7 @@ static int netbk_tx_check_mop(struct sk_ >> >> /* Check status of header. */ >> err = mop->status; >> - if (unlikely(err)) { >> + if (unlikely(err != GNTST_okay)) { >> txp = &pending_tx_info[pending_idx].req; >> make_tx_response(netif, txp, NETIF_RSP_ERROR); >> pending_ring[MASK_PEND_IDX(pending_prod++)] = pending_idx; >> @@ -1128,12 +1104,12 @@ static int netbk_tx_check_mop(struct sk_ >> >> /* Check error status: if okay then remember grant handle. */ >> newerr = (++mop)->status; >> - if (likely(!newerr)) { >> + if (likely(newerr == GNTST_okay)) { >> set_phys_to_machine(idx_to_pfn(pending_idx), >> FOREIGN_FRAME(mop->dev_bus_addr>>PAGE_SHIFT)); >> grant_tx_handle[pending_idx] = mop->handle; >> /* Had a previous error? Invalidate this fragment. */ >> - if (unlikely(err)) >> + if (unlikely(err != GNTST_okay)) >> netif_idx_release(pending_idx); >> continue; >> } >> @@ -1145,7 +1121,7 @@ static int netbk_tx_check_mop(struct sk_ >> netif_put(netif); >> >> /* Not the first error? Preceding frags already invalidated. >> */ >> - if (err) >> + if (err != GNTST_okay) >> continue; >> >> /* First error: invalidate header and preceding fragments. */ >> --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/interface.c >> @@ -64,27 +64,23 @@ static int map_frontend_page( struct vsc >> unsigned long ring_ref) >> { >> struct gnttab_map_grant_ref op; >> - int err; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)info->ring_area->addr, >> GNTMAP_host_map, ring_ref, >> info->domid); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - do { >> - err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); >> - BUG_ON(err); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> - >> - if (op.status) { >> - printk(KERN_ERR "scsiback: Grant table operation failure >> !\n"); >> - return op.status; >> + if (op.status != GNTST_okay) { >> + printk(KERN_ERR "scsiback: Grant table operation failure >> %d!\n", (int)op.status); >> + ret = -EINVAL; >> + } else { >> + info->shmem_ref = ring_ref; >> + info->shmem_handle = op.handle; >> + ret = 0; >> } >> >> - info->shmem_ref = ring_ref; >> - info->shmem_handle = op.handle; >> - >> - return (GNTST_okay); >> + return ret; >> } >> >> static void unmap_frontend_page(struct vscsibk_info *info) >> --- linux-2.6.18-xen.hg.orig/drivers/xen/scsiback/scsiback.c >> +++ linux-2.6.18-xen.hg/drivers/xen/scsiback/scsiback.c >> @@ -279,22 +279,14 @@ static int scsiback_gnttab_data_map(vscs >> >> err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, >> nr_segments); >> BUG_ON(err); >> - /* Retry maps with GNTST_eagain */ >> - for(i=0; i < nr_segments; i++) { >> - while(unlikely(map[i].status == GNTST_eagain)) >> - { >> - msleep(10); >> - err = >> HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> - &map[i], >> - 1); >> - BUG_ON(err); >> - } >> - } >> >> for (i = 0; i < nr_segments; i++) { >> struct page *pg; >> >> - if (unlikely(map[i].status != 0)) { >> + /* Retry maps with GNTST_eagain */ >> + if (unlikely(map[i].status == GNTST_eagain)) >> + >> gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); >> + if (unlikely(map[i].status != GNTST_okay)) { >> printk(KERN_ERR "scsiback: invalid buffer -- >> could not remap it\n"); >> map[i].handle = SCSIBACK_INVALID_HANDLE; >> err |= 1; >> --- linux-2.6.18-xen.hg.orig/drivers/xen/sfc_netutil/accel_util.c >> +++ linux-2.6.18-xen.hg/drivers/xen/sfc_netutil/accel_util.c >> @@ -71,24 +71,27 @@ static int net_accel_map_grant(struct xe >> u64 *dev_bus_addr, unsigned flags) >> { >> struct gnttab_map_grant_ref op; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)vaddr, flags, >> gnt_ref, dev->otherend_id); >> >> - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> if (op.status != GNTST_okay) { >> xenbus_dev_error >> (dev, op.status, >> "failed mapping in shared page %d from domain %d\n", >> gnt_ref, dev->otherend_id); >> + ret = -EINVAL; >> } else { >> *handle = op.handle; >> if (dev_bus_addr) >> *dev_bus_addr = op.dev_bus_addr; >> + ret = 0; >> } >> >> - return op.status; >> + return ret; >> } >> >> >> @@ -112,7 +115,7 @@ static int net_accel_unmap_grant(struct >> "failed unmapping page at handle %d error >> %d\n", >> handle, op.status); >> >> - return op.status; >> + return op.status == GNTST_okay ? 0 : -EINVAL; >> } >> >> >> @@ -144,7 +147,7 @@ struct net_accel_valloc_grant_mapping { >> /* Map a series of grants into a contiguous virtual area */ >> static void *net_accel_map_grants_valloc(struct xenbus_device *dev, >> unsigned *grants, int npages, >> - unsigned flags, void **priv, int >> *errno) >> + unsigned flags, void **priv) >> { >> struct net_accel_valloc_grant_mapping *map; >> struct vm_struct *vm; >> @@ -172,16 +175,12 @@ static void *net_accel_map_grants_valloc >> >> /* Do the actual mapping */ >> addr = vm->addr; >> - if(errno != NULL) *errno = 0; >> + >> for (i = 0; i < npages; i++) { >> rc = net_accel_map_grant(dev, grants[i], map->grant_handles + >> i, >> addr, NULL, flags); >> - if (rc != 0) >> - { >> - if(errno != NULL) >> - *errno = (rc == GNTST_eagain ? -EAGAIN : -EINVAL); >> + if (rc < 0) >> goto undo; >> - } >> addr = (void*)((unsigned long)addr + PAGE_SIZE); >> } >> >> @@ -230,16 +229,7 @@ void *net_accel_map_grants_contig(struct >> unsigned *grants, int npages, >> void **priv) >> { >> - int errno; >> - void *ret; >> - >> - do { >> - ret = net_accel_map_grants_valloc(dev, grants, npages, >> - GNTMAP_host_map, priv, &errno); >> - if(errno) msleep(10); >> - } while(errno == -EAGAIN); >> - >> - return ret; >> + return net_accel_map_grants_valloc(dev, grants, npages, GNTMAP_host_map, >> priv); >> } >> EXPORT_SYMBOL(net_accel_map_grants_contig); >> >> @@ -255,16 +245,7 @@ EXPORT_SYMBOL(net_accel_unmap_grants_con >> void *net_accel_map_iomem_page(struct xenbus_device *dev, int gnt_ref, >> void **priv) >> { >> - int errno; >> - void *ret; >> - >> - do { >> - ret = net_accel_map_grants_valloc(dev, &gnt_ref, 1, >> - GNTMAP_host_map, priv, &errno); >> - if(errno) msleep(10); >> - } while(errno == -EAGAIN); >> - >> - return ret; >> + return net_accel_map_grants_valloc(dev, &gnt_ref, 1, GNTMAP_host_map, >> priv); >> } >> EXPORT_SYMBOL(net_accel_map_iomem_page); >> >> --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/interface.c >> @@ -81,25 +81,23 @@ tpmif_t *tpmif_find(domid_t domid, struc >> static int map_frontend_page(tpmif_t *tpmif, unsigned long shared_page) >> { >> struct gnttab_map_grant_ref op; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr, >> GNTMAP_host_map, shared_page, tpmif->domid); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - if (op.status) { >> - DPRINTK(" Grant table operation failure !\n"); >> - return op.status; >> + if (op.status != GNTST_okay) { >> + DPRINTK(" Grant table operation failure %d!\n", >> (int)op.status); >> + ret = -EINVAL; >> + } else { >> + tpmif->shmem_ref = shared_page; >> + tpmif->shmem_handle = op.handle; >> + ret = 0; >> } >> >> - tpmif->shmem_ref = shared_page; >> - tpmif->shmem_handle = op.handle; >> - >> - return 0; >> + return ret; >> } >> >> static void unmap_frontend_page(tpmif_t *tpmif) >> --- linux-2.6.18-xen.hg.orig/drivers/xen/tpmback/tpmback.c >> +++ linux-2.6.18-xen.hg/drivers/xen/tpmback/tpmback.c >> @@ -257,20 +257,15 @@ int _packet_write(struct packet *pak, >> gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), >> GNTMAP_host_map, tx->ref, tpmif->domid); >> >> - do { >> - if >> (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> - &map_op, 1))) >> - BUG(); >> - if(map_op.status) msleep(10); >> - } while(map_op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, >> &map_op); >> >> - handle = map_op.handle; >> - >> - if (map_op.status) { >> + if (map_op.status != GNTST_okay) { >> DPRINTK(" Grant table operation failure !\n"); >> return 0; >> } >> >> + handle = map_op.handle; >> + >> tocopy = min_t(size_t, size - offset, PAGE_SIZE); >> >> if (copy_from_buffer((void *)(idx_to_kaddr(tpmif, i) | >> @@ -397,14 +392,9 @@ static int packet_read_shmem(struct pack >> gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i), >> GNTMAP_host_map, tx->ref, tpmif->domid); >> >> - do { >> - if >> (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> - &map_op, 1))) >> - BUG(); >> - if(map_op.status) msleep(10); >> - } while(map_op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, >> &map_op); >> >> - if (map_op.status) { >> + if (map_op.status != GNTST_okay) { >> DPRINTK(" Grant table operation failure !\n"); >> return -EFAULT; >> } >> --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/interface.c >> +++ linux-2.6.18-xen.hg/drivers/xen/usbback/interface.c >> @@ -110,16 +110,11 @@ static int map_frontend_pages(usbif_t *u >> gnttab_set_map_op(&op, (unsigned long)usbif->urb_ring_area->addr, >> GNTMAP_host_map, urb_ring_ref, usbif->domid); >> >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while (op.status == GNTST_eagain); >> - >> - if (op.status) { >> - printk(KERN_ERR "grant table failure mapping >> urb_ring_ref\n"); >> - return op.status; >> + if (op.status != GNTST_okay) { >> + printk(KERN_ERR "grant table failure mapping urb_ring_ref >> %d\n", (int)op.status); >> + return -EINVAL; >> } >> >> usbif->urb_shmem_ref = urb_ring_ref; >> @@ -128,21 +123,17 @@ static int map_frontend_pages(usbif_t *u >> gnttab_set_map_op(&op, (unsigned long)usbif->conn_ring_area->addr, >> GNTMAP_host_map, conn_ring_ref, usbif->domid); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while (op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> - if (op.status) { >> + if (op.status != GNTST_okay) { >> struct gnttab_unmap_grant_ref unop; >> gnttab_set_unmap_op(&unop, >> (unsigned long) usbif->urb_ring_area->addr, >> GNTMAP_host_map, usbif->urb_shmem_handle); >> VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, >> &unop, >> 1)); >> - printk(KERN_ERR "grant table failure mapping >> conn_ring_ref\n"); >> - return op.status; >> + printk(KERN_ERR "grant table failure mapping conn_ring_ref >> %d\n", (int)op.status); >> + return -EINVAL; >> } >> >> usbif->conn_shmem_ref = conn_ring_ref; >> --- linux-2.6.18-xen.hg.orig/drivers/xen/usbback/usbback.c >> +++ linux-2.6.18-xen.hg/drivers/xen/usbback/usbback.c >> @@ -392,19 +392,13 @@ static int usbbk_gnttab_map(usbif_t *usb >> ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, >> map, nr_segs); >> BUG_ON(ret); >> - /* Make sure than none of the map ops failed with GNTST_eagain */ >> - for( i = 0; i < nr_segs; i++) { >> - while(map[i].status == GNTST_eagain) { >> - msleep(10); >> - ret = HYPERVISOR_grant_table_op( >> - GNTTABOP_map_grant_ref, >> - &map[i], 1); >> - BUG_ON(ret); >> - } >> - } >> >> for (i = 0; i < nr_segs; i++) { >> - if (unlikely(map[i].status != 0)) { >> + /* Make sure than none of the map ops failed with >> GNTST_eagain */ >> + if (unlikely(map[i].status == GNTST_eagain)) >> + >> gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]); >> + >> + if (unlikely(map[i].status != GNTST_okay)) { >> printk(KERN_ERR "usbback: invalid buffer -- >> could not remap it\n"); >> map[i].handle = USBBACK_INVALID_HANDLE; >> ret |= 1; >> --- linux-2.6.18-xen.hg.orig/drivers/xen/xenbus/xenbus_backend_client.c >> +++ linux-2.6.18-xen.hg/drivers/xen/xenbus/xenbus_backend_client.c >> @@ -49,11 +49,7 @@ struct vm_struct *xenbus_map_ring_valloc >> gnttab_set_map_op(&op, (unsigned long)area->addr, GNTMAP_host_map, >> gnt_ref, dev->otherend_id); >> >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while(op.status == GNTST_eagain); >> + gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op); >> >> if (op.status != GNTST_okay) { >> free_vm_area(area); >> @@ -61,7 +57,7 @@ struct vm_struct *xenbus_map_ring_valloc >> "mapping in shared page %d from domain %d", >> gnt_ref, dev->otherend_id); >> BUG_ON(!IS_ERR(ERR_PTR(op.status))); >> - return ERR_PTR(op.status); >> + return ERR_PTR(-EINVAL); >> } >> >> /* Stuff the handle in an unused field */ >> @@ -76,23 +72,24 @@ int xenbus_map_ring(struct xenbus_device >> grant_handle_t *handle, void *vaddr) >> { >> struct gnttab_map_grant_ref op; >> + int ret; >> >> gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map, >> gnt_ref, dev->otherend_id); >> - do { >> - if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)) >> - BUG(); >> - msleep(10); >> - } while(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", >> gnt_ref, dev->otherend_id); >> - } else >> + ret = -EINVAL; >> + } else { >> *handle = op.handle; >> + ret = 0; >> + } >> >> - return op.status; >> + return ret; >> } >> EXPORT_SYMBOL_GPL(xenbus_map_ring); >> >> @@ -115,7 +112,7 @@ int xenbus_unmap_ring_vfree(struct xenbu >> "unmapping page at handle %d error %d", >> (int16_t)area->phys_addr, op.status); >> >> - return op.status; >> + return op.status == GNTST_okay ? 0 : -EINVAL; >> } >> EXPORT_SYMBOL_GPL(xenbus_unmap_ring_vfree); >> >> @@ -135,7 +132,7 @@ int xenbus_unmap_ring(struct xenbus_devi >> "unmapping page at handle %d error %d", >> handle, op.status); >> >> - return op.status; >> + return op.status == GNTST_okay ? 0 : -EINVAL; >> } >> EXPORT_SYMBOL_GPL(xenbus_unmap_ring); >> >> --- linux-2.6.18-xen.hg.orig/include/xen/gnttab.h >> +++ linux-2.6.18-xen.hg/include/xen/gnttab.h >> @@ -40,6 +40,7 @@ >> #include <asm/hypervisor.h> >> #include <asm/maddr.h> /* maddr_t */ >> #include <linux/mm.h> >> +#include <linux/delay.h> >> #include <xen/interface/grant_table.h> >> #include <xen/features.h> >> >> @@ -161,4 +162,41 @@ gnttab_set_replace_op(struct gnttab_unma >> unmap->handle = handle; >> } >> >> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) >> \ >> +{ >> \ >> + 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: %s gnt busy\n", __func__, >> current->comm); \ >> + (__HCarg_p)->status = GNTST_bad_page; >> \ >> + } >> \ >> + if ((__HCarg_p)->status != GNTST_okay) >> \ >> + printk(KERN_ERR "%s: %s gnt status %x\n", >> \ >> + __func__, current->comm, (__HCarg_p)->status); >> \ >> +} >> + >> +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p) >> \ >> +{ >> \ >> + 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: %s gnt busy\n", __func__, >> current->comm); \ >> + (__HCarg_p)->status = GNTST_bad_page; >> \ >> + } >> \ >> + if ((__HCarg_p)->status != GNTST_okay) >> \ >> + printk(KERN_ERR "%s: %s gnt status %x\n", >> \ >> + __func__, current->comm, (__HCarg_p)->status); >> \ >> +} >> + >> #endif /* __ASM_GNTTAB_H__ */ >> >> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] xenpaging: handle GNTST_eagain in kernel drivers 2010-09-16 3:44 ` Patrick Colp 2010-09-16 9:05 ` Keir Fraser @ 2010-09-16 13:33 ` Olaf Hering 1 sibling, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-16 13:33 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel On Wed, Sep 15, Patrick Colp wrote: > Is there any particular reason for 33 seconds? This number seems a bit > high to me. Did you run into situations where that amount of time was > needed? Otherwise I'm happy with everything. Thanks for all the hard > work! Its just when the wrap of u8 happens, it keeps the code simple. In my testing it was never over 100 loop iterations. Most of the time its between 1 and 10. Olaf ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: page-in granttable entries 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering 2010-09-15 16:08 ` [PATCH] xenpaging: handle GNTST_eagain in kernel drivers Olaf Hering @ 2010-09-15 16:10 ` Olaf Hering 2010-09-15 16:11 ` [PATCH] xenpaging: fix crash in notify_via_xen_event_channel Olaf Hering ` (5 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-15 16:10 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel When converting a gfn to mfn, check if the page is paged-out. If it is, request a page-in and return GNTST_eagain to the caller to indicate a retry of the hypercall is required. This fixes granttable errors when xenpaging is enabled in the guest. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- xen/common/grant_table.c | 97 ++++++++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 38 deletions(-) --- xen-unstable.hg-4.1.22132.orig/xen/common/grant_table.c +++ xen-unstable.hg-4.1.22132/xen/common/grant_table.c @@ -139,6 +139,34 @@ shared_entry_header(struct grant_table * #define active_entry(t, e) \ ((t)->active[(e)/ACGNT_PER_PAGE][(e)%ACGNT_PER_PAGE]) +/* Check if the page has been paged out */ +static int __get_paged_frame(unsigned long gfn, unsigned long *frame, int readonly, struct domain *rd) +{ + struct p2m_domain *p2m; + p2m_type_t p2mt; + mfn_t mfn; + int rc = GNTST_okay; + + p2m = p2m_get_hostp2m(rd); + if ( readonly ) + mfn = gfn_to_mfn(p2m, gfn, &p2mt); + else + mfn = gfn_to_mfn_unshare(p2m, gfn, &p2mt, 1); + + if ( p2m_is_valid(p2mt) ) { + *frame = mfn_x(mfn); + if ( p2m_is_paged(p2mt) ) + p2m_mem_paging_populate(p2m, gfn); + if ( p2m_is_paging(p2mt) ) + rc = GNTST_eagain; + } else { + *frame = INVALID_MFN; + rc = GNTST_bad_page; + } + + return rc; +} + static inline int __get_maptrack_handle( struct grant_table *t) @@ -527,14 +555,16 @@ __gnttab_map_grant_ref( if ( !act->pin ) { + unsigned long gfn; + unsigned long frame; + + gfn = sha1 ? sha1->frame : sha2->full_page.frame; + rc = __get_paged_frame(gfn, &frame, !!(op->flags & GNTMAP_readonly), rd); + if ( rc != GNTST_okay ) + goto unlock_out; + act->gfn = gfn; act->domid = ld->domain_id; - if ( sha1 ) - act->gfn = sha1->frame; - else - act->gfn = sha2->full_page.frame; - act->frame = (op->flags & GNTMAP_readonly) ? - gmfn_to_mfn(rd, act->gfn) : - gfn_to_mfn_private(rd, act->gfn); + act->frame = frame; act->start = 0; act->length = PAGE_SIZE; act->is_sub_page = 0; @@ -1697,6 +1727,7 @@ __acquire_grant_for_copy( domid_t trans_domid; grant_ref_t trans_gref; struct domain *rrd; + unsigned long gfn; unsigned long grant_frame; unsigned trans_page_off; unsigned trans_length; @@ -1814,9 +1845,11 @@ __acquire_grant_for_copy( } else if ( sha1 ) { - act->gfn = sha1->frame; - grant_frame = readonly ? gmfn_to_mfn(rd, act->gfn) : - gfn_to_mfn_private(rd, act->gfn); + gfn = sha1->frame; + rc = __get_paged_frame(gfn, &grant_frame, readonly, rd); + if ( rc != GNTST_okay ) + goto unlock_out; + act->gfn = gfn; is_sub_page = 0; trans_page_off = 0; trans_length = PAGE_SIZE; @@ -1824,9 +1857,11 @@ __acquire_grant_for_copy( } else if ( !(sha2->hdr.flags & GTF_sub_page) ) { - act->gfn = sha2->full_page.frame; - grant_frame = readonly ? gmfn_to_mfn(rd, act->gfn) : - gfn_to_mfn_private(rd, act->gfn); + gfn = sha2->full_page.frame; + rc = __get_paged_frame(gfn, &grant_frame, readonly, rd); + if ( rc != GNTST_okay ) + goto unlock_out; + act->gfn = gfn; is_sub_page = 0; trans_page_off = 0; trans_length = PAGE_SIZE; @@ -1834,9 +1869,11 @@ __acquire_grant_for_copy( } else { - act->gfn = sha2->sub_page.frame; - grant_frame = readonly ? gmfn_to_mfn(rd, act->gfn) : - gfn_to_mfn_private(rd, act->gfn); + gfn = sha2->sub_page.frame; + rc = __get_paged_frame(gfn, &grant_frame, readonly, rd); + if ( rc != GNTST_okay ) + goto unlock_out; + act->gfn = gfn; is_sub_page = 1; trans_page_off = sha2->sub_page.page_off; trans_length = sha2->sub_page.length; @@ -1932,17 +1969,9 @@ __gnttab_copy( else { #ifdef CONFIG_X86 - p2m_type_t p2mt; - struct p2m_domain *p2m = p2m_get_hostp2m(sd); - s_frame = mfn_x(gfn_to_mfn(p2m, op->source.u.gmfn, &p2mt)); - if ( !p2m_is_valid(p2mt) ) - s_frame = INVALID_MFN; - if ( p2m_is_paging(p2mt) ) - { - p2m_mem_paging_populate(p2m, op->source.u.gmfn); - rc = -ENOENT; - goto error_out; - } + rc = __get_paged_frame(op->source.u.gmfn, &s_frame, 1, sd); + if ( rc != GNTST_okay ) + goto error_out; #else s_frame = gmfn_to_mfn(sd, op->source.u.gmfn); #endif @@ -1979,17 +2008,9 @@ __gnttab_copy( else { #ifdef CONFIG_X86 - p2m_type_t p2mt; - struct p2m_domain *p2m = p2m_get_hostp2m(dd); - d_frame = mfn_x(gfn_to_mfn_unshare(p2m, op->dest.u.gmfn, &p2mt, 1)); - if ( !p2m_is_valid(p2mt) ) - d_frame = INVALID_MFN; - if ( p2m_is_paging(p2mt) ) - { - p2m_mem_paging_populate(p2m, op->dest.u.gmfn); - rc = -ENOENT; - goto error_out; - } + rc = __get_paged_frame(op->dest.u.gmfn, &d_frame, 0, dd); + if ( rc != GNTST_okay ) + goto error_out; #else d_frame = gmfn_to_mfn(dd, op->dest.u.gmfn); #endif ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: fix crash in notify_via_xen_event_channel 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering 2010-09-15 16:08 ` [PATCH] xenpaging: handle GNTST_eagain in kernel drivers Olaf Hering 2010-09-15 16:10 ` [PATCH] xenpaging: page-in granttable entries Olaf Hering @ 2010-09-15 16:11 ` Olaf Hering 2010-09-15 16:12 ` [PATCH] xenpaging: avoid empty pagefile on unsupported hardware Olaf Hering ` (4 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-15 16:11 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel Avoid crash of Xen if a xenpaging enabled guest crashes and there are still memevents in flight, as reported here: http://lists.xensource.com/archives/html/xen-devel/2010-08/msg00516.html Signed-off-by: Olaf Hering <olaf@aepfle.de> --- xen/common/event_channel.c | 6 ++++++ 1 file changed, 6 insertions(+) --- xen-unstable.hg-4.1.22132.orig/xen/common/event_channel.c +++ xen-unstable.hg-4.1.22132/xen/common/event_channel.c @@ -1030,6 +1030,12 @@ void notify_via_xen_event_channel(struct spin_lock(&ld->event_lock); + if ( unlikely(ld->is_dying) ) + { + spin_unlock(&ld->event_lock); + return; + } + ASSERT(port_is_valid(ld, lport)); lchn = evtchn_from_port(ld, lport); ASSERT(lchn->consumer_is_xen); ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: avoid empty pagefile on unsupported hardware 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering ` (2 preceding siblings ...) 2010-09-15 16:11 ` [PATCH] xenpaging: fix crash in notify_via_xen_event_channel Olaf Hering @ 2010-09-15 16:12 ` Olaf Hering 2010-09-22 15:38 ` [PATCH] xenpaging: handle XENMEM_decrease_reservation Olaf Hering ` (3 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-15 16:12 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel Open paging file only if xenpaging_init() succeeds. It can fail if the host does not support the required virtualization features such as EPT. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/xenpaging/xenpaging.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) --- xen-unstable.hg-4.1.22101.orig/tools/xenpaging/xenpaging.c +++ xen-unstable.hg-4.1.22101/tools/xenpaging/xenpaging.c @@ -501,15 +501,6 @@ int main(int argc, char *argv[]) victims = calloc(num_pages, sizeof(xenpaging_victim_t)); - /* Open file */ - sprintf(filename, "page_cache_%d", domain_id); - fd = open(filename, open_flags, open_mode); - if ( fd < 0 ) - { - perror("failed to open file"); - return -1; - } - /* Seed random-number generator */ srand(time(NULL)); @@ -521,6 +512,15 @@ int main(int argc, char *argv[]) goto out; } + /* Open file */ + sprintf(filename, "page_cache_%d", domain_id); + fd = open(filename, open_flags, open_mode); + if ( fd < 0 ) + { + perror("failed to open file"); + return -1; + } + /* Evict pages */ memset(victims, 0, sizeof(xenpaging_victim_t) * num_pages); for ( i = 0; i < num_pages; i++ ) ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: handle XENMEM_decrease_reservation 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering ` (3 preceding siblings ...) 2010-09-15 16:12 ` [PATCH] xenpaging: avoid empty pagefile on unsupported hardware Olaf Hering @ 2010-09-22 15:38 ` Olaf Hering 2010-09-22 15:40 ` [PATCH] xenpaging: break endless loop during inital page-out with large pagefiles Olaf Hering ` (2 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-22 15:38 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel Handle paged-out pages for XENMEM_decrease_reservation memory op. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/libxc/xc_domain.c | 38 ++++++++++++++++++++++++++------------ xen/common/memory.c | 4 ++++ 2 files changed, 30 insertions(+), 12 deletions(-) --- xen-unstable.hg-4.1.22155.orig/tools/libxc/xc_domain.c +++ xen-unstable.hg-4.1.22155/tools/libxc/xc_domain.c @@ -620,15 +620,15 @@ int xc_domain_memory_decrease_reservatio xen_pfn_t *extent_start) { int err; + unsigned long delay = 0; + unsigned long start = 0; + unsigned long count= nr_extents; struct xen_memory_reservation reservation = { - .nr_extents = nr_extents, .extent_order = extent_order, .mem_flags = 0, .domid = domid }; - set_xen_guest_handle(reservation.extent_start, extent_start); - if ( extent_start == NULL ) { DPRINTF("decrease_reservation extent_start is NULL!\n"); @@ -636,16 +636,30 @@ int xc_domain_memory_decrease_reservatio return -1; } - err = xc_memory_op(xch, XENMEM_decrease_reservation, &reservation); - if ( err == nr_extents ) - return 0; + while (1) { + set_xen_guest_handle(reservation.extent_start, extent_start + start); + reservation.nr_extents = count; - if ( err >= 0 ) - { - DPRINTF("Failed deallocation for dom %d: %ld extents of order %d\n", - domid, nr_extents, extent_order); - errno = EINVAL; - err = -1; + err = xc_memory_op(xch, XENMEM_decrease_reservation, &reservation); + if ( err == count ) + { + err = 0; + break; + } + + if ( err > count || err <= 0 || delay > 1000 * 1000) + { + DPRINTF("Failed deallocation for dom %d: %ld extents of order %d: 0x%x\n", + domid, nr_extents, extent_order, err); + errno = EINVAL; + err = -1; + break; + } + + start += err; + count -= err; + usleep(delay); + delay += 666; /* 1500 iterations, 12 seconds */ } return err; --- xen-unstable.hg-4.1.22155.orig/xen/common/memory.c +++ xen-unstable.hg-4.1.22155/xen/common/memory.c @@ -162,6 +162,10 @@ int guest_remove_page(struct domain *d, #ifdef CONFIG_X86 mfn = mfn_x(gfn_to_mfn(p2m_get_hostp2m(d), gmfn, &p2mt)); + if ( p2m_is_paged(p2mt) ) + p2m_mem_paging_populate(p2m_get_hostp2m(d), gmfn); + if ( p2m_is_paging(p2mt) ) + return -ENOENT; #else mfn = gmfn_to_mfn(d, gmfn); #endif ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: break endless loop during inital page-out with large pagefiles 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering ` (4 preceding siblings ...) 2010-09-22 15:38 ` [PATCH] xenpaging: handle XENMEM_decrease_reservation Olaf Hering @ 2010-09-22 15:40 ` Olaf Hering 2010-09-22 15:41 ` [PATCH] xenpaging: allow only one xenpaging call per guest Olaf Hering 2010-09-22 15:48 ` xenpaging fixes for kernel and hypervisor Olaf Hering 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-22 15:40 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel To allow the starting for xenpaging right after 'xm start XYZ', I specified a pagefile size equal to the guest memory size in the hope to catch more errors where the paged-out state of a p2mt is not checked. While doing that, xenpaging got into an endless loop because some pages cant be paged out right away. Now the policy reports an error if the gfn number wraps. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/xenpaging/policy_default.c | 30 ++++++++++++++++++++++++------ tools/xenpaging/xenpaging.c | 7 +++++-- 2 files changed, 29 insertions(+), 8 deletions(-) --- xen-unstable.hg-4.1.22155.orig/tools/xenpaging/policy_default.c +++ xen-unstable.hg-4.1.22155/tools/xenpaging/policy_default.c @@ -32,6 +32,10 @@ static unsigned long mru[MRU_SIZE]; static unsigned int i_mru = 0; static unsigned long *bitmap; +static unsigned long *unconsumed; +static unsigned long current_gfn; +static unsigned long bitmap_size; +static unsigned long max_pages; int policy_init(xenpaging_t *paging) @@ -43,6 +47,14 @@ int policy_init(xenpaging_t *paging) rc = alloc_bitmap(&bitmap, paging->bitmap_size); if ( rc != 0 ) goto out; + /* Allocate bitmap to track unusable pages */ + rc = alloc_bitmap(&unconsumed, paging->bitmap_size); + if ( rc != 0 ) + goto out; + + /* record bitmap_size */ + bitmap_size = paging->bitmap_size; + max_pages = paging->domain_info->max_pages; /* Initialise MRU list of paged in pages */ for ( i = 0; i < MRU_SIZE; i++ ) @@ -51,8 +63,6 @@ int policy_init(xenpaging_t *paging) /* Don't page out page 0 */ set_bit(0, bitmap); - rc = 0; - out: return rc; } @@ -61,17 +71,24 @@ int policy_choose_victim(xc_interface *x xenpaging_t *paging, domid_t domain_id, xenpaging_victim_t *victim) { + unsigned long wrap = current_gfn; ASSERT(victim != NULL); /* Domain to pick on */ victim->domain_id = domain_id; - + do { - /* Randomly choose a gfn to evict */ - victim->gfn = rand() % paging->domain_info->max_pages; + current_gfn++; + if ( current_gfn >= max_pages ) + current_gfn = 0; + if ( wrap == current_gfn ) + return -ENOSPC; } - while ( test_bit(victim->gfn, bitmap) ); + while ( test_bit(current_gfn, bitmap) || test_bit(current_gfn, unconsumed) ); + + set_bit(current_gfn, unconsumed); + victim->gfn = current_gfn; return 0; } @@ -79,6 +96,7 @@ int policy_choose_victim(xc_interface *x void policy_notify_paged_out(domid_t domain_id, unsigned long gfn) { set_bit(gfn, bitmap); + clear_bit(gfn, unconsumed); } void policy_notify_paged_in(domid_t domain_id, unsigned long gfn) --- xen-unstable.hg-4.1.22155.orig/tools/xenpaging/xenpaging.c +++ xen-unstable.hg-4.1.22155/tools/xenpaging/xenpaging.c @@ -446,7 +446,8 @@ static int evict_victim(xc_interface *xc ret = policy_choose_victim(xch, paging, domain_id, victim); if ( ret != 0 ) { - ERROR("Error choosing victim"); + if ( ret != -ENOSPC ) + ERROR("Error choosing victim"); goto out; } @@ -525,7 +526,9 @@ int main(int argc, char *argv[]) memset(victims, 0, sizeof(xenpaging_victim_t) * num_pages); for ( i = 0; i < num_pages; i++ ) { - evict_victim(xch, paging, domain_id, &victims[i], fd, i); + rc = evict_victim(xch, paging, domain_id, &victims[i], fd, i); + if ( rc == -ENOSPC ) + break; if ( i % 100 == 0 ) DPRINTF("%d pages evicted\n", i); } ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xenpaging: allow only one xenpaging call per guest 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering ` (5 preceding siblings ...) 2010-09-22 15:40 ` [PATCH] xenpaging: break endless loop during inital page-out with large pagefiles Olaf Hering @ 2010-09-22 15:41 ` Olaf Hering 2010-09-22 15:48 ` xenpaging fixes for kernel and hypervisor Olaf Hering 7 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-22 15:41 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel Make sure only one xenpaging binary is active per domain. Print info when the host lacks the required features for xenpaging. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/xenpaging/xenpaging.c | 12 +++++++++++- xen/arch/x86/mm/mem_event.c | 7 +++++++ 2 files changed, 18 insertions(+), 1 deletion(-) --- xen-unstable.hg-4.1.22155.orig/tools/xenpaging/xenpaging.c +++ xen-unstable.hg-4.1.22155/tools/xenpaging/xenpaging.c @@ -123,7 +123,17 @@ xenpaging_t *xenpaging_init(xc_interface paging->mem_event.ring_page); if ( rc != 0 ) { - ERROR("Error initialising shared page"); + switch ( errno ) { + case EBUSY: + ERROR("xenpaging is (or was) active on this domain"); + break; + case ENODEV: + ERROR("EPT not supported for this guest"); + break; + default: + ERROR("Error initialising shared page"); + break; + } goto err; } --- xen-unstable.hg-4.1.22155.orig/xen/arch/x86/mm/mem_event.c +++ xen-unstable.hg-4.1.22155/xen/arch/x86/mm/mem_event.c @@ -226,6 +226,13 @@ int mem_event_domctl(struct domain *d, x mfn_t ring_mfn; mfn_t shared_mfn; + /* Only one xenpaging at a time. If xenpaging crashed, + * the cache is in an undefined state and so is the guest + */ + rc = -EBUSY; + if ( d->mem_event.enabled ) + break; + /* Currently only EPT is supported */ rc = -ENODEV; if ( !(hap_enabled(d) && ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: xenpaging fixes for kernel and hypervisor 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering ` (6 preceding siblings ...) 2010-09-22 15:41 ` [PATCH] xenpaging: allow only one xenpaging call per guest Olaf Hering @ 2010-09-22 15:48 ` Olaf Hering 2010-09-22 16:49 ` Patrick Colp 7 siblings, 1 reply; 16+ messages in thread From: Olaf Hering @ 2010-09-22 15:48 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel Patrick, there are three more changes to make xenpaging more robust. Do you need to ack each one to get them merged in xen-unstable? Should any of these changes go also into the xen-4.0-testing tree for the 4.0.2 release? If so, I will prepare the patches for this branch. One more thing: In an earlier mail you mentioned that realmode support is not there yet. However, in my testing I can run grub and the bios and even boot into Linux a bit. So it appears there is realmode support, perhaps still incomplete because the guest crashes in (appearently) early Linux init functions: (XEN) realmode.c:115:d1 Failed to emulate insn. (XEN) realmode.c:165:d1 Real-mode emulation failed @ 9000:0000f81a: 0f 00 00 00 00 00 (XEN) domain_crash called from realmode.c:166 (XEN) Domain 1 (vcpu#0) crashed on cpu#0: (XEN) ----[ Xen-4.0.1_21326_01-20100922.141534 x86_64 debug=y Tainted: C ]---- (XEN) CPU: 0 (XEN) RIP: 9000:[<000000000000f81a>] (XEN) RFLAGS: 0000000000000246 CONTEXT: hvm guest (XEN) rax: 0000000000000000 rbx: 0000000000008fb8 rcx: 0000000000000000 (XEN) rdx: 0000000000009000 rsi: 0000000000000008 rdi: 0000000000099fff (XEN) rbp: 000000000000ffff rsp: 0000000000001ff2 r8: 0000000000000000 (XEN) r9: 0000000000000000 r10: 0000000000000000 r11: 0000000000000000 (XEN) r12: 0000000000000000 r13: 0000000000000000 r14: 0000000000000000 (XEN) r15: 0000000000000000 cr0: 0000000000000010 cr4: 0000000000000000 (XEN) cr3: 0000000000000000 cr2: 0000000000000000 (XEN) ds: 0000 es: 9000 fs: 9000 gs: 9000 ss: 9000 cs: 9000 (XEN) cpupool_rm_domain(dom=1,pool=0) n_dom 1 [ 127.709423] br0: port 2(vif1.0) entering disabled state [ 127.732999] br0: port 2(vif1.0) entering disabled state I'm poking around and adding debug to the gfn_to_mfn* functions, but none triggers. Where should I start looking for this kind of bug? Olaf ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: xenpaging fixes for kernel and hypervisor 2010-09-22 15:48 ` xenpaging fixes for kernel and hypervisor Olaf Hering @ 2010-09-22 16:49 ` Patrick Colp 2010-09-22 17:13 ` Keir Fraser ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Patrick Colp @ 2010-09-22 16:49 UTC (permalink / raw) To: Olaf Hering; +Cc: xen-devel I don't know if I need to ack or not, but I will: Acked-by: Patrick Colp <pjcolp@cs.ubc.ca> I think the issue with realmode is in the emulation code. Looking at where that crash occurs, it's a result of hvm_emulate_one() returning X86EMUL_UNHANDLEABLE. hvm_emulate_one() calls x86_emulate(), and for supported functions, that will call hvmemul_* using the hvm_emulate_ops pointer function struct. However, my guess is that what's causing this problem is an instruction that isn't handled by that stuff (the hvmemul_* stuff). The way I'd probably go about this is to try to find out what instruction it's trying to emulate when it fails, then to look in the x86_emulate() code and see where/if that instruction is handled. Once that's determined, then you should be able to find out why it's returning X86EMUL_UNHANDLEABLE. Either it's because x86_emulate() doesn't handle it at all (this only seems likely if the paging code has forced the realmode code down a different path, like trying to handle a page fault or something) or the code to handle it assumes that the memory is present in the guest (which was a fair assumption before paging and was a common problem when I was plumbing the rest of the paging code through the emulator). In this case, a check needs to be made to see if the memory the instruction is trying to access is paged out, and if so the result propagated back and everything plumbed through. The convention when emulating and detecting a paged out page is to call p2m_paging_populate and return X86EMUL_RETRY. This should cause the guest/emulate code to just keep retrying the instruction until it succeeds (once the page has been paged back in). There's a chance that the problem occurs when trying to fetch an instruction if that instruction lives on a page that's paged out. I know this case was handled with the regular hvmemul_* code, but not sure if it becomes an issue again with realmode. If you know the instruction or can send me whatever setup you use to cause this bug, then I can help track it down. It sounds from your other e-mails like you've just modified "xm create" to page everything out right away? Patrick On 22 September 2010 08:48, Olaf Hering <olaf@aepfle.de> wrote: > > Patrick, > > there are three more changes to make xenpaging more robust. > Do you need to ack each one to get them merged in xen-unstable? > Should any of these changes go also into the xen-4.0-testing tree for > the 4.0.2 release? If so, I will prepare the patches for this branch. > > > One more thing: In an earlier mail you mentioned that realmode support > is not there yet. However, in my testing I can run grub and the bios and > even boot into Linux a bit. So it appears there is realmode support, > perhaps still incomplete because the guest crashes in (appearently) > early Linux init functions: > > (XEN) realmode.c:115:d1 Failed to emulate insn. > (XEN) realmode.c:165:d1 Real-mode emulation failed @ 9000:0000f81a: 0f 00 00 00 00 00 > (XEN) domain_crash called from realmode.c:166 > (XEN) Domain 1 (vcpu#0) crashed on cpu#0: > (XEN) ----[ Xen-4.0.1_21326_01-20100922.141534 x86_64 debug=y Tainted: C ]---- > (XEN) CPU: 0 > (XEN) RIP: 9000:[<000000000000f81a>] > (XEN) RFLAGS: 0000000000000246 CONTEXT: hvm guest > (XEN) rax: 0000000000000000 rbx: 0000000000008fb8 rcx: 0000000000000000 > (XEN) rdx: 0000000000009000 rsi: 0000000000000008 rdi: 0000000000099fff > (XEN) rbp: 000000000000ffff rsp: 0000000000001ff2 r8: 0000000000000000 > (XEN) r9: 0000000000000000 r10: 0000000000000000 r11: 0000000000000000 > (XEN) r12: 0000000000000000 r13: 0000000000000000 r14: 0000000000000000 > (XEN) r15: 0000000000000000 cr0: 0000000000000010 cr4: 0000000000000000 > (XEN) cr3: 0000000000000000 cr2: 0000000000000000 > (XEN) ds: 0000 es: 9000 fs: 9000 gs: 9000 ss: 9000 cs: 9000 > (XEN) cpupool_rm_domain(dom=1,pool=0) n_dom 1 > [ 127.709423] br0: port 2(vif1.0) entering disabled state > [ 127.732999] br0: port 2(vif1.0) entering disabled state > > I'm poking around and adding debug to the gfn_to_mfn* functions, but none triggers. > Where should I start looking for this kind of bug? > > Olaf > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re: xenpaging fixes for kernel and hypervisor 2010-09-22 16:49 ` Patrick Colp @ 2010-09-22 17:13 ` Keir Fraser 2010-09-22 17:14 ` Keir Fraser 2010-09-22 17:20 ` Olaf Hering 2 siblings, 0 replies; 16+ messages in thread From: Keir Fraser @ 2010-09-22 17:13 UTC (permalink / raw) To: Patrick Colp, Olaf Hering; +Cc: xen-devel@lists.xensource.com On 22/09/2010 17:49, "Patrick Colp" <pjcolp@cs.ubc.ca> wrote: > I don't know if I need to ack or not, but I will: > > Acked-by: Patrick Colp <pjcolp@cs.ubc.ca> > > > I think the issue with realmode is in the emulation code. Looking at > where that crash occurs, it's a result of hvm_emulate_one() returning > X86EMUL_UNHANDLEABLE. hvm_emulate_one() calls x86_emulate(), and for > supported functions, that will call hvmemul_* using the > hvm_emulate_ops pointer function struct. However, my guess is that > what's causing this problem is an instruction that isn't handled by > that stuff (the hvmemul_* stuff). The instruction is dumped as "0f 00 00 00 00 ..." which is a bit unlikely to be a real instruction. Something went wrong before that point, most likely. -- Keir ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re: xenpaging fixes for kernel and hypervisor 2010-09-22 16:49 ` Patrick Colp 2010-09-22 17:13 ` Keir Fraser @ 2010-09-22 17:14 ` Keir Fraser 2010-09-22 17:20 ` Olaf Hering 2 siblings, 0 replies; 16+ messages in thread From: Keir Fraser @ 2010-09-22 17:14 UTC (permalink / raw) To: Patrick Colp, Olaf Hering, Ian Jackson; +Cc: xen-devel@lists.xensource.com All th epatches can be reviewed by tools maintainers and then checked in by them in their entirety as far as I'm concerned, hypervisor portions included. Acked-by: Keir Fraser <keir.fraser@citrix.com> -- Keir On 22/09/2010 17:49, "Patrick Colp" <pjcolp@cs.ubc.ca> wrote: > I don't know if I need to ack or not, but I will: > > Acked-by: Patrick Colp <pjcolp@cs.ubc.ca> > > > I think the issue with realmode is in the emulation code. Looking at > where that crash occurs, it's a result of hvm_emulate_one() returning > X86EMUL_UNHANDLEABLE. hvm_emulate_one() calls x86_emulate(), and for > supported functions, that will call hvmemul_* using the > hvm_emulate_ops pointer function struct. However, my guess is that > what's causing this problem is an instruction that isn't handled by > that stuff (the hvmemul_* stuff). > > The way I'd probably go about this is to try to find out what > instruction it's trying to emulate when it fails, then to look in the > x86_emulate() code and see where/if that instruction is handled. Once > that's determined, then you should be able to find out why it's > returning X86EMUL_UNHANDLEABLE. Either it's because x86_emulate() > doesn't handle it at all (this only seems likely if the paging code > has forced the realmode code down a different path, like trying to > handle a page fault or something) or the code to handle it assumes > that the memory is present in the guest (which was a fair assumption > before paging and was a common problem when I was plumbing the rest of > the paging code through the emulator). In this case, a check needs to > be made to see if the memory the instruction is trying to access is > paged out, and if so the result propagated back and everything plumbed > through. The convention when emulating and detecting a paged out page > is to call p2m_paging_populate and return X86EMUL_RETRY. This should > cause the guest/emulate code to just keep retrying the instruction > until it succeeds (once the page has been paged back in). There's a > chance that the problem occurs when trying to fetch an instruction if > that instruction lives on a page that's paged out. I know this case > was handled with the regular hvmemul_* code, but not sure if it > becomes an issue again with realmode. > > If you know the instruction or can send me whatever setup you use to > cause this bug, then I can help track it down. It sounds from your > other e-mails like you've just modified "xm create" to page everything > out right away? > > > Patrick > > > On 22 September 2010 08:48, Olaf Hering <olaf@aepfle.de> wrote: >> >> Patrick, >> >> there are three more changes to make xenpaging more robust. >> Do you need to ack each one to get them merged in xen-unstable? >> Should any of these changes go also into the xen-4.0-testing tree for >> the 4.0.2 release? If so, I will prepare the patches for this branch. >> >> >> One more thing: In an earlier mail you mentioned that realmode support >> is not there yet. However, in my testing I can run grub and the bios and >> even boot into Linux a bit. So it appears there is realmode support, >> perhaps still incomplete because the guest crashes in (appearently) >> early Linux init functions: >> >> (XEN) realmode.c:115:d1 Failed to emulate insn. >> (XEN) realmode.c:165:d1 Real-mode emulation failed @ 9000:0000f81a: 0f 00 00 >> 00 00 00 >> (XEN) domain_crash called from realmode.c:166 >> (XEN) Domain 1 (vcpu#0) crashed on cpu#0: >> (XEN) ----[ Xen-4.0.1_21326_01-20100922.141534 x86_64 debug=y Tainted: >> C ]---- >> (XEN) CPU: 0 >> (XEN) RIP: 9000:[<000000000000f81a>] >> (XEN) RFLAGS: 0000000000000246 CONTEXT: hvm guest >> (XEN) rax: 0000000000000000 rbx: 0000000000008fb8 rcx: 0000000000000000 >> (XEN) rdx: 0000000000009000 rsi: 0000000000000008 rdi: 0000000000099fff >> (XEN) rbp: 000000000000ffff rsp: 0000000000001ff2 r8: 0000000000000000 >> (XEN) r9: 0000000000000000 r10: 0000000000000000 r11: 0000000000000000 >> (XEN) r12: 0000000000000000 r13: 0000000000000000 r14: 0000000000000000 >> (XEN) r15: 0000000000000000 cr0: 0000000000000010 cr4: 0000000000000000 >> (XEN) cr3: 0000000000000000 cr2: 0000000000000000 >> (XEN) ds: 0000 es: 9000 fs: 9000 gs: 9000 ss: 9000 cs: 9000 >> (XEN) cpupool_rm_domain(dom=1,pool=0) n_dom 1 >> [ 127.709423] br0: port 2(vif1.0) entering disabled state >> [ 127.732999] br0: port 2(vif1.0) entering disabled state >> >> I'm poking around and adding debug to the gfn_to_mfn* functions, but none >> triggers. >> Where should I start looking for this kind of bug? >> >> Olaf >> >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: xenpaging fixes for kernel and hypervisor 2010-09-22 16:49 ` Patrick Colp 2010-09-22 17:13 ` Keir Fraser 2010-09-22 17:14 ` Keir Fraser @ 2010-09-22 17:20 ` Olaf Hering 2 siblings, 0 replies; 16+ messages in thread From: Olaf Hering @ 2010-09-22 17:20 UTC (permalink / raw) To: Patrick Colp; +Cc: xen-devel On Wed, Sep 22, Patrick Colp wrote: Thanks for your reply. I will try to fix this issue. > If you know the instruction or can send me whatever setup you use to > cause this bug, then I can help track it down. It sounds from your > other e-mails like you've just modified "xm create" to page everything > out right away? Its just xm start --vncviewer ${guest} ; xenpaging ${id} ${value}. At some point I would like to integrate it into the normal guest configuration, so its started similar to qemu-dm. Olaf ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-09-22 17:20 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-15 16:05 xenpaging fixes for kernel and hypervisor Olaf Hering 2010-09-15 16:08 ` [PATCH] xenpaging: handle GNTST_eagain in kernel drivers Olaf Hering 2010-09-16 3:44 ` Patrick Colp 2010-09-16 9:05 ` Keir Fraser 2010-09-16 13:33 ` Olaf Hering 2010-09-15 16:10 ` [PATCH] xenpaging: page-in granttable entries Olaf Hering 2010-09-15 16:11 ` [PATCH] xenpaging: fix crash in notify_via_xen_event_channel Olaf Hering 2010-09-15 16:12 ` [PATCH] xenpaging: avoid empty pagefile on unsupported hardware Olaf Hering 2010-09-22 15:38 ` [PATCH] xenpaging: handle XENMEM_decrease_reservation Olaf Hering 2010-09-22 15:40 ` [PATCH] xenpaging: break endless loop during inital page-out with large pagefiles Olaf Hering 2010-09-22 15:41 ` [PATCH] xenpaging: allow only one xenpaging call per guest Olaf Hering 2010-09-22 15:48 ` xenpaging fixes for kernel and hypervisor Olaf Hering 2010-09-22 16:49 ` Patrick Colp 2010-09-22 17:13 ` Keir Fraser 2010-09-22 17:14 ` Keir Fraser 2010-09-22 17:20 ` Olaf Hering
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).