# 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
|
Home