Gleb Natapov wrote: > On Mon, Jul 13, 2009 at 09:16:47AM -0400, Gregory Haskins wrote: > >> Gleb Natapov wrote: >> >>> On Mon, Jul 13, 2009 at 09:01:33AM -0400, Gregory Haskins wrote: >>> >>> >>>> Gleb Natapov wrote: >>>> >>>> >>>>> Signed-off-by: Gleb Natapov <[email protected]> >>>>> --- >>>>> include/linux/kvm_host.h | 2 +- >>>>> virt/kvm/irq_comm.c | 55 >>>>> +++++++++++++++++++++------------------------- >>>>> virt/kvm/kvm_main.c | 1 - >>>>> 3 files changed, 26 insertions(+), 32 deletions(-) >>>>> >>>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >>>>> index f54a0d3..6756b3e 100644 >>>>> --- a/include/linux/kvm_host.h >>>>> +++ b/include/linux/kvm_host.h >>>>> @@ -161,7 +161,7 @@ struct kvm { >>>>> >>>>> struct mutex irq_lock; >>>>> #ifdef CONFIG_HAVE_KVM_IRQCHIP >>>>> - struct list_head irq_routing; /* of kvm_kernel_irq_routing_entry */ >>>>> + struct kvm_kernel_irq_routing_entry *irq_routing; >>>>> struct hlist_head mask_notifier_list; >>>>> #endif >>>>> >>>>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c >>>>> index 7af18b8..b2fa3f6 100644 >>>>> --- a/virt/kvm/irq_comm.c >>>>> +++ b/virt/kvm/irq_comm.c >>>>> @@ -148,7 +148,8 @@ 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) >>>>> + rcu_read_lock(); >>>>> + for (e = rcu_dereference(kvm->irq_routing); e && e->set; e++) { >>>>> >>>>> >>>>> >>>> Hi Gleb, >>>> I haven't had a chance to fully digest and review these patches, but >>>> one thing I did notice is that you seem to be converting from a list to >>>> an open-coded structure. I am just curious why you made this design >>>> decision instead of using the RCU variant of list? >>>> >>>> >>>> >>> It is not scary "open-coded structure" it's just an array :) As I responded >>> to Michael the idea is to move msis out of irq_routing, make the array >>> much smaller and either use gsi as an index in the array or use hash table >>> instead looping over all entries. For now I can justify array as more >>> cache friendly data structure as we scan it linearly. >>> >>> >> Ok, but that might be a good thing to mention in the patch header ;) >> >> > What exactly? Besides this is just an RFC. By the time it will be > applied (if at all) I may do the change already :) >
Heh, thats fine. FWIW, I would suggest this: "the idea is to move msis out of irq_routing, make the array much smaller and either use gsi as an index in the array or use hash table instead looping over all entries. For now I can justify array as more cache friendly data structure as we scan it linearly" Otherwise, reviewers might be curious why you are not using list_X_rcu() ;) Kind Regards, -Greg
signature.asc
Description: OpenPGP digital signature
