[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers



 drivers/xen/blkback/blkback.c              |   6 ++-
 drivers/xen/blkback/interface.c            |   9 +++-
 drivers/xen/core/gnttab.c                  |   4 +-
 drivers/xen/gntdev/gntdev.c                |  49 +++++++++++++++++------------
 drivers/xen/netback/interface.c            |   5 +-
 drivers/xen/netback/netback.c              |  16 ++++++---
 drivers/xen/scsiback/interface.c           |  10 +++---
 drivers/xen/scsiback/scsiback.c            |   4 +-
 drivers/xen/tpmback/interface.c            |   7 +--
 drivers/xen/tpmback/tpmback.c              |  20 ++++-------
 drivers/xen/usbback/interface.c            |  16 ++++----
 drivers/xen/usbback/usbback.c              |   4 +-
 drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
 include/xen/gnttab.h                       |  37 ++++++++++++++++++++++
 14 files changed, 126 insertions(+), 71 deletions(-)


Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
GNTTABOP_copy operations properly to allow usage of xenpaging without
causing crashes or data corruption.

Replace all relevant HYPERVISOR_grant_table_op() calls with a retry
loop. This loop is implemented as a macro to allow different
GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
page to be paged in again.

All ->status checks were updated to use the GNTST_* namespace. All
return values are converted from GNTST_* namespace to 0/-EINVAL, since
all callers did not use the actual return value.

Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
Acked-by: Patrick Colp <pjcolp@xxxxxxxxx>

This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>

diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/blkback.c
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
        BUG_ON(ret);
 
        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("grant map of dom %u gref %u failed: status 
%d\n",
                                blkif->domid, req->seg[i].gref, map[i].status);
                        map[i].handle = BLKBACK_INVALID_HANDLE;
-                       ret |= 1;
+                       ret = 1;
                        continue;
                }
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/interface.c
--- a/drivers/xen/blkback/interface.c
+++ b/drivers/xen/blkback/interface.c
@@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
        struct vm_struct *area = blkif->blk_ring_area;
        struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
        unsigned int i;
-       int status = 0;
+       int status = GNTST_okay;
 
        for (i = 0; i < nr_shared_pages; i++) {
                unsigned long addr = (unsigned long)area->addr +
@@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
                BUG();
 
        for (i = 0; i < nr_shared_pages; i++) {
-               if ((status = op[i].status) != 0) {
+               if (op[i].status == GNTST_eagain)
+                       
gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op[i]);
+               if (op[i].status != GNTST_okay) {
+                       status = op[i].status;
                        blkif->shmem_handle[i] = INVALID_GRANT_HANDLE;
                        continue;
                }
@@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
 
        blkif->nr_shared_pages = nr_shared_pages;
 
-       if (status != 0) {
+       if (status != GNTST_okay) {
                DPRINTK(" Grant table operation failure !\n");
                unmap_frontend_pages(blkif);
        }
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
--- a/drivers/xen/core/gnttab.c
+++ b/drivers/xen/core/gnttab.c
@@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
                return -ENOSYS;
        }
 
-       BUG_ON(rc || setup.status);
+       BUG_ON(rc || (setup.status != GNTST_okay));
 
        if (shared.raw == NULL)
                shared.raw = arch_gnttab_alloc_shared(gframes);
@@ -901,7 +901,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_irq(&gnttab_dma_lock);
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/gntdev/gntdev.c
--- a/drivers/xen/gntdev/gntdev.c
+++ b/drivers/xen/gntdev/gntdev.c
@@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
        uint64_t ptep;
        int ret;
        int flags;
-       int i;
+       int i, exit_ret;
        struct page *page;
        gntdev_file_private_data_t *private_data = flip->private_data;
 
@@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
        vma->vm_mm->context.has_foreign_mappings = 1;
 #endif
 
+       exit_ret = -ENOMEM;
        for (i = 0; i < size; ++i) {
 
                flags = GNTMAP_host_map;
@@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
                ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, 
                                                &op, 1);
                BUG_ON(ret);
-               if (op.status) {
-                       printk(KERN_ERR "Error mapping the grant reference "
-                              "into the kernel (%d). domid = %d; ref = %d\n",
-                              op.status,
-                              private_data->grants[slot_index+i]
-                              .u.valid.domid,
-                              private_data->grants[slot_index+i]
-                              .u.valid.ref);
+               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,
+                                          private_data->grants[slot_index+i]
+                                          .u.valid.domid,
+                                          private_data->grants[slot_index+i]
+                                          .u.valid.ref);
+                       else
+                               /* Propagate instead of trying to fix it up */
+                               exit_ret = -EAGAIN;
                        goto undo_map_out;
                }
 
@@ -674,14 +679,17 @@ 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,
-                                      private_data->grants[slot_index+i].u
-                                      .valid.domid,
-                                      private_data->grants[slot_index+i].u
-                                      .valid.ref);
+                                          "reference into user space (%d). 
domid "
+                                          "= %d; ref = %d\n", op.status,
+                                          private_data->grants[slot_index+i].u
+                                          .valid.domid,
+                                          private_data->grants[slot_index+i].u
+                                          .valid.ref);
+                               /* GNTST_eagain (i.e. page paged out) sohuld 
never happen
+                                * once we've mapped into kernel space */
+                               BUG_ON(op.status == GNTST_eagain);
                                goto undo_map_out;
                        }
                        
