From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 01/10] Change irq routing table to use gsi indexed array. Date: Sun, 09 Aug 2009 17:25:19 +0300 Message-ID: <4A7EDC4F.6000104@redhat.com> References: <1249821671-32356-1-git-send-email-gleb@redhat.com> <1249821671-32356-2-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx2.redhat.com ([66.187.237.31]:55184 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754018AbZHIOTZ (ORCPT ); Sun, 9 Aug 2009 10:19:25 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n79EJO9i021148 for ; Sun, 9 Aug 2009 10:19:26 -0400 In-Reply-To: <1249821671-32356-2-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 08/09/2009 03:41 PM, Gleb Natapov wrote: > Use gsi indexed array instead of scanning all entries on each interrupt > injection. Also maintain back mapping from irqchip/pin to gsi to speedup > interrupt acknowledgment notifications. > > Signed-off-by: Gleb Natapov > --- > include/linux/kvm_host.h | 11 +++++- > virt/kvm/irq_comm.c | 87 ++++++++++++++++++++++++--------------------- > virt/kvm/kvm_main.c | 1 - > 3 files changed, 55 insertions(+), 44 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index f814512..a38f576 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -129,7 +129,14 @@ struct kvm_kernel_irq_routing_entry { > } irqchip; > struct msi_msg msi; > }; > - struct list_head link; > + struct hlist_node link; > +}; > + > +struct kvm_irq_routing_table { > + int chip[3][KVM_IOAPIC_NUM_PINS]; > + struct kvm_kernel_irq_routing_entry *rt_entries; > + u32 max_gsi; > That's the table size, no? so call it nr_rt_entries to reflect that (max_* implies an off-by-one). > + struct hlist_head map[0]; > }; > What's this? (comment explaining the type of data). > > struct kvm { > @@ -167,7 +174,7 @@ struct kvm { > > struct mutex irq_lock; > #ifdef CONFIG_HAVE_KVM_IRQCHIP > - struct list_head irq_routing; /* of kvm_kernel_irq_routing_entry */ > Like here. > @@ -150,8 +152,9 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) > * IOAPIC. So set the bit in both. The guest will ignore > * writes to the unused one. > */ > - list_for_each_entry(e,&kvm->irq_routing, link) > - if (e->gsi == irq) { > + irq_rt = kvm->irq_routing; > + if (irq< irq_rt->max_gsi) > irq here is int, so can overflow. I think all paths are protected, but better to change irq to unsigned just in case. > + hlist_for_each_entry(e, n,&irq_rt->map[irq], link) { > int r = e->set(e, kvm, sig_level); > if (r< 0) > continue; > @@ -163,20 +166,15 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, int irq, int level) > > void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin) > { > - struct kvm_kernel_irq_routing_entry *e; > struct kvm_irq_ack_notifier *kian; > struct hlist_node *n; > - unsigned gsi = pin; > + unsigned gsi; > > trace_kvm_ack_irq(irqchip, pin); > > - list_for_each_entry(e,&kvm->irq_routing, link) > - if (e->type == KVM_IRQ_ROUTING_IRQCHIP&& > - e->irqchip.irqchip == irqchip&& > - e->irqchip.pin == pin) { > - gsi = e->gsi; > - break; > - } > + gsi = kvm->irq_routing->chip[irqchip][pin]; > + if (gsi == -1) > + gsi = pin; > Please separate the reverse map to a separate patch. > + new = kzalloc(sizeof(*new) + (max_gsi * sizeof(struct hlist_head)) + > + (nr * sizeof(struct kvm_kernel_irq_routing_entry)), > + GFP_KERNEL); > Wow. -- error compiling committee.c: too many arguments to function