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@xxxxxxxxx>
Patrick
On 15 September 2010 09:08, Olaf Hering <olaf@xxxxxxxxx> 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@xxxxxxxxx>
>
> ---
> 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__ */
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home