From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 5/5] Fix kdump under KVM. Date: Mon, 02 Nov 2009 15:30:23 +0200 Message-ID: <4AEEDEEF.6090905@redhat.com> References: <1256661667-9298-1-git-send-email-clalance@redhat.com> <1256661667-9298-6-git-send-email-clalance@redhat.com> <20091027174246.GA10473@amt.cnet> <4AE81B36.5010104@redhat.com> <20091028124100.GA3063@amt.cnet> <4AEADADD.7070501@redhat.com> <4AEDA773.7030906@redhat.com> <4AEEDD19.9000105@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Chris Lalancette Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42331 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754483AbZKBNa2 (ORCPT ); Mon, 2 Nov 2009 08:30:28 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nA2DUQr8009722 for ; Mon, 2 Nov 2009 08:30:33 -0500 In-Reply-To: <4AEEDD19.9000105@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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