Re: [PATCHv1 4/6] evtchn: use a per-event channel lock for sending events

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/06/15 15:59, David Vrabel wrote:
> When sending an event, use a new per-event channel lock to safely
> validate the event channel state.
>
> This new lock must be held when changing event channel state.
>
> To avoid having to take the remote event channel locks when sending to
> an interdomain event channel, the local and remote channel locks are
> both held when binding or closing an interdomain event channel.
>
> This significantly increases the number of events that can be sent
> from multiple VCPUs.
>
> But, struct evtchn increases in size, reducing the number that fit
> into a single page to 64 (instead of 128).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  xen/common/event_channel.c |   84 +++++++++++++++++++++++++++++++++++++-------
>  xen/include/xen/sched.h    |    1 +
>  2 files changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 482c3ac..71747d1 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -139,6 +139,7 @@ static struct evtchn *alloc_evtchn_bucket(struct domain *d, unsigned int port)
>              return NULL;
>          }
>          chn[i].port = port + i;
> +        spin_lock_init(&chn[i].lock);
>      }
>      return chn;
>  }
> @@ -228,11 +229,15 @@ static long evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
>      if ( rc )
>          goto out;
>  
> +    spin_lock(&chn->lock);
> +
>      chn->state = ECS_UNBOUND;
>      if ( (chn->u.unbound.remote_domid = alloc->remote_dom) == DOMID_SELF )
>          chn->u.unbound.remote_domid = current->domain->domain_id;
>      evtchn_port_init(d, chn);
>  
> +    spin_unlock(&chn->lock);
> +
>      alloc->port = port;
>  
>   out:
> @@ -243,6 +248,30 @@ static long evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
>  }
>  
>  
> +static void double_evtchn_lock(struct domain *ld, struct evtchn *lchn,
> +                               struct domain *rd, struct evtchn *rchn)
> +{
> +    if ( ld < rd || (ld == rd && lchn->port < rchn->port) )

ld < rd is undefined behaviour as they are not part of the same
singly-allocated object.  It would be better to choose order based on
domid, and looks like the grant code suffers the same issue.

~Andrew