On 11/02/2009 03:22 PM, Chris Lalancette wrote:
Avi Kivity wrote:
On 10/30/2009 02:23 PM, Chris Lalancette wrote:
In the meantime, I've gotten the "set_irq from IRQ context" that Avi
suggested
working, and the fixing up of this IOAPIC check is the last bit to actually get
kdump working.
There are two problems with that:
- kvm_set_irq() calls some notifiers (irq_mask_notifiers, at least, but
possibly more); need to make sure those are safe from irq context
- kvm_set_irq() will loop on all vcpus, usually incurring a cache miss
or two; with large vcpu counts this can make the irq-off time quite
large, hurting our worst-case latency
we can defer the second problem since it's only a performance issue, but
how did you deal with the first?
Actually, there is a problem here, but I don't think it's as you describe.
kvm_set_irq() doesn't directly call the mask notifiers; all it really does is
call the i8259/ioapic callback, which causes an interrupt to be raised on the
LAPIC in question, which causes the interrupt to be injected into the VCPU on
the next run. That doesn't call the irq_mask_notifiers, and seems to be
IRQ-safe (with the conversion from mutex's to spin_locks).
The irq mask notifiers are called later on during an EOI. In particular,
ioapic_mmio_write() -> ioapic_write_indirect(), which is protected by the
ioapic_lock. I don't see a problem here (although please point it out if I
missed it).
If we convert the ioapic lock to spin_lock_irq, then mask notifiers will
be called under that lock. It's true that mask notifiers aren't called
directly from the kvm_set_irq path, but if we change the lock, all paths
are affected.
The problem I do see has to do with the irq_ack notifiers. In particular, if
you are on ia64 and ioapic_mmio_write() gets called for the IOAPIC_REG_EOI
address, we call __kvm_ioapic_update_eoi(). In that case, we drop the spin_lock
and call kvm_notify_acked_irq(), which can re-enter the ioapic code. This could
be IRQ unsafe, after we return from kvm_notify_acked_irq() we re-acquire the
spin-lock, but in an irq-unsafe fashion. Therefore we could take the spin-lock,
get interrupted for the timer interrupt, and try to re-acquire the lock, causing
a deadlock. I'll have to think a bit more on how to solve that one (there's a
similar situation going on in the i8259, which is not ia64 specific).
We have to reaquire the lock with spin_lock_irqsave() to avoid the deadlock.
So, either I'm missing exactly what you are talking about, or there's no
deadlock with my patch (except with ia64). Can you explain further
EOI is also called from x86 (though the lapic).
--
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
