From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH] KVM: Unregister IRQ ACK notifier with in-kernel irqchip Date: Wed, 8 Oct 2008 17:04:05 +0800 Message-ID: <200810081704.06325.sheng@linux.intel.com> References: <1223447960-30355-1-git-send-email-sheng@linux.intel.com> <200810081238.52574.amit.shah@redhat.com> <200810081654.19117.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: Amit Shah , Avi Kivity To: kvm@vger.kernel.org Return-path: Received: from mga09.intel.com ([134.134.136.24]:47967 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755427AbYJHJFQ (ORCPT ); Wed, 8 Oct 2008 05:05:16 -0400 In-Reply-To: <200810081654.19117.sheng@linux.intel.com> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Wednesday 08 October 2008 16:54:18 Sheng Yang wrote: > On Wednesday 08 October 2008 15:08:52 Amit Shah wrote: > > * On Wednesday 08 Oct 2008 12:09:20 Sheng Yang wrote: > > > Signed-off-by: Sheng Yang > > > --- > > > arch/x86/kvm/x86.c | 4 +++- > > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > > index 675fcc1..c5763d7 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -176,7 +176,9 @@ static void kvm_free_assigned_device(struct kvm > > > *kvm, if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested) > > > free_irq(assigned_dev->host_irq, (void *)assigned_dev); > > > > > > - kvm_unregister_irq_ack_notifier(kvm, &assigned_dev->ack_notifier); > > > + if (irqchip_in_kernel(kvm)) > > > + kvm_unregister_irq_ack_notifier(kvm, > > > + &assigned_dev->ack_notifier); > > > > The unregister API should perform the check whether the said notifier > > exists so this shouldn't be necessary. > > Yeah, that's more reasonable. But now I just see, > kvm_register_irq_ack_notifier() go with irqchip_in_kernel() and unregister > didn't. :) > Um... After consider a little more, I think keep it unwrapped by irqchip_in_kernel() may be a little more reasonable. The reason is we just register kvm_register_irq_ack_notifier() when we have in kernel irqchip. If we don't have in kernel irqchip, we shouldn't call them and try to perform this action. Call the function without in-kernel irqchip is wrong, ensure the exist of in-kernel irqchip is the caller's responsibility. What we can do is add a BUG_ON() or ASSERT() to the function, rather let the function tell if itself need in-kernel irqchip. I will update the patch to say we need in kernel irqchip when call the function explicitly. Thanks! -- regards Yang, Sheng