From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yu, Zhang" Subject: Re: [PATCH] Refactor ioreq server for better performance Date: Wed, 01 Jul 2015 17:02:03 +0800 Message-ID: <5593AC8B.2090103@linux.intel.com> References: <1435314588-8815-1-git-send-email-yu.c.zhang@linux.intel.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0259742CB@AMSPEX01CL02.citrite.net> <5592410B.5040509@linux.intel.com> <9AAE0902D5BC7E449B7C8E4E778ABCD02597573A@AMSPEX01CL02.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZADwz-0002ep-CA for xen-devel@lists.xenproject.org; Wed, 01 Jul 2015 09:05:01 +0000 In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD02597573A@AMSPEX01CL02.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant , "xen-devel@lists.xenproject.org" , Andrew Cooper , "JBeulich@suse.com" , Kevin Tian , "zhiyuan.lv@intel.com" List-Id: xen-devel@lists.xenproject.org On 6/30/2015 5:59 PM, Paul Durrant wrote: >> -----Original Message----- >> From: Yu, Zhang [mailto:yu.c.zhang@linux.intel.com] >> Sent: 30 June 2015 08:11 >> To: Paul Durrant; xen-devel@lists.xenproject.org; Andrew Cooper; >> JBeulich@suse.com; Kevin Tian; zhiyuan.lv@intel.com >> 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. > Thanks, Paul. Well, I agree the former approach would be simpler. But I still doubt if this is more reasonable. :) IIUC, one of the reasons for struct domain to have a rangeset list(and a spinlock - rangesets_lock), is because there are iomem_caps and irq_caps for each domain. These 2 rangeset members of struct domain are platform independent. However, struct rb_rangeset is only supposed to be used in ioreq server, which is only for x86 hvm cases. Adding a rb_rangeset list member(similarly, if so, a rb_rangesets_lock is also required) in struct domain maybe useless for hardware domain and for platforms other than x86. So, I'd like to register a new debug key, to dump the ioreq server informations, just like the keys to dump iommu p2m table or the irq mappings. With a new debug key, we do not need to add a spinlock for rb_rangeset in struct domain, the one in ioreq server would be enough. Does this sound reasonable? > [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. > Sure, and thanks. :) Yu > 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 >>>> + >>>> +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@lists.xen.org >>> http://lists.xen.org/xen-devel >>> >>> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > >