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 <gleb@xxxxxxxxxx>
>>>>> ---
>>>>> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
