Re: [PATCHv1 3/6] evtchn: simplify port_is_valid()

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

On 09/06/15 15:59, David Vrabel wrote:
> By keeping a count of the number of currently valid event channels,
> port_is_valid() can be simplified.
>
> d->valid_evtchns can also be tested without holding d->event_lock which
> will be useful later on.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  xen/common/event_channel.c |    3 +++
>  xen/include/xen/event.h    |    4 +---
>  xen/include/xen/sched.h    |    5 +++--
>  3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index e4ade17..482c3ac 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -189,6 +189,8 @@ static int get_free_port(struct domain *d)
>          return -ENOMEM;
>      bucket_from_port(d, port) = chn;
>  
> +    atomic_add(EVTCHNS_PER_BUCKET, &d->valid_evtchns);
> +
>      return port;
>  }
>  
> @@ -1254,6 +1256,7 @@ int evtchn_init(struct domain *d)
>      d->evtchn = alloc_evtchn_bucket(d, 0);
>      if ( !d->evtchn )
>          return -ENOMEM;
> +    atomic_set(&d->valid_evtchns, EVTCHNS_PER_BUCKET);
>  
>      spin_lock_init_prof(d, event_lock);
>      if ( get_free_port(d) != 0 )
> diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
> index 690f865..77b4d31 100644
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -91,9 +91,7 @@ static inline bool_t port_is_valid(struct domain *d, unsigned int p)
>          return 0;
>      if ( !d->evtchn )
>          return 0;
> -    if ( p < EVTCHNS_PER_BUCKET )
> -        return 1;
> -    return group_from_port(d, p) != NULL && bucket_from_port(d, p) != NULL;
> +    return p < atomic_read(&d->valid_evtchns);
>  }
>  
>  static inline struct evtchn *evtchn_from_port(struct domain *d, unsigned int p)
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 80c6f62..fab2e08 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -336,8 +336,9 @@ struct domain
>      /* Event channel information. */
>      struct evtchn   *evtchn;                         /* first bucket only */
>      struct evtchn  **evtchn_group[NR_EVTCHN_GROUPS]; /* all other buckets */
> -    unsigned int     max_evtchns;
> -    unsigned int     max_evtchn_port;
> +    unsigned int     max_evtchns;     /* number supported by ABI */
> +    unsigned int     max_evtchn_port; /* max permitted port number */
> +    atomic_t         valid_evtchns;   /* number of allocated event channels */

atomic_t contains a signed integer.  You probably want a BUILD_BUG_ON()
if any ABI maximum value exceeds INT_MAX.

~Andrew

>      spinlock_t       event_lock;
>      const struct evtchn_port_ops *evtchn_port_ops;
>      struct evtchn_fifo_domain *evtchn_fifo;