Re: [Xen-devel] [RFC 2/6] rangeset_destroy() refactoring

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@xxxxxxxxx]
> Sent: 16 February 2017 12:03
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: andrii_anisov@xxxxxxxx; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> jbeulich@xxxxxxxx; konrad.wilk@xxxxxxxxxx; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; sstabellini@xxxxxxxxxx; Tim (Xen.org)
> <tim@xxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>
> Subject: [RFC 2/6] rangeset_destroy() refactoring
> 
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
> 
> rangeset_destroy is made domain agnostic. The domain specific code
> is moved to common/domain.c:domain_rangeset_destroy().
> 
> It is still left a rangesets list functionality: rangeset_destroy()
> will remove itself from a list. If a spinlock is provided it will be
> held for list deletion operation. This would be reconsidered further.
> 

Maybe use the same scheme in patch #1 then and pass the lock, as well as the 
list_head, into rangeset_new().

  Paul

> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
> ---
>  xen/arch/x86/hvm/ioreq.c   |  2 +-
>  xen/arch/x86/mm/p2m.c      |  2 +-
>  xen/common/domain.c        |  5 +++++
>  xen/common/rangeset.c      | 15 ++++++++-------
>  xen/include/xen/domain.h   |  3 +++
>  xen/include/xen/rangeset.h |  9 ++++++---
>  6 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 6df191d..6ae5921 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -496,7 +496,7 @@ static void hvm_ioreq_server_free_rangesets(struct
> hvm_ioreq_server *s,
>          return;
> 
>      for ( i = 0; i < NR_IO_RANGE_TYPES; i++ )
> -        rangeset_destroy(s->range[i]);
> +        domain_rangeset_destroy(s->range[i], s->domain);
>  }
> 
>  static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s,
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 46301ad..d39c093 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -141,7 +141,7 @@ static void p2m_teardown_hostp2m(struct domain
> *d)
> 
>      if ( p2m )
>      {
> -        rangeset_destroy(p2m->logdirty_ranges);
> +        domain_rangeset_destroy(p2m->logdirty_ranges, d);
>          p2m_free_one(p2m);
>          d->arch.p2m = NULL;
>      }
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 1b9bc3c..f03a032 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1553,6 +1553,11 @@ struct rangeset *domain_rangeset_new(struct
> domain *d, char *name,
>      return r;
>  }
> 
> +void domain_rangeset_destroy(struct domain *d,
> +    struct rangeset *r)
> +{
> +    rangeset_destroy(r, &d->rangesets_lock);
> +}
> 
> 
>  /*
> diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
> index 478d232..1172950 100644
> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -354,19 +354,20 @@ struct rangeset *rangeset_new(char *name,
> unsigned int flags,
>  }
> 
>  void rangeset_destroy(
> -    struct rangeset *r)
> +    struct rangeset *r, spinlock_t *lock)
>  {
>      struct range *x;
> 
>      if ( r == NULL )
>          return;
> 
> -    if ( r->domain != NULL )
> -    {
> -        spin_lock(&r->domain->rangesets_lock);
> -        list_del(&r->rangeset_list);
> -        spin_unlock(&r->domain->rangesets_lock);
> -    }
> +    if ( lock )
> +        spin_lock(lock);
> +
> +    list_del(&r->rangeset_list);
> +
> +    if ( lock )
> +        spin_unlock(lock);
> 
>      while ( (x = first_range(r)) != NULL )
>          destroy_range(r, x);
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index cd62e6e..3d9c652 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -111,4 +111,7 @@ void vnuma_destroy(struct vnuma_info *vnuma);
>  struct rangeset *domain_rangeset_new(struct domain *d, char *name,
>                                       unsigned int flags);
> 
> +void domain_rangeset_destroy(struct domain *d,
> +    struct rangeset *r);
> +
>  #endif /* __XEN_DOMAIN_H__ */
> diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
> index 395ba62..deed54d 100644
> --- a/xen/include/xen/rangeset.h
> +++ b/xen/include/xen/rangeset.h
> @@ -14,6 +14,7 @@
> 
>  struct domain;
>  struct list_head;
> +struct spinlock;
>  struct rangeset;
> 
>  /*
> @@ -37,11 +38,13 @@ struct rangeset *rangeset_new(char *name,
> unsigned int flags,
>                                struct list_head **head);
> 
>  /*
> - * Destroy a rangeset. It is invalid to perform any operation on a rangeset 
> @r
> + * Destroy a rangeset. Rangeset will take an action to remove itself from a
> + * list. If a spinlock is provided it will be held during list deletion
> + * operation.
> + * It is invalid to perform any operation on a rangeset @r
>   * after calling rangeset_destroy(r).
>   */
> -void rangeset_destroy(
> -    struct rangeset *r);
> +void rangeset_destroy(struct rangeset *r, struct spinlock *lock);
> 
>  /*
>   * Set a limit on the number of ranges that may exist in set @r.
> --
> 2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel