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

2017-02-16 Thread 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

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

2017-02-16 Thread Paul Durrant 
> -Original Message-
> From: Andrii Anisov [mailto:andrii.ani...@gmail.com]
> Sent: 16 February 2017 12:03
> To: xen-de...@lists.xenproject.org
> Cc: andrii_ani...@epam.com; Andrew Cooper
> ; George Dunlap
> ; Ian Jackson ;
> jbeul...@suse.com; konrad.w...@oracle.com; Paul Durrant
> ; sstabell...@kernel.org; Tim (Xen.org)
> ; Wei Liu 
> Subject: [RFC 1/6] rangeset_new() refactoring
> 
> From: Andrii Anisov 
> 
> 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 
> ---
>  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, );
> +
> +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(>rangesets_lock);
> +

[Xen-devel] [RFC 1/6] rangeset_new() refactoring

2017-02-16 Thread Andrii Anisov 
From: Andrii Anisov 

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 
---
 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, );
+
+if ( d != NULL )
+{
+spin_lock(>rangesets_lock);
+list_add(head, >rangesets);
+spin_unlock(>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(>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(>rangesets_lock);
-list_add(>rangeset_list, >rangesets);
-