On 11/09/2010 04:11 PM, Jan Kiszka wrote:
> >>> but what about PCI_COMMAND_INTX_DISABLE? >>> >>> Perhaps we can hook the kernel's handler for this bit? >> >> Some IRQ registration notifier that would allow us to reregister our >> handler with IRQ sharing support? Maybe. > > Adding an internal API if preferable to an external one (it may be a > pain to kvm-kmod users though). Primary concern should be a clean and robust API. From that POV, I would prefer an official hook with genirq maintainer blessing over fragile detection heuristics in kvm.
That is what I meant (internal to the kernel, not to kvm, vs the external kvm/user interface).
Given that VSIO should benefit from any transparent pattern we develop here as well, it's probably worth to go that path - if it is really preferred over manual control like this patch proposes.
Yes. The API is already sufficiently complicated that it is very hard for userspace to get right.
For kvm-kmod, we could simply enforce IRQ sharing measures unconditionally. Not optimal from the performance POV, but people concerned that much about performance should better use KVM over the corresponding kernel anyway.
Or backport the needed patches. kvm-kmod is now no longer needed for enterprise users with fixed distros, it's mostly useful for the embedded guys which have more flexibility.
>>> >>> I saw no reason this can't be a spinlock, but perhaps I missed >>> something. This would allow us to avoid srcu, which is slightly more >>> expensive than rcu. Since pci 2.3 assigned devices are not a major use >>> case, I'd like not to penalize the mainstream users for this. >> >> The lock has to be held across kvm_set_irq, which is the potentially >> expensive (O(n), n == number of VCPUs) operation. > > What we should probably do is have broadcast interrupts deferred to a > thread. I agree it isn't pretty. Not sure we could defer it that easily (if at all). However, I think such improvements should be done on top if this already complex change.
That means flipping locking to srcu and back to rcu. What I'm proposing is to keep broadcast within the spinlock for now, and move it later, provided we agree that it's doable.
-- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
