From: Ricardo Neri <ricardo.neri-calderon@linux•intel.com>
To: Peter Zijlstra <peterz@infradead•org>
Cc: Thomas Gleixner <tglx@linutronix•de>,
Ingo Molnar <mingo@kernel•org>, "H. Peter Anvin" <hpa@zytor•com>,
Andi Kleen <andi.kleen@intel•com>,
Ashok Raj <ashok.raj@intel•com>, Borislav Petkov <bp@suse•de>,
Tony Luck <tony.luck@intel•com>,
"Ravi V. Shankar" <ravi.v.shankar@intel•com>,
x86@kernel•org, sparclinux@vger•kernel.org,
linuxppc-dev@lists•ozlabs.org, linux-kernel@vger•kernel.org,
Jacob Pan <jacob.jun.pan@intel•com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel•com>,
Don Zickus <dzickus@redhat•com>,
Nicholas Piggin <npiggin@gmail•com>,
Michael Ellerman <mpe@ellerman•id.au>,
Frederic Weisbecker <frederic@kernel•org>,
Alexei Starovoitov <ast@kernel•org>,
Babu Moger <babu.moger@oracle•com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios•com>,
Masami Hiramatsu <mhiramat@kernel•org>,
Andrew Morton <akpm@linux-foundation•org>,
Philippe Ombredanne <pombredanne@nexb•com>,
Colin Ian King <colin.king@canonical•com>,
Byungchul Park <byungchul.park@lge•com>,
"Paul E. McKenney" <paulmck@linux•vnet.ibm.com>,
"Luis R. Rodriguez" <mcgrof@kernel•org>,
Waiman Long <longman@redhat•com>,
Josh Poimboeuf <jpoimboe@redhat•com>,
Randy Dunlap <rdunlap@infradead•org>,
Davidlohr Bueso <dave@stgolabs•net>,
Christoffer Dall <cdall@linaro•org>,
Marc Zyngier <marc.zyngier@arm•com>,
Kai-Heng Feng <kai.heng.feng@canonical•com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle•com>,
David Rientjes <rientjes@google•com>,
iommu@lists•linux-foundation.org
Subject: Re: [RFC PATCH 17/23] watchdog/hardlockup/hpet: Convert the timer's interrupt to NMI
Date: Thu, 14 Jun 2018 19:07:38 -0700 [thread overview]
Message-ID: <20180615020738.GB11625@voyager> (raw)
In-Reply-To: <20180613090720.GV12258@hirez.programming.kicks-ass.net>
On Wed, Jun 13, 2018 at 11:07:20AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 12, 2018 at 05:57:37PM -0700, Ricardo Neri wrote:
>
> +static bool is_hpet_wdt_interrupt(struct hpet_hld_data *hdata)
> +{
> + unsigned long this_isr;
> + unsigned int lvl_trig;
> +
> + this_isr = hpet_readl(HPET_STATUS) & BIT(hdata->num);
> +
> + lvl_trig = hpet_readl(HPET_Tn_CFG(hdata->num)) & HPET_TN_LEVEL;
> +
> + if (lvl_trig && this_isr)
> + return true;
> +
> + return false;
> +}
>
> > +static int hardlockup_detector_nmi_handler(unsigned int val,
> > + struct pt_regs *regs)
> > +{
> > + struct hpet_hld_data *hdata = hld_data;
> > + unsigned int use_fsb;
> > +
> > + /*
> > + * If FSB delivery mode is used, the timer interrupt is programmed as
> > + * edge-triggered and there is no need to check the ISR register.
> > + */
> > + use_fsb = hdata->flags & HPET_DEV_FSB_CAP;
>
> Please do explain.. That FSB thing basically means MSI. But there's only
> a single NMI vector. How do we know this NMI came from the HPET?
Indeed, I see now that this is wrong. There is no way to know. The only way
is to use an IO APIC interrupt and read the HPET status register.
>
> > +
> > + if (!use_fsb && !is_hpet_wdt_interrupt(hdata))
>
> So you add _2_ HPET reads for every single NMI that gets triggered...
> and IIRC HPET reads are _sllooooowwwwww_.
Since the trigger mode of the HPET timer is not expected to change,
perhaps is_hpet_wdt_interrupt() can only need the interrupt status
register. This would reduce the reads to one. Furthermore, the hardlockup
detector can skip an X number of NMIs and reduce further the frequency
of reads. Does this make sense?
>
> > + return NMI_DONE;
> > +
> > + inspect_for_hardlockups(regs);
> > +
> > + if (!(hdata->flags & HPET_DEV_PERI_CAP))
> > + kick_timer(hdata);
> > +
> > + /* Acknowledge interrupt if in level-triggered mode */
> > + if (!use_fsb)
> > + hpet_writel(BIT(hdata->num), HPET_STATUS);
> > +
> > + return NMI_HANDLED;
>
> So if I read this right, when in FSB/MSI mode, we'll basically _always_
> claim every single NMI as handled?
>
> That's broken.
Yes, this is not correct. I will drop the functionality to use
FSB/MSI mode.
Thanks and BR,
Ricardo
next prev parent reply other threads:[~2018-06-15 2:11 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-13 0:57 [RFC PATCH 00/23] Implement an HPET-based hardlockup detector Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 01/23] x86/apic: Add a parameter for the APIC delivery mode Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 02/23] genirq: Introduce IRQD_DELIVER_AS_NMI Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 03/23] genirq: Introduce IRQF_DELIVER_AS_NMI Ricardo Neri
2018-06-13 8:34 ` Peter Zijlstra
2018-06-13 8:59 ` Julien Thierry
2018-06-13 9:20 ` Thomas Gleixner
2018-06-13 9:36 ` Julien Thierry
2018-06-13 9:49 ` Julien Thierry
2018-06-13 9:57 ` Thomas Gleixner
2018-06-13 10:25 ` Julien Thierry
2018-06-13 10:06 ` Marc Zyngier
2018-06-15 2:12 ` Ricardo Neri
2018-06-15 8:01 ` Julien Thierry
2018-06-16 0:39 ` Ricardo Neri
2018-06-16 13:36 ` Thomas Gleixner
2018-06-13 0:57 ` [RFC PATCH 04/23] iommu/vt-d/irq_remapping: Add support for IRQCHIP_CAN_DELIVER_AS_NMI Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 05/23] x86/msi: " Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 06/23] x86/ioapic: Add support for IRQCHIP_CAN_DELIVER_AS_NMI with interrupt remapping Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 07/23] x86/hpet: Expose more functions to read and write registers Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 08/23] x86/hpet: Calculate ticks-per-second in a separate function Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 09/23] x86/hpet: Reserve timer for the HPET hardlockup detector Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 10/23] x86/hpet: Relocate flag definitions to a header file Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 11/23] x86/hpet: Configure the timer used by the hardlockup detector Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 12/23] kernel/watchdog: Introduce a struct for NMI watchdog operations Ricardo Neri
2018-06-13 7:41 ` Nicholas Piggin
2018-06-13 8:42 ` Peter Zijlstra
2018-06-13 9:26 ` Thomas Gleixner
2018-06-13 11:52 ` Nicholas Piggin
2018-06-14 1:31 ` Ricardo Neri
2018-06-14 2:32 ` Nicholas Piggin
2018-06-14 8:32 ` Thomas Gleixner
2018-06-15 2:21 ` Ricardo Neri
2018-06-14 1:26 ` Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 13/23] watchdog/hardlockup: Define a generic function to detect hardlockups Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 14/23] watchdog/hardlockup: Decouple the hardlockup detector from perf Ricardo Neri
2018-06-13 8:43 ` Peter Zijlstra
2018-06-14 1:19 ` Ricardo Neri
2018-06-14 1:41 ` Nicholas Piggin
2018-06-15 2:23 ` Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 15/23] kernel/watchdog: Add a function to obtain the watchdog_allowed_mask Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 16/23] watchdog/hardlockup: Add an HPET-based hardlockup detector Ricardo Neri
2018-06-13 5:23 ` Randy Dunlap
2018-06-14 1:00 ` Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 17/23] watchdog/hardlockup/hpet: Convert the timer's interrupt to NMI Ricardo Neri
2018-06-13 9:07 ` Peter Zijlstra
2018-06-15 2:07 ` Ricardo Neri [this message]
2018-06-13 9:40 ` Thomas Gleixner
2018-06-15 2:03 ` Ricardo Neri
2018-06-15 9:19 ` Thomas Gleixner
2018-06-16 0:51 ` Ricardo Neri
2018-06-16 13:24 ` Thomas Gleixner
2018-06-20 0:15 ` Ricardo Neri
2018-06-20 0:25 ` Randy Dunlap
2018-06-21 0:25 ` Ricardo Neri
2018-06-20 7:47 ` Thomas Gleixner
2018-06-13 0:57 ` [RFC PATCH 18/23] watchdog/hardlockup/hpet: Add the NMI watchdog operations Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 19/23] watchdog/hardlockup: Make arch_touch_nmi_watchdog() to hpet-based implementation Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 20/23] watchdog/hardlockup/hpet: Rotate interrupt among all monitored CPUs Ricardo Neri
2018-06-13 9:48 ` Thomas Gleixner
2018-06-15 2:16 ` Ricardo Neri
2018-06-15 10:29 ` Thomas Gleixner
2018-06-16 0:46 ` Ricardo Neri
2018-06-16 13:27 ` Thomas Gleixner
2018-06-13 0:57 ` [RFC PATCH 21/23] watchdog/hardlockup/hpet: Adjust timer expiration on the number of " Ricardo Neri
2018-06-13 0:57 ` [RFC PATCH 22/23] watchdog/hardlockup/hpet: Only enable the HPET watchdog via a boot parameter Ricardo Neri
2018-06-13 5:26 ` Randy Dunlap
2018-06-14 0:58 ` Ricardo Neri
2018-06-14 3:30 ` Randy Dunlap
2018-06-13 0:57 ` [RFC PATCH 23/23] watchdog/hardlockup: Activate the HPET-based lockup detector Ricardo Neri
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180615020738.GB11625@voyager \
--to=ricardo.neri-calderon@linux$(echo .)intel.com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=andi.kleen@intel$(echo .)com \
--cc=ashok.raj@intel$(echo .)com \
--cc=ast@kernel$(echo .)org \
--cc=babu.moger@oracle$(echo .)com \
--cc=bp@suse$(echo .)de \
--cc=byungchul.park@lge$(echo .)com \
--cc=cdall@linaro$(echo .)org \
--cc=colin.king@canonical$(echo .)com \
--cc=dave@stgolabs$(echo .)net \
--cc=dzickus@redhat$(echo .)com \
--cc=frederic@kernel$(echo .)org \
--cc=hpa@zytor$(echo .)com \
--cc=iommu@lists$(echo .)linux-foundation.org \
--cc=jacob.jun.pan@intel$(echo .)com \
--cc=jpoimboe@redhat$(echo .)com \
--cc=kai.heng.feng@canonical$(echo .)com \
--cc=konrad.wilk@oracle$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=longman@redhat$(echo .)com \
--cc=marc.zyngier@arm$(echo .)com \
--cc=mathieu.desnoyers@efficios$(echo .)com \
--cc=mcgrof@kernel$(echo .)org \
--cc=mhiramat@kernel$(echo .)org \
--cc=mingo@kernel$(echo .)org \
--cc=mpe@ellerman$(echo .)id.au \
--cc=npiggin@gmail$(echo .)com \
--cc=paulmck@linux$(echo .)vnet.ibm.com \
--cc=peterz@infradead$(echo .)org \
--cc=pombredanne@nexb$(echo .)com \
--cc=rafael.j.wysocki@intel$(echo .)com \
--cc=ravi.v.shankar@intel$(echo .)com \
--cc=rdunlap@infradead$(echo .)org \
--cc=rientjes@google$(echo .)com \
--cc=sparclinux@vger$(echo .)kernel.org \
--cc=tglx@linutronix$(echo .)de \
--cc=tony.luck@intel$(echo .)com \
--cc=x86@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox