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

[Xen-changelog] Increment page reference count for every host/device



# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID 48eb10d7a2d608351023b14e45a9ddb7f99cab8a
# Parent  f2a08a5a807acddced92db03a5a717bc66c95cc5
Increment page reference count for every host/device
mapping created via a grant reference, rather than one
increment for all uses of a single grant reference.
This avoids a nasty situation in put_page_from_l1e()
where we cannot reliably determine whether a mapping was
created via a grant reference and so we must always
decrement the mapped page's reference count.

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

diff -r f2a08a5a807a -r 48eb10d7a2d6 xen/common/grant_table.c
--- a/xen/common/grant_table.c  Wed Dec 21 17:16:31 2005
+++ b/xen/common/grant_table.c  Wed Dec 21 17:25:34 2005
@@ -177,17 +177,19 @@
 
     spin_lock(&rd->grant_table->lock);
 
-    if ( act->pin == 0 )
-    {
-        /* CASE 1: Activating a previously inactive entry. */
-
+    if ( !act->pin ||
+         (!(dev_hst_ro_flags & GNTMAP_readonly) &&
+          !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask))) )
+    {
         sflags = sha->flags;
         sdom   = sha->domid;
 
-        /* This loop attempts to set the access (reading/writing) flags
+        /*
+         * This loop attempts to set the access (reading/writing) flags
          * in the grant table entry.  It tries a cmpxchg on the field
          * up to five times, and then fails under the assumption that 
-         * the guest is misbehaving. */
+         * the guest is misbehaving.
+         */
         for ( ; ; )
         {
             u32 scombo, prev_scombo, new_scombo;
@@ -196,7 +198,7 @@
                  unlikely(sdom != led->domain->domain_id) )
                 PIN_FAIL(unlock_out, GNTST_general_error,
                          "Bad flags (%x) or dom (%d). (NB. expected dom %d)\n",
-                        sflags, sdom, led->domain->domain_id);
+                         sflags, sdom, led->domain->domain_id);
 
             /* Merge two 16-bit values into a 32-bit combined update. */
             /* NB. Endianness! */
@@ -232,132 +234,50 @@
             sdom   = (u16)(prev_scombo >> 16);
         }
 