@@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
                }
 
        }
+       exit_ret = 0;
 
        up_write(&private_data->grants_sem);
-       return 0;
+       return exit_ret;
 
 undo_map_out:
        /* If we have a mapping failure, the unmapping will be taken care of
@@ -725,7 +734,7 @@ undo_map_out:
        
        up_write(&private_data->grants_sem);
 
-       return -ENOMEM;
+       return exit_ret;
 }
 
 static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned long addr,
@@ -777,7 +786,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 {
@@ -794,7 +803,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);
 
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/interface.c
--- a/drivers/xen/netback/interface.c
+++ b/drivers/xen/netback/interface.c
@@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
        gnttab_set_map_op(&op, (unsigned long)comms->ring_area->addr,
                          GNTMAP_host_map, ring_ref, domid);
     
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-       if (op.status) {
+       if (op.status != GNTST_okay) {
                DPRINTK(" Gnttab failure mapping ring_ref!\n");
                free_vm_area(comms->ring_area);
                return op.status;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/netback.c
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
 
        for (i = 0; i < nr_copy_slots; i++) {
                copy_op = npo->copy + npo->copy_cons++;
+               if (copy_op->status == GNTST_eagain)
+                       gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
                if (copy_op->status != GNTST_okay) {
-                               DPRINTK("Bad status %d from copy to DOM%d.\n",
-                                       copy_op->status, domid);
-                               status = NETIF_RSP_ERROR;
-                       }
+                       DPRINTK("Bad status %d from copy to DOM%d.\n",
+                               copy_op->status, domid);
+                       status = NETIF_RSP_ERROR;
+               }
        }
 
        return status;
@@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct xen
 
        /* Check status of header. */
        err = mop->status;
-       if (unlikely(err)) {
+       if (err == GNTST_eagain) {
+               gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, mop);
+               err = mop->status;
+       }
+       if (unlikely(err != GNTST_okay)) {
                pending_ring_idx_t index;
                index = pending_index(netbk->pending_prod++);
                txp = &pending_tx_info[pending_idx].req;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/interface.c
--- a/drivers/xen/scsiback/interface.c
+++ b/drivers/xen/scsiback/interface.c
@@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
                                GNTMAP_host_map, ring_ref,
                                info->domid);
 
-       err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1);
-       BUG_ON(err);
+       gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-       if (op.status) {
-               printk(KERN_ERR "scsiback: Grant table operation failure !\n");
+       if (op.status != GNTST_okay) {
+               printk(KERN_ERR "scsiback: Grant table operation failure 
%d!\n", 
+                                                       (int) op.status);
                return op.status;
        }
 
        info->shmem_ref    = ring_ref;
        info->shmem_handle = op.handle;
 
-       return (GNTST_okay);
+       return 0;
 }
 
 static void unmap_frontend_page(struct vscsibk_info *info)
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/scsiback.c
--- a/drivers/xen/scsiback/scsiback.c
+++ b/drivers/xen/scsiback/scsiback.c
@@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
                for_each_sg (pending_req->sgl, sg, nr_segments, i) {
                        struct page *pg;
 
-                       if (unlikely(map[i].status != 0)) {
+                       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;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/interface.c
--- a/drivers/xen/tpmback/interface.c
+++ b/drivers/xen/tpmback/interface.c
@@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
        gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr,
                          GNTMAP_host_map, shared_page, tpmif->domid);
 
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-       if (op.status) {
-               DPRINTK(" Grant table operation failure !\n");
+       if (op.status != GNTST_okay) {
+               DPRINTK(" Grant table operation failure %d!\n", (int) 
op.status);
                return op.status;
        }
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/tpmback.c
--- a/drivers/xen/tpmback/tpmback.c
+++ b/drivers/xen/tpmback/tpmback.c
@@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
                gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
                                  GNTMAP_host_map, tx->ref, tpmif->domid);
 
-               if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
-                                                      &map_op, 1))) {
-                       BUG();
-               }
+               gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, 
&map_op);
 
                handle = map_op.handle;
 
