[RFC 0/6] Rangeset generalisation

[RFC 0/6] Rangeset generalisation

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Rangesets in XEN seems to be a pretty generic thing slightly poisoned with.
domain specific funtionality in initialization and deinitialization code.

So make the rangeset code generic with moving domain specific code to
common/domain.c

Andrii Anisov (6):
  rangeset_new() refactoring
  rangeset_destroy() refactoring
  Drop rangeset_domain_initialise()
  rangeset_domain_destroy() refactoring
  rangeset_domain_printk() refactoring
  Drop domain remains from rangeset

 xen/arch/x86/domain.c      |  2 +-
 xen/arch/x86/hvm/ioreq.c   |  6 ++---
 xen/arch/x86/mm/p2m.c      |  6 ++---
 xen/arch/x86/setup.c       |  4 +--
 xen/common/domain.c        | 48 +++++++++++++++++++++++++++++++----
 xen/common/keyhandler.c    |  2 +-
 xen/common/rangeset.c      | 63 +++++++++++++++-------------------------------
 xen/include/xen/domain.h   |  9 +++++++
 xen/include/xen/rangeset.h | 42 +++++++++++++++----------------
 9 files changed, 102 insertions(+), 80 deletions(-)

-- 
2.7.4


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

[RFC 1/6] rangeset_new() refactoring

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

rangeset_new is made domain agnostic. The domain specific code
is moved to common/domain.c:domain_rangeset_new().

It is still left a rangesets list functionality: rangeset_new() is
returning its list head if requested. This would be reconsidered
further.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/arch/x86/domain.c      |  2 +-
 xen/arch/x86/hvm/ioreq.c   |  4 ++--
 xen/arch/x86/mm/p2m.c      |  4 ++--
 xen/arch/x86/setup.c       |  4 ++--
 xen/common/domain.c        | 22 ++++++++++++++++++++--
 xen/common/rangeset.c      | 12 ++++--------
 xen/include/xen/domain.h   |  3 +++
 xen/include/xen/rangeset.h | 19 +++++++++++--------
 8 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index eae643f..93f18d7 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -630,7 +630,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
         d->arch.x86_model  = boot_cpu_data.x86_model;
 
         d->arch.ioport_caps = 