-        /* rmb(); */ /* not on x86 */
-
-        frame = __gpfn_to_mfn(rd, sha->frame);
-
-        if ( unlikely(!pfn_valid(frame)) ||
-             unlikely(!((dev_hst_ro_flags & GNTMAP_readonly) ?
-                        get_page(pfn_to_page(frame), rd) :
-                        get_page_and_type(pfn_to_page(frame), rd,
-                                          PGT_writable_page))) )
-        {
-            clear_bit(_GTF_writing, &sha->flags);
-            clear_bit(_GTF_reading, &sha->flags);
-            PIN_FAIL(unlock_out, GNTST_general_error,
-                     "Could not pin the granted frame (%lx)!\n", frame);
+        if ( !act->pin )
+        {
+            act->domid = sdom;
+            act->frame = __gpfn_to_mfn(rd, sha->frame);
+        }
+    }
+    else if ( (act->pin & 0x80808080U) != 0 )
+        PIN_FAIL(unlock_out, ENOSPC,
+                 "Risk of counter overflow %08x\n", act->pin);
+
+    if ( dev_hst_ro_flags & GNTMAP_device_map )
+        act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
+            GNTPIN_devr_inc : GNTPIN_devw_inc;
+    if ( dev_hst_ro_flags & GNTMAP_host_map )
+        act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
+            GNTPIN_hstr_inc : GNTPIN_hstw_inc;
+
+    spin_unlock(&rd->grant_table->lock);
+
+    frame = act->frame;
+    if ( unlikely(!pfn_valid(frame)) ||
+         unlikely(!((dev_hst_ro_flags & GNTMAP_readonly) ?
+                    get_page(pfn_to_page(frame), rd) :
+                    get_page_and_type(pfn_to_page(frame), rd,
+                                      PGT_writable_page))) )
+        PIN_FAIL(undo_out, GNTST_general_error,
+                 "Could not pin the granted frame (%lx)!\n", frame);
+
+    if ( dev_hst_ro_flags & GNTMAP_host_map )
+    {
+        rc = create_grant_host_mapping(addr, frame, dev_hst_ro_flags);
+        if ( rc != GNTST_okay )
+        {
+            if ( !(dev_hst_ro_flags & GNTMAP_readonly) )
+                put_page_type(pfn_to_page(frame));
+            put_page(pfn_to_page(frame));
+            goto undo_out;
         }
 
         if ( dev_hst_ro_flags & GNTMAP_device_map )
-            act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
-                GNTPIN_devr_inc : GNTPIN_devw_inc;
-        if ( dev_hst_ro_flags & GNTMAP_host_map )
-            act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
-                GNTPIN_hstr_inc : GNTPIN_hstw_inc;
-        act->domid = sdom;
-        act->frame = frame;
-    }
-    else 
-    {
-        /* CASE 2: Active modications to an already active entry. */
-
-        /*
-         * A cheesy check for possible pin-count overflow.
-         * A more accurate check cannot be done with a single comparison.
-         */
-        if ( (act->pin & 0x80808080U) != 0 )
-            PIN_FAIL(unlock_out, ENOSPC,
-                     "Risk of counter overflow %08x\n", act->pin);
-
-        sflags = sha->flags;
-        frame  = act->frame;
-
-        if ( !(dev_hst_ro_flags & GNTMAP_readonly) &&
-             !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
-        {
-            for ( ; ; )
-            {
-                u16 prev_sflags;
-                
-                if ( unlikely(sflags & GTF_readonly) )
-                    PIN_FAIL(unlock_out, GNTST_general_error,
-                             "Attempt to write-pin a r/o grant entry.\n");
-
-                prev_sflags = sflags;
-
-                /* NB. prev_sflags is updated in place to seen value. */
-                if ( unlikely(cmpxchg_user(&sha->flags, prev_sflags, 
-                                           prev_sflags | GTF_writing)) )
-                    PIN_FAIL(unlock_out, GNTST_general_error,
-                         "Fault while modifying shared flags.\n");
-
-                if ( likely(prev_sflags == sflags) )
-                    break;
-
-                if ( retries++ == 4 )
-                    PIN_FAIL(unlock_out, GNTST_general_error,
-                             "Shared grant entry is unstable.\n");
-
-                sflags = prev_sflags;
-            }
-
-            if ( unlikely(!get_page_type(pfn_to_page(frame),
-                                         PGT_writable_page)) )
-            {
-                clear_bit(_GTF_writing, &sha->flags);
-                PIN_FAIL(unlock_out, GNTST_general_error,
-                         "Attempt to write-pin a unwritable page.\n");
-            }
-        }
-
-        if ( dev_hst_ro_flags & GNTMAP_device_map )
-            act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ? 
-                GNTPIN_devr_inc : GNTPIN_devw_inc;
-
-        if ( dev_hst_ro_flags & GNTMAP_host_map )
-            act->pin += (dev_hst_ro_flags & GNTMAP_readonly) ?
-                GNTPIN_hstr_inc : GNTPIN_hstw_inc;
-    }
-
-    /*
-     * At this point:
-     * act->pin updated to reference count mappings.
-     * sha->flags updated to indicate to granting domain mapping done.
-     * frame contains the mfn.
-     */
-
-    spin_unlock(&rd->grant_table->lock);
-
-    if ( dev_hst_ro_flags & GNTMAP_host_map )
-    {
-        rc = create_grant_host_mapping(addr, frame, dev_hst_ro_flags);
-        if ( rc < 0 )
-        {
-            /* Failure: undo and abort. */
-
-            spin_lock(&rd->grant_table->lock);
-
-            if ( dev_hst_ro_flags & GNTMAP_readonly )
-            {
-                act->pin -= GNTPIN_hstr_inc;
-            }
-            else
-            {
-                act->pin -= GNTPIN_hstw_inc;
-                if ( (act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) == 0 )
-                {
-                    clear_bit(_GTF_writing, &sha->flags);
-                    put_page_type(pfn_to_page(frame));
-                }
-            }
-
-            if ( act->pin == 0 )
-            {
-                clear_bit(_GTF_reading, &sha->flags);
-                put_page(pfn_to_page(frame));
-            }
-
-            spin_unlock(&rd->grant_table->lock);
+        {
+            (void)get_page(pfn_to_page(frame), rd);
+            if ( !(dev_hst_ro_flags & GNTMAP_readonly) )
+                get_page_type(pfn_to_page(frame), PGT_writable_page);
         }
     }
 
@@ -375,6 +295,24 @@
     put_domain(rd);
     return rc;
 
+ undo_out:
+    spin_lock(&rd->grant_table->lock);
+
+    if ( dev_hst_ro_flags & GNTMAP_device_map )
+        act->pin -= (dev_hst_ro_flags & GNTMAP_readonly) ?
+            GNTPIN_devr_inc : GNTPIN_devw_inc;
+    if ( dev_hst_ro_flags & GNTMAP_host_map )
+        act->pin -= (dev_hst_ro_flags & GNTMAP_readonly) ?
+            GNTPIN_hstr_inc : GNTPIN_hstw_inc;
+
+    if ( !(dev_hst_ro_flags & GNTMAP_readonly) &&
+         !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
+        clear_bit(_GTF_writing, &sha->flags);
+
+    if ( !act->pin )
+        clear_bit(_GTF_reading, &sha->flags);
+
+    spin_unlock(&rd->grant_table->lock);
 
  unlock_out:
     spin_unlock(&rd->grant_table->lock);
@@ -465,27 +403,42 @@
             PIN_FAIL(unmap_out, GNTST_general_error,
                      "Bad frame number doesn't match gntref.\n");
         if ( flags & GNTMAP_device_map )
-            act->pin -= (flags & GNTMAP_readonly) ? GNTPIN_devr_inc
-                                                  : GNTPIN_devw_inc;
-
-        map->ref_and_flags &= ~GNTMAP_device_map;
-        (void)__put_user(0, &uop->dev_bus_addr);
-    }
-
-    if ( (addr != 0) &&
-         (flags & GNTMAP_host_map) &&
-         ((act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask)) > 0))
+        {
+            ASSERT(act->pin & (GNTPIN_devw_mask | GNTPIN_devr_mask));
+            map->ref_and_flags &= ~GNTMAP_device_map;
+            if ( flags & GNTMAP_readonly )
+            {
+                act->pin -= GNTPIN_devr_inc;
+                put_page(pfn_to_page(frame));
+            }
+            else
+            {
+                act->pin -= GNTPIN_devw_inc;
+                put_page_and_type(pfn_to_page(frame));
+            }
+        }
+    }
+
+    if ( (addr != 0) && (flags & GNTMAP_host_map) )
     {
         if ( (rc = destroy_grant_host_mapping(addr, frame, flags)) < 0 )
             goto unmap_out;
 
+        ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask));
         map->ref_and_flags &= ~GNTMAP_host_map;
