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] [xen-unstable] Revert 19658:28a197617286 "Fix up the syn

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-unstable] Revert 19658:28a197617286 "Fix up the synchronisation around grant
From: Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Tue, 02 Jun 2009 10:45:21 -0700
Delivery-date: Tue, 02 Jun 2009 10:46:33 -0700
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/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/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 Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1243864532 -3600
# Node ID 2e83c670f6806fa2d1e769d0131115f4143a705d
# Parent  28c6c955998ce7cc710e8eeda0087c1066aaba02
Revert 19658:28a197617286 "Fix up the synchronisation around grant
table map track handles".

There is no race since the hypercall takes the
domain-lock. Furthermore removing locking from get_maptrack_handle()
races gnttab_setup_table().

Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
 xen/common/grant_table.c      |   89 +++++++++++++++++-------------------------
 xen/include/xen/grant_table.h |    2 
 2 files changed, 37 insertions(+), 54 deletions(-)

diff -r 28c6c955998c -r 2e83c670f680 xen/common/grant_table.c
--- a/xen/common/grant_table.c  Mon Jun 01 14:39:25 2009 +0100
+++ b/xen/common/grant_table.c  Mon Jun 01 14:55:32 2009 +0100
@@ -111,26 +111,6 @@ static unsigned inline int max_nr_maptra
 #define active_entry(t, e) \
     ((t)->active[(e)/ACGNT_PER_PAGE][(e)%ACGNT_PER_PAGE])
 
-/* Technically, we only really need to acquire the lock for SMP
-   guests, because you only ever touch the maptrack tables from the
-   context of the guest which owns them, so if it's uniproc then the
-   lock can't be contended, and is therefore pointless.  Don't bother
-   with that optimisation for now, though, because it's scary and
-   confusing. */
-/* The maptrack lock is top-level: you're not allowed to be holding
-   any other locks when you acquire it. */
-static void
-maptrack_lock(struct grant_table *lgt)
-{
-    spin_lock(&lgt->maptrack_lock);
-}
-
-static void
-maptrack_unlock(struct grant_table *lgt)
-{
-    spin_unlock(&lgt->maptrack_lock);
-}
-
 static inline int
 __get_maptrack_handle(
     struct grant_table *t)
@@ -161,30 +141,43 @@ get_maptrack_handle(
 
     if ( unlikely((handle = __get_maptrack_handle(lgt)) == -1) )
     {
-        nr_frames = nr_maptrack_frames(lgt);
-        if ( nr_frames >= max_nr_maptrack_frames() )
-            return -1;
-
-        new_mt = alloc_xenheap_page();
-        if ( new_mt == NULL )
-            return -1;
-
-        clear_page(new_mt);
-
-        new_mt_limit = lgt->maptrack_limit + MAPTRACK_PER_PAGE;
-
-        for ( i = lgt->maptrack_limit; i < new_mt_limit; i++ )
-        {
-            new_mt[i % MAPTRACK_PER_PAGE].ref = i+1;
-            new_mt[i % MAPTRACK_PER_PAGE].flags = 0;
-        }
-
-        lgt->maptrack[nr_frames] = new_mt;
-        lgt->maptrack_limit      = new_mt_limit;
-
-        gdprintk(XENLOG_INFO,
-                 "Increased maptrack size to %u frames.\n", nr_frames + 1);
-        handle = __get_maptrack_handle(lgt);
+        spin_lock(&lgt->lock);
+
+        if ( unlikely((handle = __get_maptrack_handle(lgt)) == -1) )
+        {
+            nr_frames = nr_maptrack_frames(lgt);
+            if ( nr_frames >= max_nr_maptrack_frames() )
+            {
+                spin_unlock(&lgt->lock);
+                return -1;
+            }
+
+            new_mt = alloc_xenheap_page();
+            if ( new_mt == NULL )
+            {
+                spin_unlock(&lgt->lock);
+                return -1;
+            }
+
+            clear_page(new_mt);
+
+            new_mt_limit = lgt->maptrack_limit + MAPTRACK_PER_PAGE;
+
+            for ( i = lgt->maptrack_limit; i < new_mt_limit; i++ )
+            {
+                new_mt[i % MAPTRACK_PER_PAGE].ref = i+1;
+                new_mt[i % MAPTRACK_PER_PAGE].flags = 0;
+            }
+
+            lgt->maptrack[nr_frames] = new_mt;
+            lgt->maptrack_limit      = new_mt_limit;
+
+            gdprintk(XENLOG_INFO,
+                    "Increased maptrack size to %u frames.\n", nr_frames + 1);
+            handle = __get_maptrack_handle(lgt);
+        }
+
+        spin_unlock(&lgt->lock);
     }
     return handle;
 }
@@ -1515,9 +1508,7 @@ do_grant_table_op(
             guest_handle_cast(uop, gnttab_map_grant_ref_t);
         if ( unlikely(!guest_handle_okay(map, count)) )
             goto out;
-        maptrack_lock(current->domain->grant_table);
         rc = gnttab_map_grant_ref(map, count);
-        maptrack_unlock(current->domain->grant_table);
         break;
     }
     case GNTTABOP_unmap_grant_ref:
@@ -1526,9 +1517,7 @@ do_grant_table_op(
             guest_handle_cast(uop, gnttab_unmap_grant_ref_t);
         if ( unlikely(!guest_handle_okay(unmap, count)) )
             goto out;
-        maptrack_lock(current->domain->grant_table);
         rc = gnttab_unmap_grant_ref(unmap, count);
-        maptrack_unlock(current->domain->grant_table);
         break;
     }
     case GNTTABOP_unmap_and_replace:
@@ -1540,9 +1529,7 @@ do_grant_table_op(
         rc = -ENOSYS;
         if ( unlikely(!replace_grant_supported()) )
             goto out;
-        maptrack_lock(current->domain->grant_table);
         rc = gnttab_unmap_and_replace(unmap, count);
-        maptrack_unlock(current->domain->grant_table);
         break;
     }
     case GNTTABOP_setup_table:
@@ -1613,7 +1600,6 @@ grant_table_create(
     /* Simple stuff. */
     memset(t, 0, sizeof(*t));
     spin_lock_init(&t->lock);
-    spin_lock_init(&t->maptrack_lock);
     t->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
 
     /* Active grant table. */
@@ -1694,7 +1680,6 @@ gnttab_release_mappings(
 
     for ( handle = 0; handle < gt->maptrack_limit; handle++ )
     {
-        /* Domain is dying, so don't need maptrack lock */
         map = &maptrack_entry(gt, handle);
         if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) )
             continue;
diff -r 28c6c955998c -r 2e83c670f680 xen/include/xen/grant_table.h
--- a/xen/include/xen/grant_table.h     Mon Jun 01 14:39:25 2009 +0100
+++ b/xen/include/xen/grant_table.h     Mon Jun 01 14:55:32 2009 +0100
@@ -91,8 +91,6 @@ struct grant_table {
     struct grant_mapping **maptrack;
     unsigned int          maptrack_head;
     unsigned int          maptrack_limit;
-    /* Lock protecting maptrack-related fields. */
-    spinlock_t            maptrack_lock;
     /* Lock protecting updates to active and shared grant tables. */
     spinlock_t            lock;
 };

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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-unstable] Revert 19658:28a197617286 "Fix up the synchronisation around grant, Xen patchbot-unstable <=