On 08/12/2009 12:47 PM, Gleb Natapov wrote:
Ah, the real motivation is msi. Pushing locks down doesn't help if we
keep locking them. But for msi we avoid the lock entirely.
Yes. MSI is one. Multiple IOAPICs may inject interrupt in parallel too
(if we will choose to implement multiple IOAPICs sometime).
Right. Given msi, I don't think we'll have multiple ioapics though.
Why a spinlock and not a mutex?
Protected sections are small and we do not sleep there.
So what? A mutex is better since it allows preemption (and still has
spinlock performance if it isn't preempted).
This lock will be taken during irq injection from irqfd, so may be leave
it as spinlock and take it _irqsave()? Do we want to allow irq injection
from interrupt context? Otherwise if you say that performance is the
same I don't care one way or the other.
Let's leave _irqsave() until later since that has other implications.
Need to explain why this is safe. I'm not sure it is, because we touch
state afterwards in pic_intack(). We need to do all vcpu-synchronous
operations before dropping the lock.
Forst pic_intack() calls pic_clear_isr() only in auto eoi mode and this mode
is already broken for assigned devices. Second for level triggered
interrupts pic_intack() does nothing after calling pic_clear_isr() and
third I can move pic_clear_isr() call to the end of pic_intack().
I meant, in a comment.
I you agree with above I'll add it as a comment.
Sure.
It should. What if there's a reset with an assigned device? We need to
release the device interrupt (after doing FLR?).
Doing this will just re-enable host interrupt while irq condition is not
cleared in the device. The host will hang. So I think we really shouldn't.
Ok. What about timer acks?
I don't see why we clear remote_irr before dropping the lock. If, while
lock was dropped, interrupt was delivered to this entry it will be
injected when ack notifier returns.
But we'll clear remote_irr afterward the redelivery, and we should to
that only after the new interrupt is acked.
It depend on whether you consider calling ack notifiers a part of
interrupt acknowledgement process.
I don't really care, but the ack process has to be atomic. Since we
need to drop the lock, it means the notifier is not part of the process.
If you do then remote_irr should not
be cleared before ack notifiers since ack process is not completed yet.
With current users functionally it shouldn't matter when we clear
remote_irr. I prefer doing it like we do it now since this how it was
before my patches and since code is simpler this way.
No, I think it introduces a race if an interrupt is raised while the ack
notifier is running.
No, I mean keep the for loop in kvm_ioapic_update_eoi().
Can't do that. __kvm_ioapic_update_eoi() is called from other place with
lock held already.
Ok.
--
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