-
-        act->pin -= (flags & GNTMAP_readonly) ? GNTPIN_hstr_inc
-                                              : GNTPIN_hstw_inc;
-    }
-
-    if ( (map->ref_and_flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0)
+        if ( flags & GNTMAP_readonly )
+        {
+            act->pin -= GNTPIN_hstr_inc;
+            put_page(pfn_to_page(frame));
+        }
+        else
+        {
+            act->pin -= GNTPIN_hstw_inc;
+            put_page_and_type(pfn_to_page(frame));
+        }
+    }
+
+    if ( (map->ref_and_flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0 )
     {
         map->ref_and_flags = 0;
         put_maptrack_handle(ld->grant_table, handle);
@@ -495,20 +448,12 @@
     if ( !(flags & GNTMAP_readonly) )
          gnttab_log_dirty(rd, frame);
 
-    /* If the last writable mapping has been removed, put_page_type */
     if ( ((act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0) &&
          !(flags & GNTMAP_readonly) )
-    {
         clear_bit(_GTF_writing, &sha->flags);
-        put_page_type(pfn_to_page(frame));
-    }
 
     if ( act->pin == 0 )
-    {
-        act->frame = 0xdeadbeef;
         clear_bit(_GTF_reading, &sha->flags);
-        put_page(pfn_to_page(frame));
-    }
 
  unmap_out:
     (void)__put_user(rc, &uop->status);
@@ -989,42 +934,40 @@
         {
             if ( map->ref_and_flags & GNTMAP_device_map )
             {
-                BUG_ON((act->pin & GNTPIN_devr_mask) == 0);
+                BUG_ON(!(act->pin & GNTPIN_devr_mask));
                 act->pin -= GNTPIN_devr_inc;
+                put_page(pfn_to_page(act->frame));
             }
 
             if ( map->ref_and_flags & GNTMAP_host_map )
             {
-                BUG_ON((act->pin & GNTPIN_hstr_mask) == 0);
+                BUG_ON(!(act->pin & GNTPIN_hstr_mask));
                 act->pin -= GNTPIN_hstr_inc;
+                put_page(pfn_to_page(act->frame));
             }
         }
         else
         {
             if ( map->ref_and_flags & GNTMAP_device_map )
             {
-                BUG_ON((act->pin & GNTPIN_devw_mask) == 0);
+                BUG_ON(!(act->pin & GNTPIN_devw_mask));
                 act->pin -= GNTPIN_devw_inc;
+                put_page_and_type(pfn_to_page(act->frame));
             }
 
             if ( map->ref_and_flags & GNTMAP_host_map )
             {
-                BUG_ON((act->pin & GNTPIN_hstw_mask) == 0);
+                BUG_ON(!(act->pin & GNTPIN_hstw_mask));
                 act->pin -= GNTPIN_hstw_inc;
+                put_page_and_type(pfn_to_page(act->frame));
             }
 
             if ( (act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0 )
-            {
                 clear_bit(_GTF_writing, &sha->flags);
-                put_page_type(pfn_to_page(act->frame));
-            }
         }
 
         if ( act->pin == 0 )
-        {
             clear_bit(_GTF_reading, &sha->flags);
-            put_page(pfn_to_page(act->frame));
-        }
 
         spin_unlock(&rd->grant_table->lock);
 

_______________________________________________
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®.