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

[Xen-changelog] [xen-3.4-testing] Revert 19658:28a197617286 "Fix up the synchronisation around grant



# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1243864577 -3600
# Node ID 29d7e3522cc5e0efb898cf4eb0f95e75e53b7a2d
# Parent  18d0a3f3356f72506c2628f09586c2c44654be66
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-unstable changeset:   19694:2e83c670f680
xen-unstable date:        Mon Jun 01 14:55:32 2009 +0100
---
 xen/common/grant_table.c      |   89 +++++++++++++++++-------------------------
 xen/include/xen/grant_table.h |    2 
 2 files changed, 37 insertions(+), 54 deletions(-)

diff -r 18d0a3f3356f -r 29d7e3522cc5 xen/common/grant_table.c
--- a/xen/common/grant_table.c  Mon Jun 01 14:45:22 2009 +0100
+++ b/xen/common/grant_table.c  Mon Jun 01 14:56:17 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 18d0a3f3356f -r 29d7e3522cc5 xen/include/xen/grant_table.h
--- a/xen/include/xen/grant_table.h     Mon Jun 01 14:45:22 2009 +0100
+++ b/xen/include/xen/grant_table.h     Mon Jun 01 14:56:17 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


 


Rackspace

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