-               if (map_op.status) {
-                       DPRINTK(" Grant table operation failure !\n");
+               if (map_op.status != GNTST_okay) {
+                       DPRINTK(" Grant table operation failure %d!\n", 
+                                               (int) map_op.status);
                        return 0;
                }
 
@@ -394,13 +392,11 @@ 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);
 
-               if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
-                                                      &map_op, 1))) {
-                       BUG();
-               }
+               gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, 
&map_op);
 
-               if (map_op.status) {
-                       DPRINTK(" Grant table operation failure !\n");
+               if (map_op.status != GNTST_okay) {
+                       DPRINTK(" Grant table operation failure %d!\n",
+                                               (int) map_op.status);
                        return -EFAULT;
                }
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/interface.c
--- a/drivers/xen/usbback/interface.c
+++ b/drivers/xen/usbback/interface.c
@@ -109,11 +109,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);
 
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-       if (op.status) {
-               printk(KERN_ERR "grant table failure mapping urb_ring_ref\n");
+       if (op.status != GNTST_okay) {
+               printk(KERN_ERR "grant table failure mapping urb_ring_ref %d\n",
+                                                       (int) op.status);
                return op.status;
        }
 
@@ -123,17 +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);
 
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       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");
+               printk(KERN_ERR "grant table failure mapping conn_ring_ref 
%d\n",
+                                                       (int) op.status);
                return op.status;
        }
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/usbback.c
--- a/drivers/xen/usbback/usbback.c
+++ b/drivers/xen/usbback/usbback.c
@@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
                BUG_ON(ret);
 
                for (i = 0; i < nr_segs; i++) {
-                       if (unlikely(map[i].status != 0)) {
+                       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;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/xenbus/xenbus_backend_client.c
--- a/drivers/xen/xenbus/xenbus_backend_client.c
+++ b/drivers/xen/xenbus/xenbus_backend_client.c
@@ -48,8 +48,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);
        
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
        if (op.status != GNTST_okay) {
                free_vm_area(area);
@@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
        
        gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map,
                          gnt_ref, dev->otherend_id);
-       if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-               BUG();
+       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
+       } else {
                *handle = op.handle;
+               return 0;
+       }
 
        return op.status;
 }
diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
--- a/include/xen/gnttab.h
+++ b/include/xen/gnttab.h
@@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
        unmap->handle = handle;
 }
 
+#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)                     
                \
+do {                                                                           
                                                                \
+       u8 __hc_delay = 1;                                                      
                                                        \
+       int __ret;                                                              
                                                                \
+       while (unlikely((__HCarg_p)->status == GNTST_eagain && __hc_delay)) {   
\
+               msleep(__hc_delay++);                                           
                                                \
+               __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);      
                \
+               BUG_ON(__ret);                                                  
                                                        \
+       }                                                                       
                                                                        \
+       if (__hc_delay == 0) {                                                  
                                                \
+               printk(KERN_ERR "%s: %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);          
                        \
+} while(0)
+
+#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)                  
\
+do {                                                                           
                                                        \
+       u8 __hc_delay = 1;                                                      
                                                \
+       int __ret;                                                              
                                                        \
+       do {                                                                    
                                                        \
+               __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);      
        \
+               BUG_ON(__ret);                                                  
                                                \
+               if ((__HCarg_p)->status == GNTST_eagain)                        
                        \
+                       msleep(__hc_delay++);                                   
                                        \
+       } while ((__HCarg_p)->status == GNTST_eagain && __hc_delay);            
\
+       if (__hc_delay == 0) {                                                  
                                        \
+               printk(KERN_ERR "%s: %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);          
                \
+} while(0)
+
 #endif /* __ASM_GNTTAB_H__ */

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.