lkml.org 
[lkml]   [2014]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4] kvm/irqchip: Speed up KVM_SET_GSI_ROUTING
Il 05/05/2014 16:21, Christian Borntraeger ha scritto:
> On 28/04/14 18:39, Paolo Bonzini wrote:
>> From: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Given all your work, What about From: Paolo Bonzini <pbonzini@redhat.com>
> plus "Based on an inital patch from Christian Borntraeger"

No big deal, I don't care about authorship that much.

>> @@ -221,17 +225,18 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
>> unsigned long flags = (unsigned long)key;
>> struct kvm_kernel_irq_routing_entry *irq;
>> struct kvm *kvm = irqfd->kvm;
>> + int idx;
>>
>> if (flags & POLLIN) {
>> - rcu_read_lock();
>> - irq = rcu_dereference(irqfd->irq_entry);
>> + idx = srcu_read_lock(&kvm->irq_srcu);
>> + irq = srcu_dereference(irqfd->irq_entry, &kvm->irq_srcu);
>> /* An event has been signaled, inject an interrupt */
>> if (irq)
>> kvm_set_msi(irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
>> false);
>> else
>> schedule_work(&irqfd->inject);
>> - rcu_read_unlock();
>> + srcu_read_unlock(&kvm->irq_srcu, idx);
>> }
>>
>> if (flags & POLLHUP) {
>> @@ -363,7 +368,7 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
>> }
>>
>> list_add_rcu(&irqfd->resampler_link, &irqfd->resampler->list);
>> - synchronize_rcu();
>> + synchronize_srcu(&kvm->irq_srcu);
>
> No idea what resampler is, can this become time critical as well - iow do we need expedited here?

It's for level-triggered interrupts. I decided that if synchronize_rcu
was good enough before, synchronize_srcu will do after the patch.

>> @@ -85,7 +86,7 @@ void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
>> mutex_lock(&kvm->irq_lock);
>> hlist_del_init_rcu(&kian->link);
>> mutex_unlock(&kvm->irq_lock);
>> - synchronize_rcu();
>> + synchronize_srcu_expedited(&kvm->irq_srcu);
>
> Hmm, looks like all callers are slow path (shutdown, deregister assigned dev). Couldnt
> we use the non expedited variant?

... but I have screwed up this one. Thanks, I'll change it.

>> r = kvm_arch_init_vm(kvm, type);
>> if (r)
>> - goto out_err_nodisable;
>> + goto out_err_no_disable;
>>
>> r = hardware_enable_all();
>> if (r)
>> - goto out_err_nodisable;
>> + goto out_err_no_disable;
>>
>> #ifdef CONFIG_HAVE_KVM_IRQCHIP
>> INIT_HLIST_HEAD(&kvm->mask_notifier_list);
>> @@ -473,10 +473,12 @@ static struct kvm *kvm_create_vm(unsigned long type)
>> r = -ENOMEM;
>> kvm->memslots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
>> if (!kvm->memslots)
>> - goto out_err_nosrcu;
>> + goto out_err_no_srcu;
>> kvm_init_memslots_id(kvm);
>> if (init_srcu_struct(&kvm->srcu))
>> - goto out_err_nosrcu;
>> + goto out_err_no_srcu;
>> + if (init_srcu_struct(&kvm->irq_srcu))
>> + goto out_err_no_irq_srcu;
>> for (i = 0; i < KVM_NR_BUSES; i++) {
>> kvm->buses[i] = kzalloc(sizeof(struct kvm_io_bus),
>> GFP_KERNEL);
>> @@ -505,10 +507,12 @@ static struct kvm *kvm_create_vm(unsigned long type)
>> return kvm;
>>
>> out_err:
>> + cleanup_srcu_struct(&kvm->irq_srcu);
>> +out_err_no_irq_srcu:
>> cleanup_srcu_struct(&kvm->srcu);
>> -out_err_nosrcu:
>> +out_err_no_srcu:
>> hardware_disable_all();
>> -out_err_nodisable:
>> +out_err_no_disable:
>
>
> the patch would be smaller without this change, but it makes the naming more consistent, so ok.

Yeah, out_err_noirq_srcu or out_err_noirqsrcu are both very ugly.

Thanks for the review, I'm making the small change to remove expedited
and applying to kvm/queue.

Paolo

>
>> for (i = 0; i < KVM_NR_BUSES; i++)
>> kfree(kvm->buses[i]);
>> kfree(kvm->memslots);
>>
>



\
 
 \ /
  Last update: 2014-05-05 17:01    [W:0.071 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site