Re: [Xen-devel] [PATCH] Refactor ioreq server for better performance

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> Sent: 30 June 2015 08:11
> To: Paul Durrant; xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper;
> JBeulich@xxxxxxxx; Kevin Tian; zhiyuan.lv@xxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] Refactor ioreq server for better
> performance
> 
> Thanks you, Paul.
> 

No problem :-)

[snip]
> >
> > I think assigning a name to the rangeset and having a debug-key dump is
> useful. Can you not duplicate that in your new implementation?
> >
> 
> Well, I can add some dump routines, e.g. hvm_ioreq_server_dump_range(),
> to dump the ranges inside each ioreq server of a domain. This routine
> is similar to the rangeset_domain_printk(). But unlike the rangeset,
> which is also inserted in domain->rangesets list, the new rb_rangeset
> is only a member of ioreq server. So we can dump the ranges inside a
> domain by first access each ioreq server.
> Do you think this approach duplicated?
> 

Either add an rb_rangesets list to the domain, or have the debug key walk the 
ioreq server list and dump the rangesets in each. The former is obviously the 
simplest.

[snip]
> >
> > I this limit enough? I think having something that's toolstack-tunable would
> be more future-proof.
> >
> 
> By now, this value would suffice. I'd prefer this as a default value.
> As you know, I have also considered a xl param to do so. And one
> question is that, would a per-domain param appropriate? I mean, each
> hvm can have multiple ioreq servers. If this is acceptible, I can cook
> another patch to do so, is this OK?
> 

That's ok with me, but you may need to mention the idea of a follow up patch in 
the check-in comment.

  Paul

> Thanks
> Yu
> 
> >    Paul
> >
> >>
> >>   struct hvm_ioreq_server {
> >>       struct list_head       list_entry;
> >> @@ -68,7 +68,7 @@ struct hvm_ioreq_server {
> >>       /* Lock to serialize access to buffered ioreq ring */
> >>       spinlock_t             bufioreq_lock;
> >>       evtchn_port_t          bufioreq_evtchn;
> >> -    struct rangeset        *range[NR_IO_RANGE_TYPES];
> >> +    struct rb_rangeset     *range[NR_IO_RANGE_TYPES];
> >>       bool_t                 enabled;
> >>       bool_t                 bufioreq_atomic;
> >>   };
> >> diff --git a/xen/include/xen/rb_rangeset.h
> >> b/xen/include/xen/rb_rangeset.h
> >> new file mode 100644
> >> index 0000000..768230c
> >> --- /dev/null
> >> +++ b/xen/include/xen/rb_rangeset.h
> >> @@ -0,0 +1,45 @@
> >> +/*
> >> +  Red-black tree based rangeset
> >> +
> >> +  This program is free software; you can redistribute it and/or modify
> >> +  it under the terms of the GNU General Public License as published by
> >> +  the Free Software Foundation; either version 2 of the License, or
> >> +  (at your option) any later version.
> >> +
> >> +  This program is distributed in the hope that it will be useful,
> >> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> +  GNU General Public License for more details.
> >> +
> >> +  You should have received a copy of the GNU General Public License
> >> +  along with this program; if not, write to the Free Software
> >> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> USA
> >> +*/
> >> +
> >> +#ifndef __RB_RANGESET_H__
> >> +#define __RB_RANGESET_H__
> >> +
> >> +#include <xen/rbtree.h>
> >> +
> >> +struct rb_rangeset {
> >> +    long             nr_ranges;
> >> +    struct rb_root   rbroot;
> >> +};
> >> +
> >> +struct rb_range {
> >> +    struct rb_node node;
> >> +    unsigned long s, e;
> >> +};
> >> +
> >> +struct rb_rangeset *rb_rangeset_new(void);
> >> +void rb_rangeset_destroy(struct rb_rangeset *r);
> >> +bool_t rb_rangeset_overlaps_range(struct rb_rangeset *r,
> >> +    unsigned long s, unsigned long e);
> >> +bool_t rb_rangeset_contains_range(
> >> +    struct rb_rangeset *r, unsigned long s, unsigned long e);
> >> +int rb_rangeset_add_range(struct rb_rangeset *r,
> >> +    unsigned long s, unsigned long e);
> >> +int rb_rangeset_remove_range(struct rb_rangeset *r,
> >> +    unsigned long s, unsigned long e);
> >> +
> >> +#endif /* __RB_RANGESET_H__ */
> >> --
> >> 1.9.1
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> >
> >

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