WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

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

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] Increment page reference count for every host/device
From: Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Wed, 21 Dec 2005 18:56:08 +0000
Delivery-date: Wed, 21 Dec 2005 18:59:16 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# 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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] Increment page reference count for every host/device, Xen patchbot -unstable <=