| Message ID | 1487246610-8298-2-git-send-email-andrii.anisov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----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
>> + 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.
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);