-            rangeset_new(d, "I/O Ports", RANGESETF_prettyprint_hex);
+            domain_rangeset_new(d, "I/O Ports", RANGESETF_prettyprint_hex);
         rc = -ENOMEM;
         if ( d->arch.ioport_caps == NULL )
             goto fail;
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 88071ab..6df191d 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -520,8 +520,8 @@ static int hvm_ioreq_server_alloc_rangesets(struct hvm_ioreq_server *s,
         if ( rc )
             goto fail;
 
-        s->range[i] = rangeset_new(s->domain, name,
-                                   RANGESETF_prettyprint_hex);
+        s->range[i] = domain_rangeset_new(s->domain, name,
+                                          RANGESETF_prettyprint_hex);
 
         xfree(name);
 
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 6a45185..46301ad 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -122,8 +122,8 @@ static int p2m_init_hostp2m(struct domain *d)
 
     if ( p2m )
     {
-        p2m->logdirty_ranges = rangeset_new(d, "log-dirty",
-                                            RANGESETF_prettyprint_hex);
+        p2m->logdirty_ranges = domain_rangeset_new(d, "log-dirty",
+                                                   RANGESETF_prettyprint_hex);
         if ( p2m->logdirty_ranges )
         {
             d->arch.p2m = p2m;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b130671..69a961c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1419,8 +1419,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     /* Low mappings were only needed for some BIOS table parsing. */
     zap_low_mappings();
 
-    mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
-                                  RANGESETF_prettyprint_hex);
+    mmio_ro_ranges = rangeset_new("r/o mmio ranges",
+                                  RANGESETF_prettyprint_hex, NULL);
 
     init_apic_mappings();
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 3abaca9..1b9bc3c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -329,8 +329,8 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
     rangeset_domain_initialise(d);
     init_status |= INIT_rangeset;
 
-    d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
-    d->irq_caps   = rangeset_new(d, "Interrupts", 0);
+    d->iomem_caps = domain_rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
+    d->irq_caps   = domain_rangeset_new(d, "Interrupts", 0);
     if ( (d->iomem_caps == NULL) || (d->irq_caps == NULL) )
         goto fail;
 
@@ -1537,6 +1537,24 @@ int continue_hypercall_on_cpu(
     return 0;
 }
 
+struct rangeset *domain_rangeset_new(struct domain *d, char *name,
+                                     unsigned int flags)
+{
+    struct list_head *head;
+    struct rangeset *r = rangeset_new(name, flags, &head);
+
+    if ( d != NULL )
+    {
+        spin_lock(&d->rangesets_lock);
+        list_add(head, &d->rangesets);
+        spin_unlock(&d->rangesets_lock);
+    }
+
+    return r;
+}
+
+
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 6c6293c..478d232 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -322,8 +322,8 @@ bool_t rangeset_is_empty(
     return ((r == NULL) || list_empty(&r->range_list));
 }
 
-struct rangeset *rangeset_new(
-    struct domain *d, char *name, unsigned int flags)
+struct rangeset *rangeset_new(char *name, unsigned int flags,
+                              struct list_head **head)
 {
     struct rangeset *r;
 
@@ -347,12 +347,8 @@ struct rangeset *rangeset_new(
         snprintf(r->name, sizeof(r->name), "(no name)");
     }
 
-    if ( (r->domain = d) != NULL )
-    {
-        spin_lock(&d->rangesets_lock);
-        list_add(&r->rangeset_list, &d->rangesets);
-        spin_unlock(&d->rangesets_lock);
-    }
+    if (head)
+        *head = &r->rangeset_list;
 
     return r;
 }
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index bce0ea1..cd62e6e 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -108,4 +108,7 @@ struct vnuma_info {
 
 void vnuma_destroy(struct vnuma_info *vnuma);
 
+struct rangeset *domain_rangeset_new(struct domain *d, char *name,
+                                     unsigned int flags);
+
 #endif /* __XEN_DOMAIN_H__ */
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index aa64082..395ba62 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -13,6 +13,7 @@
 #include <xen/types.h>
 
 struct domain;
+struct list_head;
 struct rangeset;
 
 /*
@@ -28,15 +29,17 @@ void rangeset_domain_destroy(
     struct domain *d);
 
 /*
- * Create/destroy a rangeset. Optionally attach to specified domain @d for
- * auto-destruction when the domain dies. A name may be specified, for use
- * in debug pretty-printing, and various RANGESETF flags (defined below).
- * 
- * It is invalid to perform any operation on a rangeset @r after calling
- * rangeset_destroy(r).
+ * Create a rangeset. Optionally attach to a specified list @head.
+ * A name may be specified, for use in debug pretty-printing, and various
+ * RANGESETF flags (defined below).
+ */
+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
+ * after calling rangeset_destroy(r).
  */
-struct rangeset *rangeset_new(
-    struct domain *d, char *name, unsigned int flags);
 void rangeset_destroy(
     struct rangeset *r);
 
-- 
2.7.4


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

Re: [RFC 1/6] rangeset_new() refactoring

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 12:03
> To: xen-devel@lists.xenproject.org
> Cc: andrii_anisov@epam.com; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; Paul Durrant
> <Paul.Durrant@citrix.com>; sstabellini@kernel.org; Tim (Xen.org)
> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: [RFC 1/6] rangeset_new() refactoring
> 
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> rangeset_new is made domain agnostic. The domain specific code
> is moved to common/domain.c:domain_rangeset_new().
> 
> It is still left a rangesets list functionality: rangeset_new() is
> returning its list head if requested. This would be reconsidered
> further.
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> ---
>  xen/arch/x86/domain.c      |  2 +-
>  xen/arch/x86/hvm/ioreq.c   |  4 ++--
>  xen/arch/x86/mm/p2m.c      |  4 ++--
>  xen/arch/x86/setup.c       |  4 ++--
>  xen/common/domain.c        | 22 ++++++++++++++++++++--
>  xen/common/rangeset.c      | 12 ++++--------
>  xen/include/xen/domain.h   |  3 +++
>  xen/include/xen/rangeset.h | 19 +++++++++++--------
>  8 files changed, 45 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index eae643f..93f18d7 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -630,7 +630,7 @@ int arch_domain_create(struct domain *d, unsigned
> int domcr_flags,
>          d->arch.x86_model  = boot_cpu_data.x86_model;
> 
>          d->arch.ioport_caps =
> -            rangeset_new(d, "I/O Ports", RANGESETF_prettyprint_hex);
> +            domain_rangeset_new(d, "I/O Ports", RANGESETF_prettyprint_hex);
>          rc = -ENOMEM;
>          if ( d->arch.ioport_caps == NULL )
>              goto fail;
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 88071ab..6df191d 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -520,8 +520,8 @@ static int hvm_ioreq_server_alloc_rangesets(struct
> hvm_ioreq_server *s,
>          if ( rc )
>              goto fail;
> 
> -        s->range[i] = rangeset_new(s->domain, name,
> -                                   RANGESETF_prettyprint_hex);
> +        s->range[i] = domain_rangeset_new(s->domain, name,
> +                                          RANGESETF_prettyprint_hex);
> 
>          xfree(name);
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 6a45185..46301ad 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -122,8 +122,8 @@ static int p2m_init_hostp2m(struct domain *d)
> 
>      if ( p2m )
>      {
> -        p2m->logdirty_ranges = rangeset_new(d, "log-dirty",
> -                                            RANGESETF_prettyprint_hex);
> +        p2m->logdirty_ranges = domain_rangeset_new(d, "log-dirty",
> +                                                   RANGESETF_prettyprint_hex);
>          if ( p2m->logdirty_ranges )
>          {
>              d->arch.p2m = p2m;
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index b130671..69a961c 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1419,8 +1419,8 @@ void __init noreturn __start_xen(unsigned long
> mbi_p)
>      /* Low mappings were only needed for some BIOS table parsing. */
>      zap_low_mappings();
> 
> -    mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
> -                                  RANGESETF_prettyprint_hex);
> +    mmio_ro_ranges = rangeset_new("r/o mmio ranges",
> +                                  RANGESETF_prettyprint_hex, NULL);
> 
>      init_apic_mappings();
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 3abaca9..1b9bc3c 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -329,8 +329,8 @@ struct domain *domain_create(domid_t domid,
> unsigned int domcr_flags,
>      rangeset_domain_initialise(d);
>      init_status |= INIT_rangeset;
> 
> -    d->iomem_caps = rangeset_new(d, "I/O Memory",
> RANGESETF_prettyprint_hex);
> -    d->irq_caps   = rangeset_new(d, "Interrupts", 0);
> +    d->iomem_caps = domain_rangeset_new(d, "I/O Memory",
> RANGESETF_prettyprint_hex);
> +    d->irq_caps   = domain_rangeset_new(d, "Interrupts", 0);
>      if ( (d->iomem_caps == NULL) || (d->irq_caps == NULL) )
>          goto fail;
> 
> @@ -1537,6 +1537,24 @@ int continue_hypercall_on_cpu(
>      return 0;
>  }
> 
> +struct rangeset *domain_rangeset_new(struct domain *d, char *name,
> +                                     unsigned int flags)
> +{
> +    struct list_head *head;
> +    struct rangeset *r = rangeset_new(name, flags, &head);
> +
> +    if ( d != NULL )

Shouldn't d == NULL be a hard error now, in which you could pass d->rangesets directly into rangeset_new() (under lock).

  Paul

> +    {
> +        spin_lock(&d->rangesets_lock);
> +        list_add(head, &d->rangesets);
> +        spin_unlock(&d->rangesets_lock);
> +    }
> +
> +    return r;
> +}
> +
> +
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
> index 6c6293c..478d232 100644
> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -322,8 +322,8 @@ bool_t rangeset_is_empty(
>      return ((r == NULL) || list_empty(&r->range_list));
>  }
> 
> -struct rangeset *rangeset_new(
> -    struct domain *d, char *name, unsigned int flags)
> +struct rangeset *rangeset_new(char *name, unsigned int flags,
> +                              struct list_head **head)
>  {
>      struct rangeset *r;
> 
> @@ -347,12 +347,8 @@ struct rangeset *rangeset_new(
>          snprintf(r->name, sizeof(r->name), "(no name)");
>      }
> 
> -    if ( (r->domain = d) != NULL )
> -    {
> -        spin_lock(&d->rangesets_lock);
> -        list_add(&r->rangeset_list, &d->rangesets);
> -        spin_unlock(&d->rangesets_lock);
> -    }
> +    if (head)
> +        *head = &r->rangeset_list;
> 
>      return r;
>  }
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index bce0ea1..cd62e6e 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -108,4 +108,7 @@ struct vnuma_info {
> 
>  void vnuma_destroy(struct vnuma_info *vnuma);
> 
> +struct rangeset *domain_rangeset_new(struct domain *d, char *name,
> +                                     unsigned int flags);
> +
>  #endif /* __XEN_DOMAIN_H__ */
> diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
> index aa64082..395ba62 100644
> --- a/xen/include/xen/rangeset.h
> +++ b/xen/include/xen/rangeset.h
> @@ -13,6 +13,7 @@
>  #include <xen/types.h>
> 
>  struct domain;
> +struct list_head;
>  struct rangeset;
> 
>  /*
> @@ -28,15 +29,17 @@ void rangeset_domain_destroy(
>      struct domain *d);
> 
>  /*
> - * Create/destroy a rangeset. Optionally attach to specified domain @d for
> - * auto-destruction when the domain dies. A name may be specified, for
> use
> - * in debug pretty-printing, and various RANGESETF flags (defined below).
> - *
> - * It is invalid to perform any operation on a rangeset @r after calling
> - * rangeset_destroy(r).
> + * Create a rangeset. Optionally attach to a specified list @head.
> + * A name may be specified, for use in debug pretty-printing, and various
> + * RANGESETF flags (defined below).
> + */
> +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
> + * after calling rangeset_destroy(r).
>   */
> -struct rangeset *rangeset_new(
> -    struct domain *d, char *name, unsigned int flags);
>  void rangeset_destroy(
>      struct rangeset *r);
> 
> --
> 2.7.4


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

Re: [RFC 1/6] rangeset_new() refactoring

[Andrii Anisov]

>> +    if ( d != NULL )
>
> Shouldn't d == NULL be a hard error now, in which you could pass d->rangesets directly into rangeset_new() (under lock).

It definitely should. Because this is a domain specific code.

Sincerely,
Andrii Anisov.

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

[RFC 2/6] rangeset_destroy() refactoring

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

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.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 12:03
> To: xen-devel@lists.xenproject.org
> Cc: andrii_anisov@epam.com; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; Paul Durrant
> <Paul.Durrant@citrix.com>; sstabellini@kernel.org; Tim (Xen.org)
> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: [RFC 2/6] rangeset_destroy() refactoring
> 
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> 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@epam.com>
> ---
>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Andrii Anisov]

Dear Paul,

>> 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().
Initialized list head in patch#1 is provided by rangeset to user if
user needs it to link to some list. So user should do the locking on
tree insert if it needs. Here rangeset user can not acquire the tree
head even if it has a rangeset itself, because `struct rangeset` is
not exposed via header file. So rangeset_destroy() should take care of
tree head remove and spinlock locking if needed.

I still have concerns why rangeset list keeping should be inside
rangeset itself.

Sincerely,
Andrii Anisov.


2017-02-16 14:26 GMT+02:00 Paul Durrant <Paul.Durrant@citrix.com>:
>> -----Original Message-----
>> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
>> Sent: 16 February 2017 12:03
>> To: xen-devel@lists.xenproject.org
>> Cc: andrii_anisov@epam.com; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; George Dunlap
>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
>> jbeulich@suse.com; konrad.wilk@oracle.com; Paul Durrant
>> <Paul.Durrant@citrix.com>; sstabellini@kernel.org; Tim (Xen.org)
>> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
>> Subject: [RFC 2/6] rangeset_destroy() refactoring
>>
>> From: Andrii Anisov <andrii_anisov@epam.com>
>>
>> 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@epam.com>
>> ---
>>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 14:26
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; andrii_anisov@epam.com; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; sstabellini@kernel.org; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [RFC 2/6] rangeset_destroy() refactoring
> 
> Dear Paul,
> 
> >> 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().
> Initialized list head in patch#1 is provided by rangeset to user if
> user needs it to link to some list. So user should do the locking on
> tree insert if it needs. Here rangeset user can not acquire the tree
> head even if it has a rangeset itself, because `struct rangeset` is
> not exposed via header file. So rangeset_destroy() should take care of
> tree head remove and spinlock locking if needed.
> 
> I still have concerns why rangeset list keeping should be inside
> rangeset itself.

What use are rangesets if the implementation doesn't control the list/tree? How on earth would you implement an allocation function otherwise?

  Paul

> 
> Sincerely,
> Andrii Anisov.
> 
> 
> 2017-02-16 14:26 GMT+02:00 Paul Durrant <Paul.Durrant@citrix.com>:
> >> -----Original Message-----
> >> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> >> Sent: 16 February 2017 12:03
> >> To: xen-devel@lists.xenproject.org
> >> Cc: andrii_anisov@epam.com; Andrew Cooper
> >> <Andrew.Cooper3@citrix.com>; George Dunlap
> >> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> >> jbeulich@suse.com; konrad.wilk@oracle.com; Paul Durrant
> >> <Paul.Durrant@citrix.com>; sstabellini@kernel.org; Tim (Xen.org)
> >> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> >> Subject: [RFC 2/6] rangeset_destroy() refactoring
> >>
> >> From: Andrii Anisov <andrii_anisov@epam.com>
> >>
> >> 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@epam.com>
> >> ---
> >>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Andrii Anisov]

> What use are rangesets if the implementation doesn't control the list/tree? How on earth would you implement an allocation function otherwise?
Just to be on the same page, my understanding of the rangesets is as following:

 - Currently the `struct rangeset` is a list of `ranges`. This list
head is a `range_list` of `struct rangeset`. Currently `range_list`
manipulations are not protected by any locks. IMO this is the core
functionality of the rangeset.

 - Also there is another list head `rangeset_list` inside `struct
rangeset`. It is used to link a rangeset to an external list of
rangesets. This is protected by spinlocks now. IMO this functionality
is odd to the rangeset itself.

Sincerely,
Andrii Anisov.

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

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

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 16:22
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; andrii_anisov@epam.com; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; sstabellini@kernel.org; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [RFC 2/6] rangeset_destroy() refactoring
> 
> > What use are rangesets if the implementation doesn't control the list/tree?
> How on earth would you implement an allocation function otherwise?
> Just to be on the same page, my understanding of the rangesets is as
> following:
> 
>  - Currently the `struct rangeset` is a list of `ranges`. This list
> head is a `range_list` of `struct rangeset`. Currently `range_list`
> manipulations are not protected by any locks. IMO this is the core
> functionality of the rangeset.
> 
>  - Also there is another list head `rangeset_list` inside `struct
> rangeset`. It is used to link a rangeset to an external list of
> rangesets. This is protected by spinlocks now. IMO this functionality
> is odd to the rangeset itself.

Ok. Thanks for the clarification. Yes, that second list_head does not strictly belong inside the rangeset structure itself. I guess it could live in a 'domain_rangeset' wrapper structure.

  Paul

> 
> Sincerely,
> Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[RFC 3/6] Drop rangeset_domain_initialise()

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

The rangeset_domain_initialise() does nothing rangeset specific.
Even keeping it specific to domain looks not reasonable. So drop it.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/common/domain.c        | 3 ++-
 xen/common/rangeset.c      | 7 -------
 xen/include/xen/rangeset.h | 4 +---
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index f03a032..7fe69c6 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -326,7 +326,8 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
         d->disable_migrate = 1;
     }
 
-    rangeset_domain_initialise(d);
+    INIT_LIST_HEAD(&d->rangesets);
+    spin_lock_init(&d->rangesets_lock);
     init_status |= INIT_rangeset;
 
     d->iomem_caps = domain_rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 1172950..1a13a32 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -381,13 +381,6 @@ void rangeset_limit(
     r->nr_ranges = limit;
 }
 
-void rangeset_domain_initialise(
-    struct domain *d)
-{
-    INIT_LIST_HEAD(&d->rangesets);
-    spin_lock_init(&d->rangesets_lock);
-}
-
 void rangeset_domain_destroy(
     struct domain *d)
 {
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index deed54d..e8244a0 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -18,14 +18,12 @@ struct spinlock;
 struct rangeset;
 
 /*
- * Initialise/destroy per-domain rangeset information.
+ * Destroy per-domain rangeset information.
  * 
  * It is invalid to create or destroy a rangeset belonging to a domain @d
  * before rangeset_domain_initialise(d) returns or after calling
  * rangeset_domain_destroy(d).
  */
-void rangeset_domain_initialise(
-    struct domain *d);
 void rangeset_domain_destroy(
     struct domain *d);
 
-- 
2.7.4


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

[RFC 4/6] rangeset_domain_destroy() refactoring

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

rangeset_domain_destroy() is rather rangeset list helper and does nothing really
domain specific. So replace it with rangeset_list_destroy() helper.

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/common/domain.c        |  4 ++--
 xen/common/rangeset.c      | 11 ++++-------
 xen/include/xen/rangeset.h |  9 ++-------
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 7fe69c6..47c45f2 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -420,7 +420,7 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
         radix_tree_destroy(&d->pirq_tree, free_pirq_struct);
     }
     if ( init_status & INIT_rangeset )
-        rangeset_domain_destroy(d);
+        rangeset_list_destroy(&d->rangesets);
     if ( init_status & INIT_watchdog )
         watchdog_domain_destroy(d);
     if ( init_status & INIT_xsm )
@@ -815,7 +815,7 @@ static void complete_domain_destroy(struct rcu_head *head)
 
     watchdog_domain_destroy(d);
 
-    rangeset_domain_destroy(d);
+    rangeset_list_destroy(&d->rangesets);
 
     sched_destroy_domain(d);
 
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 1a13a32..a8b5a5d 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -381,20 +381,17 @@ void rangeset_limit(
     r->nr_ranges = limit;
 }
 
-void rangeset_domain_destroy(
-    struct domain *d)
+void rangeset_list_destroy(struct list_head *list)
 {
     struct rangeset *r;
 
-    while ( !list_empty(&d->rangesets) )
+    while ( !list_empty(list) )
     {
-        r = list_entry(d->rangesets.next, struct rangeset, rangeset_list);
+        r = list_entry(list->next, struct rangeset, rangeset_list);
 
-        BUG_ON(r->domain != d);
-        r->domain = NULL;
         list_del(&r->rangeset_list);
 
-        rangeset_destroy(r);
+        rangeset_destroy(r, NULL);
     }
 }
 
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index e8244a0..cc795d1 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -18,14 +18,9 @@ struct spinlock;
 struct rangeset;
 
 /*
- * Destroy per-domain rangeset information.
- * 
- * It is invalid to create or destroy a rangeset belonging to a domain @d
- * before rangeset_domain_initialise(d) returns or after calling
- * rangeset_domain_destroy(d).
+ * Destroy a list of rangesets.
  */
-void rangeset_domain_destroy(
-    struct domain *d);
+void rangeset_list_destroy(struct list_head *list);
 
 /*
  * Create a rangeset. Optionally attach to a specified list @head.
-- 
2.7.4


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

[RFC 5/6] rangeset_domain_printk() refactoring

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

rangeset_domain_printk() is split into generic rangeset_list_printk() function
and domain specific common/domain.c:domain_rangeset_printk().

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/common/domain.c        | 14 ++++++++++++++
 xen/common/keyhandler.c    |  2 +-
 xen/common/rangeset.c      | 15 +++------------
 xen/include/xen/domain.h   |  3 +++
 xen/include/xen/rangeset.h |  4 ++--
 5 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 47c45f2..9b68e2f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1560,6 +1560,20 @@ void domain_rangeset_destroy(struct domain *d,
     rangeset_destroy(r, &d->rangesets_lock);
 }
 
+void domain_rangeset_printk(
+    struct domain *d)
+{
+    printk("Rangesets belonging to domain %u:\n", d->domain_id);
+
+    spin_lock(&d->rangesets_lock);
+
+    if ( list_empty(&d->rangesets) )
+        printk("    None\n");
+
+    rangeset_list_printk(&d->rangesets);
+
+    spin_unlock(&d->rangesets_lock);
+}
 
 /*
  * Local variables:
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 16de6e8..4f237f0 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -320,7 +320,7 @@ static void dump_domains(unsigned char key)
 
         arch_dump_domain_info(d);
 
-        rangeset_domain_printk(d);
+        domain_rangeset_printk(d);
 
         dump_pageframe_info(d);
 
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index a8b5a5d..c18fb21 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -455,26 +455,17 @@ void rangeset_printk(
     read_unlock(&r->lock);
 }
 
-void rangeset_domain_printk(
-    struct domain *d)
+void rangeset_list_printk(
+    struct list_head *list)
 {
     struct rangeset *r;
 
-    printk("Rangesets belonging to domain %u:\n", d->domain_id);
-
-    spin_lock(&d->rangesets_lock);
-
-    if ( list_empty(&d->rangesets) )
-        printk("    None\n");
-
-    list_for_each_entry ( r, &d->rangesets, rangeset_list )
+    list_for_each_entry ( r, list, rangeset_list )
     {
         printk("    ");
         rangeset_printk(r);
         printk("\n");
     }
-
-    spin_unlock(&d->rangesets_lock);
 }
 
 /*
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 3d9c652..b2dca15 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -114,4 +114,7 @@ struct rangeset *domain_rangeset_new(struct domain *d, char *name,
 void domain_rangeset_destroy(struct domain *d,
     struct rangeset *r);
 
+void domain_rangeset_printk(
+    struct domain *d);
+
 #endif /* __XEN_DOMAIN_H__ */
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index cc795d1..8fd8164 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -81,8 +81,8 @@ void rangeset_swap(struct rangeset *a, struct rangeset *b);
 /* Rangeset pretty printing. */
 void rangeset_printk(
     struct rangeset *r);
-void rangeset_domain_printk(
-    struct domain *d);
+void rangeset_list_printk(
+    struct list_head *list);
 
 #endif /* __XEN_RANGESET_H__ */
 
-- 
2.7.4


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

[RFC 6/6] Drop domain remains from rangeset

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/common/rangeset.c      | 3 +--
 xen/include/xen/rangeset.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index c18fb21..857615b 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -19,9 +19,8 @@ struct range {
 };
 
 struct rangeset {
-    /* Owning domain and threaded list of rangesets. */
+    /* threaded list of rangesets. */
     struct list_head rangeset_list;
-    struct domain   *domain;
 
     /* Ordered list of ranges contained in this set, and protecting lock. */
     struct list_head range_list;
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index 8fd8164..8462071 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -12,7 +12,6 @@
 
 #include <xen/types.h>
 
-struct domain;
 struct list_head;
 struct spinlock;
 struct rangeset;
-- 
2.7.4


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

Re: [RFC 0/6] Rangeset generalisation

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 12:03
> To: xen-devel@lists.xenproject.org
> Cc: andrii_anisov@epam.com; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; Paul Durrant
> <Paul.Durrant@citrix.com>; sstabellini@kernel.org; Tim (Xen.org)
> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: [RFC 0/6] Rangeset generalisation
> 
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> Rangesets in XEN seems to be a pretty generic thing slightly poisoned with.
> domain specific funtionality in initialization and deinitialization code.
> 
> So make the rangeset code generic with moving domain specific code to
> common/domain.c

Any particular reason this series is RFC? The cleanup seems a good thing to do to me.

  Paul

> 
> Andrii Anisov (6):
>   rangeset_new() refactoring
>   rangeset_destroy() refactoring
>   Drop rangeset_domain_initialise()
>   rangeset_domain_destroy() refactoring
>   rangeset_domain_printk() refactoring
>   Drop domain remains from rangeset
> 
>  xen/arch/x86/domain.c      |  2 +-
>  xen/arch/x86/hvm/ioreq.c   |  6 ++---
>  xen/arch/x86/mm/p2m.c      |  6 ++---
>  xen/arch/x86/setup.c       |  4 +--
>  xen/common/domain.c        | 48 +++++++++++++++++++++++++++++++----
>  xen/common/keyhandler.c    |  2 +-
>  xen/common/rangeset.c      | 63 +++++++++++++++-----------------------------
> --
>  xen/include/xen/domain.h   |  9 +++++++
>  xen/include/xen/rangeset.h | 42 +++++++++++++++----------------
>  9 files changed, 102 insertions(+), 80 deletions(-)
> 
> --
> 2.7.4


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

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

Dear Paul,

> The cleanup seems a good thing to do to me.

So I would collect comments, rebase it to latest master and push the
second version without RFC.

> Any particular reason this series is RFC?

The reason to make this series was an intention to use rangesets to
manage mmio ranges in our shared coprocessor framework. It was planned
to extend range with `void* priv` to extend functionality.
Unfortunately the rangeset feature to merge ranges makes it unusable
for our needs. Also linked list, even sorted, is not really good in
search.
Another concern was how community react on the change into generic code.

Sincerely,
Andrii Anisov.

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

Re: [RFC 0/6] Rangeset generalisation

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 12:46
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; andrii_anisov@epam.com; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; sstabellini@kernel.org; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [RFC 0/6] Rangeset generalisation
> 
> Dear Paul,
> 
> > The cleanup seems a good thing to do to me.
> 
> So I would collect comments, rebase it to latest master and push the
> second version without RFC.
> 
> > Any particular reason this series is RFC?
> 
> The reason to make this series was an intention to use rangesets to
> manage mmio ranges in our shared coprocessor framework. It was planned
> to extend range with `void* priv` to extend functionality.
> Unfortunately the rangeset feature to merge ranges makes it unusable
> for our needs. Also linked list, even sorted, is not really good in
> search.
> Another concern was how community react on the change into generic code.

Many moons ago there were patches to use rbtree for rangesets. Perhaps it would be worth reviving that idea?

  Paul

> 
> Sincerely,
> Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

Dear Paul,

> Many moons ago there were patches to use rbtree for rangesets. Perhaps it would be worth reviving that idea?
rbtree is a thing I think of now for our needs.
Even more, currently I think of refactoring ARM mmio ranges managing
to use rbtree one day. Currently there is a static array of 16 ranges
maximum with sort on insert and bsearch there. With introducing of
shared coprocessor framework system would need more mmio ranges, and
dynamic data structure fast search capable looks good to fit their
needs.
I assume Julien did not consider rangesets for ARM mmio managing due
to no range priv data for r/w handlers, linked list search slowness
and ranges merging capabilities.

Sincerely,
Andrii Anisov.

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

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

Paul,

> Many moons ago there were patches to use rbtree for rangesets. Perhaps it would be worth reviving that idea?
Do you have a link to look at those patches?

Sincerely,
Andrii Anisov.

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

Re: [RFC 0/6] Rangeset generalisation

[Paul Durrant]

> -----Original Message-----
> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
> Sent: 16 February 2017 13:24
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; andrii_anisov@epam.com; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> jbeulich@suse.com; konrad.wilk@oracle.com; sstabellini@kernel.org; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [RFC 0/6] Rangeset generalisation
> 
> Paul,
> 
> > Many moons ago there were patches to use rbtree for rangesets. Perhaps
> it would be worth reviving that idea?
> Do you have a link to look at those patches?

The relevant patch is:

https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg01619.html

  Paul

> 
> Sincerely,
> Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

> The relevant patch is:
>
> https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg01619.html

Thank you for the link.
I would try to realize why it is left unmerged.

Sincerely,
Andrii Anisov.


2017-02-16 16:02 GMT+02:00 Paul Durrant <Paul.Durrant@citrix.com>:
>> -----Original Message-----
>> From: Andrii Anisov [mailto:andrii.anisov@gmail.com]
>> Sent: 16 February 2017 13:24
>> To: Paul Durrant <Paul.Durrant@citrix.com>
>> Cc: xen-devel@lists.xenproject.org; andrii_anisov@epam.com; Andrew
>> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
>> jbeulich@suse.com; konrad.wilk@oracle.com; sstabellini@kernel.org; Tim
>> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
>> Subject: Re: [RFC 0/6] Rangeset generalisation
>>
>> Paul,
>>
>> > Many moons ago there were patches to use rbtree for rangesets. Perhaps
>> it would be worth reviving that idea?
>> Do you have a link to look at those patches?
>
> The relevant patch is:
>
> https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg01619.html
>
>   Paul
>
>>
>> Sincerely,
>> Andrii Anisov.

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

Re: [RFC 0/6] Rangeset generalisation

[Jan Beulich]

>>> On 16.02.17 at 13:45, <andrii.anisov@gmail.com> wrote:
> Dear Paul,
> 
>> The cleanup seems a good thing to do to me.
> 
> So I would collect comments, rebase it to latest master and push the
> second version without RFC.
> 
>> Any particular reason this series is RFC?
> 
> The reason to make this series was an intention to use rangesets to
> manage mmio ranges in our shared coprocessor framework.

If this is meant to be per-domain management - how many such
ranges do you expect to be necessary for any one domain? We've
had attempts before to (ab)use rangesets for such a purpose.

> It was planned
> to extend range with `void* priv` to extend functionality.
> Unfortunately the rangeset feature to merge ranges makes it unusable
> for our needs. Also linked list, even sorted, is not really good in
> search.

This concern makes me assume there might be quite many of them,
which then makes this a no-go for unprivileged domains.

Jan


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

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

Dear Jan,

> If this is meant to be per-domain management - how many such
> ranges do you expect to be necessary for any one domain? We've
> had attempts before to (ab)use rangesets for such a purpose.
It is meant to be the per-domain management. To handle per-domain
vcoproc register access emulation described here [1] in terms of
shared coproc framework [2].

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg01935.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2016-10/msg01966.html

Sincerely,
Andrii Anisov.

2017-02-16 15:25 GMT+02:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 16.02.17 at 13:45, <andrii.anisov@gmail.com> wrote:
>> Dear Paul,
>>
>>> The cleanup seems a good thing to do to me.
>>
>> So I would collect comments, rebase it to latest master and push the
>> second version without RFC.
>>
>>> Any particular reason this series is RFC?
>>
>> The reason to make this series was an intention to use rangesets to
>> manage mmio ranges in our shared coprocessor framework.
>
> If this is meant to be per-domain management - how many such
> ranges do you expect to be necessary for any one domain? We've
> had attempts before to (ab)use rangesets for such a purpose.
>
>> It was planned
>> to extend range with `void* priv` to extend functionality.
>> Unfortunately the rangeset feature to merge ranges makes it unusable
>> for our needs. Also linked list, even sorted, is not really good in
>> search.
>
> This concern makes me assume there might be quite many of them,
> which then makes this a no-go for unprivileged domains.
>
> Jan
>

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

Re: [RFC 0/6] Rangeset generalisation

[Andrii Anisov]

Dear Jan,

I'm really sorry, but I did not get your point here:

> This concern makes me assume there might be quite many of them,
> which then makes this a no-go for unprivileged domains.

Could you please provide wider explanation.

Sincerely,
Andrii Anisov.

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

Re: [RFC 0/6] Rangeset generalisation

[Jan Beulich]

>>> On 16.02.17 at 14:42, <andrii.anisov@gmail.com> wrote:
> I'm really sorry, but I did not get your point here:
> 
>> This concern makes me assume there might be quite many of them,
>> which then makes this a no-go for unprivileged domains.
> 
> Could you please provide wider explanation.

Well, as it had been discussed before, I thought you would have
found it the latest along with Paul having pointed you at the
patches. The issue is with resource consumption: We can't allow
a guest to consume basically arbitrary amounts of memory in the
rangeset control structures. Iirc this was in the context of the
now stuck persistent memory support series as well as around
Intel's graphics virtualization one.

Jan


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

Re: [RFC 0/6] Rangeset generalisation

[George Dunlap]

On 16/02/17 14:30, Jan Beulich wrote:
>>>> On 16.02.17 at 14:42, <andrii.anisov@gmail.com> wrote:
>> I'm really sorry, but I did not get your point here:
>>
>>> This concern makes me assume there might be quite many of them,
>>> which then makes this a no-go for unprivileged domains.
>>
>> Could you please provide wider explanation.
> 
> Well, as it had been discussed before, I thought you would have
> found it the latest along with Paul having pointed you at the
> patches. The issue is with resource consumption: We can't allow
> a guest to consume basically arbitrary amounts of memory in the
> rangeset control structures. Iirc this was in the context of the
> now stuck persistent memory support series as well as around
> Intel's graphics virtualization one.

I'm not sure it's reasonable to expect people to wade through all the
discussions of previous threads, complete with style nitpicks and
what-not, when it could be summarized in a sentence or two. :-)

I think the key thing with the graphics virtualization one, anyway, was
that the guest could *provoke* the devicemodel into creating arbitrary
numbers of "ranges" by using them in the GPT tables.  If the number of
ranges is large but bounded, that's less of an issue.

Andrii, could you describe a bit more how you had thought of using the
rangesets?

 -George

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