[Xen-changelog] Change semantics of gnttab_transfer to take the page away

[Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID 52020caa244f9c86c434b8332783c1edc658d852
# Parent  e336e186e5f96a0e93f842ca3a2e39601c2a0193
Change semantics of gnttab_transfer to take the page away
from the calling domain, even on error (except for special
error GNTST_bad_page). This has a knock-on advantage of
simplifying Linux's netback driver.

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

diff -r e336e186e5f9 -r 52020caa244f 
linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Nov 21 
16:30:43 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Nov 21 
17:10:35 2005
@@ -97,27 +97,6 @@
                mfn = mfn_list[--alloc_index];
        spin_unlock_irqrestore(&mfn_lock, flags);
        return mfn;
-}
-
-static void free_mfn(unsigned long mfn)
-{
-       unsigned long flags;
-       struct xen_memory_reservation reservation = {
-               .extent_start = &mfn,
-               .nr_extents   = 1,
-               .extent_order = 0,
-               .domid        = DOMID_SELF
-       };
-       spin_lock_irqsave(&mfn_lock, flags);
-       if ( alloc_index != MAX_MFN_ALLOC )
-               mfn_list[alloc_index++] = mfn;
-       else {
-               int ret;
-               ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation,
-                                           &reservation);
-               BUG_ON(ret != 1);
-       }
-       spin_unlock_irqrestore(&mfn_lock, flags);
 }
 
 static inline void maybe_schedule_tx_action(void)
@@ -306,14 +285,18 @@
                netif->stats.tx_packets++;
 
                /* The update_va_mapping() must not fail. */
-               BUG_ON(mcl[0].result != 0);
+               BUG_ON(mcl->result != 0);
 
                /* Check the reassignment error code. */
                status = NETIF_RSP_OKAY;
                if (gop->status != 0) { 
                        DPRINTK("Bad status %d from grant transfer to DOM%u\n",
                                gop->status, netif->domid);
-                       free_mfn(old_mfn);
+                       /*
+                         * Page no longer belongs to us unless GNTST_bad_page,
+                         * but that should be a fatal error anyway.
+                         */
+                       BUG_ON(gop->status == GNTST_bad_page);
                        status = NETIF_RSP_ERROR; 
                }
                irq = netif->irq;
diff -r e336e186e5f9 -r 52020caa244f xen/common/grant_table.c
--- a/xen/common/grant_table.c  Mon Nov 21 16:30:43 2005
+++ b/xen/common/grant_table.c  Mon Nov 21 17:10:35 2005
@@ -714,26 +714,18 @@
         /* 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;
+            (void)__put_user(GNTST_bad_page, &uop[i].status);
+            return -EFAULT; /* This is *very* fatal. */
         }
 
         /* Check the passed page frame for basic validity. */
         page = &frame_table[gop.mfn];
         if ( unlikely(!pfn_valid(gop.mfn) || IS_XEN_HEAP_FRAME(page)) )
         { 
-            /* 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);
+            (void)__put_user(GNTST_bad_page, &uop[i].status);
             continue;
         }
 
@@ -752,15 +744,14 @@
             x = y;
             if (unlikely((x & (PGC_count_mask|PGC_allocated)) !=
                          (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 -EINVAL;
+                (void)__put_user(GNTST_bad_page, &uop[i].status);
+                continue;
             }
             __asm__ __volatile__(
                 LOCK_PREFIX "cmpxchg8b %2"
@@ -778,6 +769,16 @@
         list_del(&page->list);
 
         spin_unlock(&d->page_alloc_lock);
+
+        /* 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);
+            page->count_info &= ~(PGC_count_mask|PGC_allocated);
+            free_domheap_page(page);
+            continue;
+        }
 
         spin_lock(&e->page_alloc_lock);
 
@@ -797,6 +798,8 @@
             spin_unlock(&e->page_alloc_lock);
             put_domain(e);
             (void)__put_user(GNTST_general_error, &uop[i].status);
+            page->count_info &= ~(PGC_count_mask|PGC_allocated);
+            free_domheap_page(page);
             continue;
         }
 
diff -r e336e186e5f9 -r 52020caa244f xen/common/page_alloc.c
--- a/xen/common/page_alloc.c   Mon Nov 21 16:30:43 2005
+++ b/xen/common/page_alloc.c   Mon Nov 21 17:10:35 2005
@@ -645,7 +645,9 @@
     }
     else
     {
-        /* Freeing an anonymous domain-heap page. */
+        /* Freeing anonymous domain-heap pages. */
+        for ( i = 0; i < (1 << order); i++ )
+            pg[i].u.free.cpumask = CPU_MASK_NONE;
         free_heap_pages(pfn_dom_zone_type(page_to_pfn(pg)), pg, order);
         drop_dom_ref = 0;
     }
diff -r e336e186e5f9 -r 52020caa244f xen/include/public/grant_table.h
--- a/xen/include/public/grant_table.h  Mon Nov 21 16:30:43 2005
+++ b/xen/include/public/grant_table.h  Mon Nov 21 17:10:35 2005
@@ -215,9 +215,12 @@
 } gnttab_dump_table_t;
 
 /*
- * GNTTABOP_transfer_grant_ref: Transfer <frame> to a foreign domain.  The
- * foreign domain has previously registered the details of the transfer.
- * These can be identified from <handle>, a grant reference.
+ * GNTTABOP_transfer_grant_ref: Transfer <frame> to a foreign domain. The
+ * foreign domain has previously registered its interest in the transfer via
+ * <domid, ref>.
+ * 
+ * Note that, even if the transfer fails, the specified page no longer belongs
+ * to the calling domain *unless* the error is GNTST_bad_page.
  */
 #define GNTTABOP_transfer                4
 typedef struct {
@@ -269,6 +272,7 @@
 #define GNTST_bad_dev_addr     (-6) /* Inappropriate device address to unmap.*/
 #define GNTST_no_device_space  (-7) /* Out of space in I/O MMU.              */
 #define GNTST_permission_denied (-8) /* Not enough privilege for operation.  */
+#define GNTST_bad_page         (-9) /* Specified page was invalid for op.    */
 
 #define GNTTABOP_error_msgs {                   \
     "okay",                                     \

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