[PATCHv6 4/5] gnttab: remove unnecessary grant table locks

[David Vrabel <david.vrabel@citrix.com>]

From: Malcolm Crossley <malcolm.crossley@citrix.com>

The grant table lock is not required to protect reads of the
table version, the active entry array, or the map track array.

This is safe because: a) the grant table version only changes once
from 0 to 1 or 2; b) the active entry array only grows; and c) the map
track array only grows.

Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 xen/common/grant_table.c      |   76 +++++++++--------------------------------
 xen/include/xen/grant_table.h |    8 ++---
 2 files changed, 21 insertions(+), 63 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index a5470c8..8f3d125 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -196,8 +196,6 @@ active_entry_acquire(struct grant_table *t, grant_ref_t e)
 {
     struct active_grant_entry *act;
 
-    ASSERT(spin_is_locked(&t->lock));
-
     act = &_active_entry(t, e);
     spin_lock(&act->lock);
 
@@ -647,7 +645,6 @@ __gnttab_map_grant_ref(
     }
 
     rgt = rd->grant_table;
-    spin_lock(&rgt->lock);
 
     if ( rgt->gt_version == 0 )
         PIN_FAIL(unlock_out, GNTST_general_error,
@@ -827,7 +824,6 @@ __gnttab_map_grant_ref(
     mt->flags = op->flags;
 
     active_entry_release(act);
-    spin_unlock(&rgt->lock);
 
     op->dev_bus_addr = (u64)frame << PAGE_SHIFT;
     op->handle       = handle;
@@ -869,7 +865,6 @@ __gnttab_map_grant_ref(
     active_entry_release(act);
 
  unlock_out:
-    spin_unlock(&rgt->lock);
     op->status = rc;
     put_maptrack_handle(lgt, handle);
     rcu_unlock_domain(rd);
@@ -919,18 +914,15 @@ __gnttab_unmap_common(
     }
 
     op->map = &maptrack_entry(lgt, op->handle);
-    spin_lock(&lgt->lock);
 
     if ( unlikely(!op->map->flags) )
     {
-        spin_unlock(&lgt->lock);
         gdprintk(XENLOG_INFO, "Zero flags for handle (%d).\n", op->handle);
         op->status = GNTST_bad_handle;
         return;
     }
 
     dom = op->map->domid;
-    spin_unlock(&lgt->lock);
 
     if ( unlikely((rd = rcu_lock_domain_by_id(dom)) == NULL) )
     {
@@ -952,7 +944,6 @@ __gnttab_unmap_common(
 
     rgt = rd->grant_table;
 
-    spin_lock(&rgt->lock);
     act = active_entry_acquire(rgt, op->map->ref);
 
     op->flags = op->map->flags;
@@ -1023,7 +1014,6 @@ __gnttab_unmap_common(
 
  act_release_out:
     active_entry_release(act);
-    spin_unlock(&rgt->lock);
 
     op->status = rc;
     rcu_unlock_domain(rd);
@@ -1055,7 +1045,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
     rcu_lock_domain(rd);
     rgt = rd->grant_table;
 
-    spin_lock(&rgt->lock);
     if ( rgt->gt_version == 0 )
         goto unlock_out;
 
@@ -1122,8 +1111,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
     active_entry_release(act);
     
  unlock_out:
-    spin_unlock(&rgt->lock);
-
     if ( put_handle )
     {
         op->map->flags = 0;
@@ -1277,7 +1264,7 @@ gnttab_populate_status_frames(struct domain *d, struct grant_table *gt,
     for ( i = nr_status_frames(gt); i < req_status_frames; i++ )
         gnttab_create_status_page(d, gt, i);
 
-    gt->nr_status_frames = req_status_frames;
+    atomic_set(&gt->nr_status_frames, req_status_frames);
 
     return 0;
 
@@ -1306,7 +1293,7 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt)
         free_xenheap_page(gt->status[i]);
         gt->status[i] = NULL;
     }
-    gt->nr_status_frames = 0;
+    atomic_set(&gt->nr_status_frames, 0);
 }
 
 /* 
@@ -1356,7 +1343,7 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
     /* Share the new shared frames with the recipient domain */
     for ( i = nr_grant_frames(gt); i < req_nr_frames; i++ )
         gnttab_create_shared_page(d, gt, i);
-    gt->nr_grant_frames = req_nr_frames;
+    atomic_set(&gt->nr_grant_frames, req_nr_frames);
 
     return 1;
 
@@ -1423,7 +1410,17 @@ gnttab_setup_table(
     }
 
     gt = d->grant_table;
-    spin_lock(&gt->lock);
+
+    /* Tracking of mapped foreign frames table */
+    if ( (gt->maptrack = xzalloc_array(struct grant_mapping *,
+                                       max_maptrack_frames * d->max_vcpus)) == NULL )
+        goto out2;
+    for_each_vcpu( d, v )
+    {
+        v->maptrack_head = MAPTRACK_TAIL;
+        v->maptrack_tail = MAPTRACK_TAIL;
+    }
+    gt->maptrack_pages = 0;
 
     if ( gt->gt_version == 0 )
         gt->gt_version = 1;
@@ -1437,7 +1434,7 @@ gnttab_setup_table(
                  "Expand grant table to %u failed. Current: %u Max: %u\n",
                  op.nr_frames, nr_grant_frames(gt), max_grant_frames);
         op.status = GNTST_general_error;
-        goto out3;
+        goto out2;
     }
  
     op.status = GNTST_okay;
@@ -1450,8 +1447,6 @@ gnttab_setup_table(
             op.status = GNTST_bad_virt_addr;
     }
 
- out3:
-    spin_unlock(&gt->lock);
  out2:
     rcu_unlock_domain(d);
  out1:
@@ -1525,8 +1520,6 @@ gnttab_prepare_for_transfer(
     union grant_combo   scombo, prev_scombo, new_scombo;
     int                 retries = 0;
 
-    spin_lock(&rgt->lock);
-
     if ( rgt->gt_version == 0 )
     {
         gdprintk(XENLOG_INFO,
@@ -1575,12 +1568,9 @@ gnttab_prepare_for_transfer(
 
         scombo = prev_scombo;
     }
-
-    spin_unlock(&rgt->lock);
     return 1;
 
  fail:
-    spin_unlock(&rgt->lock);
     return 0;
 }
 
@@ -1773,8 +1763,6 @@ gnttab_transfer(
         TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
 
         /* Tell the guest about its new page frame. */
-        spin_lock(&e->grant_table->lock);
-
         if ( e->grant_table->gt_version == 1 )
         {
             grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref);
@@ -1791,8 +1779,6 @@ gnttab_transfer(
         shared_entry_header(e->grant_table, gop.ref)->flags |=
             GTF_transfer_completed;
 
-        spin_unlock(&e->grant_table->lock);
-
         rcu_unlock_domain(e);
 
         gop.status = GNTST_okay;
@@ -1829,8 +1815,6 @@ __release_grant_for_copy(
     released_read = 0;
     released_write = 0;
 
-    spin_lock(&rgt->lock);
-
     act = active_entry_acquire(rgt, gref);
     sha = shared_entry_header(rgt, gref);
     r_frame = act->frame;
@@ -1871,7 +1855,6 @@ __release_grant_for_copy(
     }
 
     active_entry_release(act);
-    spin_unlock(&rgt->lock);
 
     if ( td != rd )
     {
@@ -1929,8 +1912,6 @@ __acquire_grant_for_copy(
 
     *page = NULL;
 
-    spin_lock(&rgt->lock);
-
     if ( rgt->gt_version == 0 )
         PIN_FAIL(gnt_unlock_out, GNTST_general_error,
                  "remote grant table not ready\n");
@@ -2005,19 +1986,16 @@ __acquire_grant_for_copy(
              * lock here and reacquire
              */
             active_entry_release(act);
-            spin_unlock(&rgt->lock);
 
             rc = __acquire_grant_for_copy(td, trans_gref, rd->domain_id,
                                           readonly, &grant_frame, page,
                                           &trans_page_off, &trans_length, 0);
 
-            spin_lock(&rgt->lock);
             act = active_entry_acquire(rgt, gref);
 
             if ( rc != GNTST_okay ) {
                 __fixup_status_for_copy_pin(act, status);
                 active_entry_release(act);
-                spin_unlock(&rgt->lock);
                 rcu_unlock_domain(td);
                 return rc;
             }
@@ -2031,7 +2009,6 @@ __acquire_grant_for_copy(
                 __fixup_status_for_copy_pin(act, status);
                 rcu_unlock_domain(td);
                 active_entry_release(act);
-                spin_unlock(&rgt->lock);
                 put_page(*page);
                 return __acquire_grant_for_copy(rd, gref, ldom, readonly,
                                                 frame, page, page_off, length,
@@ -2100,7 +2077,6 @@ __acquire_grant_for_copy(
     *frame = act->frame;
 
     active_entry_release(act);
-    spin_unlock(&rgt->lock);
     return rc;
  
  unlock_out_clear:
@@ -2115,8 +2091,6 @@ __acquire_grant_for_copy(
     active_entry_release(act);
 
  gnt_unlock_out:
-    spin_unlock(&rgt->lock);
-
     return rc;
 }
 
@@ -2432,7 +2406,6 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
     if ( gt->gt_version == op.version )
         goto out;
 
-    spin_lock(&gt->lock);
     /* Make sure that the grant table isn't currently in use when we
        change the version number, except for the first 8 entries which
        are allowed to be in use (xenstore/xenconsole keeps them mapped).
@@ -2642,8 +2615,6 @@ __gnttab_swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b)
          */
         return rc;
 
-    spin_lock(&gt->lock);
-
     /* Bounds check on the grant refs */
     if ( unlikely(ref_a >= nr_grant_entries(d->grant_table)))
         PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-a (%d).\n", ref_a);
@@ -2686,7 +2657,6 @@ out:
         active_entry_release(act_b);
     if ( act_a != NULL )
         active_entry_release(act_a);
-    spin_unlock(&gt->lock);
 
     rcu_unlock_domain(d);
 
@@ -2757,12 +2727,9 @@ static int __gnttab_cache_flush(gnttab_cache_flush_t *cflush,
 
     if ( d != owner )
     {
-        spin_lock(&owner->grant_table->lock);
-
         ret = grant_map_exists(d, owner->grant_table, mfn, ref_count);
         if ( ret != 0 )
         {
-            spin_unlock(&owner->grant_table->lock);
             rcu_unlock_domain(d);
             put_page(page);
             return ret;
@@ -2781,8 +2748,6 @@ static int __gnttab_cache_flush(gnttab_cache_flush_t *cflush,
     else
         ret = 0;
 
-    if ( d != owner )
-        spin_unlock(&owner->grant_table->lock);
     unmap_domain_page(v);
     put_page(page);
 
@@ -3003,7 +2968,7 @@ grant_table_create(
     /* Simple stuff. */
     spin_lock_init(&t->lock);
     spin_lock_init(&t->maptrack_lock);
-    t->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
+    atomic_set(&t->nr_grant_frames, INITIAL_NR_GRANT_FRAMES);
 
     /* Active grant table. */
     if ( (t->active = xzalloc_array(struct active_grant_entry *,
@@ -3050,7 +3015,7 @@ grant_table_create(
     for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
         gnttab_create_shared_page(d, t, i);
 
-    t->nr_status_frames = 0;
+    atomic_set(&t->nr_status_frames, 0);
 
     /* Okay, install the structure. */
     d->grant_table = t;
@@ -3111,8 +3076,6 @@ gnttab_release_mappings(
         }
 
         rgt = rd->grant_table;
-        spin_lock(&rgt->lock);
-
         act = active_entry_acquire(rgt, ref);
         sha = shared_entry_header(rgt, ref);
         if (rgt->gt_version == 1)
@@ -3172,7 +3135,6 @@ gnttab_release_mappings(
             gnttab_clear_flag(_GTF_reading, status);
 
         active_entry_release(act);
-        spin_unlock(&rgt->lock);
 
         rcu_unlock_domain(rd);
 
@@ -3224,8 +3186,6 @@ static void gnttab_usage_print(struct domain *rd)
     printk("      -------- active --------       -------- shared --------\n");
     printk("[ref] localdom mfn      pin          localdom gmfn     flags\n");
 
-    spin_lock(&gt->lock);
-
     if ( gt->gt_version == 0 )
         goto out;
 
@@ -3277,8 +3237,6 @@ static void gnttab_usage_print(struct domain *rd)
     }
 
  out:
-    spin_unlock(&gt->lock);
-
     if ( first )
         printk("grant-table for remote domain:%5d ... "
                "no active grant table entries\n", rd->domain_id);
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 8ff1883..d78c381 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -65,7 +65,7 @@ struct grant_mapping {
 /* Per-domain grant information. */
 struct grant_table {
     /* Table size. Number of frames shared with guest */
-    unsigned int          nr_grant_frames;
+    atomic_t nr_grant_frames;
     /* Shared grant table (see include/public/grant_table.h). */
     union {
         void **shared_raw;
@@ -73,7 +73,7 @@ struct grant_table {
         union grant_entry_v2 **shared_v2;
     };
     /* Number of grant status frames shared with guest (for version 2) */
-    unsigned int          nr_status_frames;
+    atomic_t nr_status_frames;
     /* State grant table (see include/public/grant_table.h). */
     grant_status_t       **status;
     /* Active grant table. */
@@ -114,13 +114,13 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames);
 /* Number of grant table frames. Caller must hold d's grant table lock. */
 static inline unsigned int nr_grant_frames(struct grant_table *gt)
 {
-    return gt->nr_grant_frames;
+    return atomic_read(&gt->nr_grant_frames);
 }
 
 /* Number of status grant table frames. Caller must hold d's gr. table lock.*/
 static inline unsigned int nr_status_frames(struct grant_table *gt)
 {
-    return gt->nr_status_frames;
+    return atomic_read(&gt->nr_status_frames);
 }
 
 #define GRANT_STATUS_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t))
-- 
1.7.10.4