Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752430AbbESB6j (ORCPT ); Mon, 18 May 2015 21:58:39 -0400 Received: from mga03.intel.com ([134.134.136.65]:3777 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752022AbbESB6U convert rfc822-to-8bit (ORCPT ); Mon, 18 May 2015 21:58:20 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,456,1427785200"; d="scan'208";a="696889064" From: "Wu, Feng" To: Thomas Gleixner CC: "mingo@redhat.com" , "hpa@zytor.com" , "linux-kernel@vger.kernel.org" , "jiang.liu@linux.intel.com" , "Wu, Feng" Subject: RE: [v5 3/3] x86, irq: Define a global vector for VT-d Posted-Interrupts Thread-Topic: [v5 3/3] x86, irq: Define a global vector for VT-d Posted-Interrupts Thread-Index: AQHQkXVsyPOLW7G/XEiFumCerlK4W52Civbg Date: Tue, 19 May 2015 01:58:10 +0000 Message-ID: References: <1431943995-3344-1-git-send-email-feng.wu@intel.com> <1431943995-3344-4-git-send-email-feng.wu@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2593 Lines: 97 > -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: Monday, May 18, 2015 10:18 PM > To: Wu, Feng > Cc: mingo@redhat.com; hpa@zytor.com; linux-kernel@vger.kernel.org; > jiang.liu@linux.intel.com > Subject: Re: [v5 3/3] x86, irq: Define a global vector for VT-d Posted-Interrupts > > On Mon, 18 May 2015, Feng Wu wrote: > > diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h > > index 0f5fb6b..9866065 100644 > > --- a/arch/x86/include/asm/hardirq.h > > +++ b/arch/x86/include/asm/hardirq.h > > @@ -14,6 +14,7 @@ typedef struct { > > #endif > > #ifdef CONFIG_HAVE_KVM > > unsigned int kvm_posted_intr_ipis; > > + unsigned int kvm_posted_intr_wakeup_ipis; > > So now we have another IPI with statistics and nothing which makes it > accessible. kvm_posted_intr_ipis lacks a printout in > arch_show_interrupts() as well. > I will add the printouts for these two IPIs. > > #ifdef CONFIG_HAVE_KVM > > +void (*wakeup_handler_callback)(void); > > +EXPORT_SYMBOL_GPL(wakeup_handler_callback); > > + > > The naming sucks. Which wakeup? > > As this is kvm specific, it should have a kvm_ prefix. And it should > tell what it actually does: > > kvm_posted_intr_wakeup_handler > > Hmm? Good suggestion! > > > /* > > * Handler for POSTED_INTERRUPT_VECTOR. > > */ > > @@ -256,6 +259,26 @@ __visible void smp_kvm_posted_intr_ipi(struct > pt_regs *regs) > > > > set_irq_regs(old_regs); > > } > > + > > +/* > > + * Handler for POSTED_INTERRUPT_WAKEUP_VECTOR. > > + */ > > +__visible void smp_kvm_posted_intr_wakeup_ipi(struct pt_regs *regs) > > +{ > > + struct pt_regs *old_regs = set_irq_regs(regs); > > + > > + entering_ack_irq(); > > + > > + inc_irq_stat(kvm_posted_intr_wakeup_ipis); > > + > > + if (wakeup_handler_callback) > > + wakeup_handler_callback(); > > Why do we need a conditional here? > > staic void dummy_handler(void) { } > static void *kvm_posted_intr_wakeup_handler = dummy_handler; > > void kvm_set_posted_intr_wakeup_handler(void (*handler)(void)) > { > if (handler) > kvm_posted_intr_wakeup_handler = handler; > else > kvm_posted_intr_wakeup_handler = dummy_handler; > } > > avoids the conditional in the exception handler.... Got it. Conditional branch in a critical path is really a bad thing... Thanks, Feng > > Thanks, > > tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/