[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |