xen/vm-event: Expand vm_event_* spinlock macros and rename the lock

These serve no purpose, but to add to the congnitive load of following
the code.  Remove the level of indirection.

Furthermore, the lock protects all data in vm_event_domain, making
ring_lock a poor choice of name.

For vm_event_get_response() and vm_event_grab_slot(), fold the exit
paths to have a single unlock, as the compiler can't make this
optimisation itself.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 902e152e21d03f86dcdd58bf2b9dac7eff085981..db975e9eeb33dc2d03835b7b320a0d7d8dab5656 100644 (file)
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -35,10 +35,6 @@
 #define xen_rmb()  smp_rmb()
 #define xen_wmb()  smp_wmb()
 
-#define vm_event_ring_lock_init(_ved)  spin_lock_init(&(_ved)->ring_lock)
-#define vm_event_ring_lock(_ved)       spin_lock(&(_ved)->ring_lock)
-#define vm_event_ring_unlock(_ved)     spin_unlock(&(_ved)->ring_lock)
-
 static int vm_event_enable(
     struct domain *d,
     struct xen_domctl_vm_event_op *vec,
@@ -66,8 +62,8 @@ static int vm_event_enable(
     if ( ring_gfn == 0 )
         return -EOPNOTSUPP;
 
-    vm_event_ring_lock_init(*ved);
-    vm_event_ring_lock(*ved);
+    spin_lock_init(&(*ved)->lock);
+    spin_lock(&(*ved)->lock);
 
     rc = vm_event_init_domain(d);
 
@@ -101,13 +97,13 @@ static int vm_event_enable(
     /* Initialize the last-chance wait queue. */
     init_waitqueue_head(&(*ved)->wq);
 
-    vm_event_ring_unlock(*ved);
+    spin_unlock(&(*ved)->lock);
     return 0;
 
  err:
     destroy_ring_for_helper(&(*ved)->ring_page,
                             (*ved)->ring_pg_struct);
-    vm_event_ring_unlock(*ved);
+    spin_unlock(&(*ved)->lock);
     xfree(*ved);
     *ved = NULL;
 
@@ -200,11 +196,11 @@ static int vm_event_disable(struct domain *d, struct vm_event_domain **ved)
     {
         struct vcpu *v;
 
-        vm_event_ring_lock(*ved);
+        spin_lock(&(*ved)->lock);
 
         if ( !list_empty(&(*ved)->wq.list) )
         {
-            vm_event_ring_unlock(*ved);
+            spin_unlock(&(*ved)->lock);
             return -EBUSY;
         }
 
@@ -226,7 +222,7 @@ static int vm_event_disable(struct domain *d, struct vm_event_domain **ved)
 
         vm_event_cleanup_domain(d);
 
-        vm_event_ring_unlock(*ved);
+        spin_unlock(&(*ved)->lock);
     }
 
     xfree(*ved);
@@ -292,7 +288,7 @@ void vm_event_put_request(struct domain *d,
 
     req->version = VM_EVENT_INTERFACE_VERSION;
 
-    vm_event_ring_lock(ved);
+    spin_lock(&ved->lock);
 
     /* Due to the reservations, this step must succeed. */
     front_ring = &ved->front_ring;
@@ -319,7 +315,7 @@ void vm_event_put_request(struct domain *d,
         !atomic_read(&curr->vm_event_pause_count) )
         vm_event_mark_and_pause(curr, ved);
 
-    vm_event_ring_unlock(ved);
+    spin_unlock(&ved->lock);
 
     notify_via_xen_event_channel(d, ved->xen_port);
 }
@@ -329,17 +325,15 @@ static int vm_event_get_response(struct domain *d, struct vm_event_domain *ved,
 {
     vm_event_front_ring_t *front_ring;
     RING_IDX rsp_cons;
+    int rc = 0;
 
-    vm_event_ring_lock(ved);
+    spin_lock(&ved->lock);
 
     front_ring = &ved->front_ring;
     rsp_cons = front_ring->rsp_cons;
 
     if ( !RING_HAS_UNCONSUMED_RESPONSES(front_ring) )
-    {
-        vm_event_ring_unlock(ved);
-        return 0;
-    }
+        goto out;
 
     /* Copy response */
     memcpy(rsp, RING_GET_RESPONSE(front_ring, rsp_cons), sizeof(*rsp));
@@ -353,9 +347,12 @@ static int vm_event_get_response(struct domain *d, struct vm_event_domain *ved,
      * there may be additional space available in the ring. */
     vm_event_wake(d, ved);
 
-    vm_event_ring_unlock(ved);
+    rc = 1;
 
-    return 1;
+ out:
+    spin_unlock(&ved->lock);
+
+    return rc;
 }
 
 /*
@@ -455,35 +452,38 @@ void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved)
     if( !vm_event_check_ring(ved) )
         return;
 
-    vm_event_ring_lock(ved);
+    spin_lock(&ved->lock);
     vm_event_release_slot(d, ved);
-    vm_event_ring_unlock(ved);
+    spin_unlock(&ved->lock);
 }
 
 static int vm_event_grab_slot(struct vm_event_domain *ved, int foreign)
 {
     unsigned int avail_req;
+    int rc;
 
     if ( !ved->ring_page )
         return -EOPNOTSUPP;
 
-    vm_event_ring_lock(ved);
+    spin_lock(&ved->lock);
 
     avail_req = vm_event_ring_available(ved);
+
+    rc = -EBUSY;
     if ( avail_req == 0 )
-    {
-        vm_event_ring_unlock(ved);
-        return -EBUSY;
-    }
+        goto out;
 
     if ( !foreign )
         ved->target_producers++;
     else
         ved->foreign_producers++;
 
-    vm_event_ring_unlock(ved);
+    rc = 0;
 
-    return 0;
+ out:
+    spin_unlock(&ved->lock);
+
+    return rc;
 }
 
 /* Simple try_grab wrapper for use in the wait_event() macro. */

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 2201faca6b429f970bd26296a2589ebc14f3b73d..b9691fc96ec9abcc815c6aba084a5e1c9da062af 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -282,8 +282,7 @@ struct vcpu
 /* VM event */
 struct vm_event_domain
 {
-    /* ring lock */
-    spinlock_t ring_lock;
+    spinlock_t lock;
     /* The ring has 64 entries */
     unsigned char foreign_producers;
     unsigned char target_producers;