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

[Xen-changelog] Fix grant-table transfer implementation. Also fix transfer



# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID 0add9fbe93ab843d239364386ff2e5bc82f75c5f
# Parent  675862d22347c8f7efa87b9b49fff5d62e2b3516
Fix grant-table transfer implementation. Also fix transfer
error paths in network frontend and backend drivers.

Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>

diff -r 675862d22347 -r 0add9fbe93ab 
linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c
--- a/linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c     Mon Nov 21 12:17:29 2005
+++ b/linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c     Mon Nov 21 13:17:50 2005
@@ -229,20 +229,29 @@
 unsigned long
 gnttab_end_foreign_transfer_ref(grant_ref_t ref)
 {
-       unsigned long frame = 0;
+       unsigned long frame;
        u16           flags;
 
-       flags = shared[ref].flags;
-
        /*
-        * If a transfer is committed then wait for the frame address to
-        * appear. Otherwise invalidate the grant entry against future use.
-        */
-       if (likely(flags != GTF_accept_transfer) ||
-           (synch_cmpxchg(&shared[ref].flags, flags, 0) !=
-            GTF_accept_transfer))
-               while (unlikely((frame = shared[ref].frame) == 0))
-                       cpu_relax();
+         * If a transfer is not even yet started, try to reclaim the grant
+         * reference and return failure (== 0).
+         */
+       while (!((flags = shared[ref].flags) & GTF_transfer_committed)) {
+               if ( synch_cmpxchg(&shared[ref].flags, flags, 0) == flags )
+                       return 0;
+               cpu_relax();
+       }
+
+       /* If a transfer is in progress then wait until it is completed. */
+       while (!(flags & GTF_transfer_completed)) {
+               flags = shared[ref].flags;
+               cpu_relax();
+       }
+
+       /* Read the frame number /after/ reading completion status. */
+       rmb();
+       frame = shared[ref].frame;
+       BUG_ON(frame == 0);
 
        return frame;
 }
diff -r 675862d22347 -r 0add9fbe93ab 
linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Nov 21 
12:17:29 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Nov 21 
13:17:50 2005
@@ -99,7 +99,6 @@
        return mfn;
 }
 
-#if 0
 static void free_mfn(unsigned long mfn)
 {
        unsigned long flags;
@@ -120,7 +119,6 @@
        }
        spin_unlock_irqrestore(&mfn_lock, flags);
 }
-#endif
 
 static inline void maybe_schedule_tx_action(void)
 {
@@ -287,28 +285,19 @@
        ret = HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl);
        BUG_ON(ret != 0);
 
+       ret = HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, 
+                                       gop - grant_rx_op);
+       BUG_ON(ret != 0);
+
        mcl = rx_mcl;
-       if( HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, 
-                                     gop - grant_rx_op)) { 
-               /*
-                * The other side has given us a bad grant ref, or has no 
-                * headroom, or has gone away. Unfortunately the current grant
-                * table code doesn't inform us which is the case, so not much
-                * we can do. 
-                */
-               DPRINTK("net_rx: transfer to DOM%u failed; dropping (up to) "
-                       "%d packets.\n",
-                       grant_rx_op[0].domid, gop - grant_rx_op); 
-       }
        gop = grant_rx_op;
