evtchn: slightly defer lock acquire where possible

port_is_valid() and evtchn_from_port() are fine to use without holding
any locks. Accordingly acquire the per-domain lock slightly later in
evtchn_close() and evtchn_bind_vcpu(). Especially for the use by the
former (but there are pre-existing uses) add a comment about
port_is_valid()'s guarantees.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <julien@xen.org>

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 5479315aae1bdb99f8b48b9de3e8a77c4fe035f0..8a5f8bb9d43f491f820eade2adbfa50d4a911741 100644 (file)
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -606,17 +606,14 @@ int evtchn_close(struct domain *d1, int port1, bool guest)
     int            port2;
     long           rc = 0;
 
- again:
-    spin_lock(&d1->event_lock);
-
     if ( !port_is_valid(d1, port1) )
-    {
-        rc = -EINVAL;
-        goto out;
-    }
+        return -EINVAL;
 
     chn1 = evtchn_from_port(d1, port1);
 
+ again:
+    spin_lock(&d1->event_lock);
+
     /* Guest cannot close a Xen-attached event channel. */
     if ( unlikely(consumer_is_xen(chn1)) && guest )
     {
@@ -1041,16 +1038,13 @@ long evtchn_bind_vcpu(unsigned int port, unsigned int vcpu_id)
     if ( (v = domain_vcpu(d, vcpu_id)) == NULL )
         return -ENOENT;
 
-    spin_lock(&d->event_lock);
-
     if ( !port_is_valid(d, port) )
-    {
-        rc = -EINVAL;
-        goto out;
-    }
+        return -EINVAL;
 
     chn = evtchn_from_port(d, port);
 
+    spin_lock(&d->event_lock);
+
     /* Guest cannot re-bind a Xen-attached event channel. */
     if ( unlikely(consumer_is_xen(chn)) )
     {

diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
index a0a85cdda8992f1e10205f8bcbd0faf7be650dd1..a185a5b8758c45c4195ff21b5f99d5d8be6d45c4 100644 (file)
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -120,6 +120,16 @@ static inline void evtchn_read_unlock(struct evtchn *evtchn)
     read_unlock(&evtchn->lock);
 }
 
+/*
+ * While calling the function is okay without holding a suitable lock yet
+ * (see the comment ahead of struct evtchn_port_ops for which ones those
+ * are), for a dying domain it may start returning false at any point - see
+ * evtchn_destroy(). This is not a fundamental problem though, as the
+ * struct evtchn instance won't disappear (and will continue to hold valid
+ * data) until final cleanup of the domain, at which point the domain itself
+ * cannot be looked up anymore and hence calls here can't occur any longer
+ * in the first place.
+ */
 static inline bool_t port_is_valid(struct domain *d, unsigned int p)
 {
     if ( p >= read_atomic(&d->valid_evtchns) )