-
        while ((skb = __skb_dequeue(&rxq)) != NULL) {
                netif   = netdev_priv(skb->dev);
                size    = skb->tail - skb->data;
 
                /* Rederive the machine addresses. */
-               new_mfn = mcl[0].args[1] >> PAGE_SHIFT;
-               old_mfn = 0; /* XXX Fix this so we can free_mfn() on error! */
+               new_mfn = mcl->args[1] >> PAGE_SHIFT;
+               old_mfn = gop->mfn;
                atomic_set(&(skb_shinfo(skb)->dataref), 1);
                skb_shinfo(skb)->nr_frags = 0;
                skb_shinfo(skb)->frag_list = NULL;
@@ -321,10 +310,10 @@
 
                /* Check the reassignment error code. */
                status = NETIF_RSP_OKAY;
-               if(gop->status != 0) { 
+               if (gop->status != 0) { 
                        DPRINTK("Bad status %d from grant transfer to DOM%u\n",
                                gop->status, netif->domid);
-                       /* XXX SMH: should free 'old_mfn' here */
+                       free_mfn(old_mfn);
                        status = NETIF_RSP_ERROR; 
                }
                irq = netif->irq;
diff -r 675862d22347 -r 0add9fbe93ab 
linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Mon Nov 21 
12:17:29 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Mon Nov 21 
13:17:50 2005
@@ -739,14 +739,23 @@
             (i != rp) && (work_done < budget);
             i++, work_done++) {
                rx = &np->rx->ring[MASK_NETIF_RX_IDX(i)].resp;
+
                /*
-                * An error here is very odd. Usually indicates a backend bug,
-                * low-mem condition, or we didn't have reservation headroom.
-                */
-               if (unlikely(rx->status <= 0)) {
+                 * This definitely indicates a bug, either in this driver or
+                 * in the backend driver. In future this should flag the bad
+                 * situation to the system controller to reboot the backed.
+                 */
+               if ((ref = np->grant_rx_ref[rx->id]) == GRANT_INVALID_REF) {
+                       WPRINTK("Bad rx response id %d.\n", rx->id);
+                       work_done--;
+                       continue;
+               }
+
+               /* Memory pressure, insufficient buffer headroom, ... */
+               if ((mfn = gnttab_end_foreign_transfer_ref(ref)) == 0) {
                        if (net_ratelimit())
-                               printk(KERN_WARNING "Bad rx buffer "
-                                      "(memory squeeze?).\n");
+                               WPRINTK("Unfulfilled rx req (id=%d, st=%d).\n",
+                                       rx->id, rx->status);
                        np->rx->ring[MASK_NETIF_RX_IDX(np->rx->req_prod)].
                                req.id = rx->id;
                        wmb();
@@ -755,23 +764,8 @@
                        continue;
                }
 
-               ref = np->grant_rx_ref[rx->id]; 
-
-               if(ref == GRANT_INVALID_REF) { 
-                       printk(KERN_WARNING "Bad rx grant reference %d "
-                              "from dom %d.\n",
-                              ref, np->xbdev->otherend_id);
-                       np->rx->ring[MASK_NETIF_RX_IDX(np->rx->req_prod)].
-                               req.id = rx->id;
-                       wmb();
-                       np->rx->req_prod++;
-                       work_done--;
-                       continue;
-               }
-
+               gnttab_release_grant_reference(&np->gref_rx_head, ref);
                np->grant_rx_ref[rx->id] = GRANT_INVALID_REF;
-               mfn = gnttab_end_foreign_transfer_ref(ref);
-               gnttab_release_grant_reference(&np->gref_rx_head, ref);
 
                skb = np->rx_skbs[rx->id];
                ADD_ID_TO_FREELIST(np->rx_skbs, rx->id);
diff -r 675862d22347 -r 0add9fbe93ab xen/common/grant_table.c
--- a/xen/common/grant_table.c  Mon Nov 21 12:17:29 2005
+++ b/xen/common/grant_table.c  Mon Nov 21 13:17:50 2005
@@ -706,35 +706,34 @@
     struct pfn_info *page;
     u32 _d, _nd, x, y;
     int i;
-    int result = GNTST_okay;
     grant_entry_t *sha;
+    gnttab_transfer_t gop;
 
     for ( i = 0; i < count; i++ )
     {
-        gnttab_transfer_t *gop = &uop[i];
-
-        page = &frame_table[gop->mfn];
-        
-        if ( unlikely(IS_XEN_HEAP_FRAME(page)))
+        /* Read from caller address space. */
+        if ( unlikely(__copy_from_user(&gop, &uop[i], sizeof(gop))) )
+        {
+            /* Caller error: bail immediately. */
+            DPRINTK("gnttab_transfer: error reading req %d/%d\n", i, count);
+            return -EFAULT;
+        }
+
+        /* Check the passed page frame for basic validity. */
+        page = &frame_table[gop.mfn];
+        if ( unlikely(!pfn_valid(gop.mfn) || IS_XEN_HEAP_FRAME(page)) )
         { 
-            printk("gnttab_transfer: xen heap frame mfn=%lx\n", 
-                   (unsigned long) gop->mfn);
-            gop->status = GNTST_bad_virt_addr;
-            continue;
-        }
-        
-        if ( unlikely(!pfn_valid(page_to_pfn(page))) )
-        {
-            printk("gnttab_transfer: invalid pfn for mfn=%lx\n", 
-                   (unsigned long) gop->mfn);
-            gop->status = GNTST_bad_virt_addr;
-            continue;
-        }
-
-        if ( unlikely((e = find_domain_by_id(gop->domid)) == NULL) )
-        {
-            printk("gnttab_transfer: can't find domain %d\n", gop->domid);
-            gop->status = GNTST_bad_domain;
+            /* Caller error: bail immediately. */
+            DPRINTK("gnttab_transfer: out-of-range or xen frame %lx\n",
+                    (unsigned long)gop.mfn);
+            return -EINVAL;
+        }
+
+        /* Find the target domain. */
+        if ( unlikely((e = find_domain_by_id(gop.domid)) == NULL) )
+        {
+            DPRINTK("gnttab_transfer: can't find domain %d\n", gop.domid);
+            (void)__put_user(GNTST_bad_domain, &uop[i].status);
             continue;
         }
 
@@ -752,15 +751,16 @@
         do {
             x = y;
             if (unlikely((x & (PGC_count_mask|PGC_allocated)) !=
-                         (1 | PGC_allocated)) || unlikely(_nd != _d)) {
-                printk("gnttab_transfer: Bad page values %p: ed=%p(%u), sd=%p,"
+                         (1 | PGC_allocated)) || unlikely(_nd != _d)) { 
+                /* Caller error: bail immediately. */
+                DPRINTK("gnttab_transfer: Bad page %p: ed=%p(%u), sd=%p,"
                        " caf=%08x, taf=%" PRtype_info "\n", 
                        (void *) page_to_pfn(page),
                         d, d->domain_id, unpickle_domptr(_nd), x, 
                         page->u.inuse.type_info);
                 spin_unlock(&d->page_alloc_lock);
                 put_domain(e);
-                return 0;
+                return -EINVAL;
             }
             __asm__ __volatile__(
                 LOCK_PREFIX "cmpxchg8b %2"
@@ -788,16 +788,16 @@
          */
         if ( unlikely(test_bit(_DOMF_dying, &e->domain_flags)) ||
              unlikely(e->tot_pages >= e->max_pages) ||
-             unlikely(!gnttab_prepare_for_transfer(e, d, gop->ref)) )
+             unlikely(!gnttab_prepare_for_transfer(e, d, gop.ref)) )
         {
             DPRINTK("gnttab_transfer: Transferee has no reservation headroom "
                     "(%d,%d) or provided a bad grant ref (%08x) or "
                     "is dying (%lx)\n",
-                    e->tot_pages, e->max_pages, gop->ref, e->domain_flags);
+                    e->tot_pages, e->max_pages, gop.ref, e->domain_flags);
             spin_unlock(&e->page_alloc_lock);
             put_domain(e);
-            gop->status = result = GNTST_general_error;
-            break;
+            (void)__put_user(GNTST_general_error, &uop[i].status);
+            continue;
         }
 
         /* Okay, add the page to 'e'. */
@@ -805,23 +805,23 @@
             get_knownalive_domain(e);
         list_add_tail(&page->list, &e->page_list);
         page_set_owner(page, e);
-        
+
         spin_unlock(&e->page_alloc_lock);
 
         TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
-        
+
         /* Tell the guest about its new page frame. */
-        sha = &e->grant_table->shared[gop->ref];
-        sha->frame = gop->mfn;
+        sha = &e->grant_table->shared[gop.ref];
+        sha->frame = gop.mfn;
         wmb();
         sha->flags |= GTF_transfer_completed;
-        
+
         put_domain(e);
-        
-        gop->status = GNTST_okay;
-    }
-
-    return result;
+
+        (void)__put_user(GNTST_okay, &uop[i].status);
+    }
+
+    return 0;
 }
 
 long 

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


 


Rackspace

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