2019-10-10 08:44:02

by Li RongQing

Subject: [PATCH][RFC] perf/x86: avoid false-positives hard lockup

if perf counter is used as nmi watchdog, and twice nmi in soft
watchdog sample period will trigger hard lockup

make sure left time is not less than soft watchdog period by
compared with 3/5 period to skip forward, since soft watchdog
sample period is 2/5 of watchdog_thresh, nmi watchdog sample
period, computed by set_sample_period

Signed-off-by: Li RongQing <[email protected]>
---
arch/x86/events/core.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 7b21455d7504..1f5309456d4c 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1196,7 +1196,11 @@ int x86_perf_event_set_period(struct perf_event *event)
/*
* If we are way outside a reasonable range then just skip forward:
*/
+#ifdef CONFIG_HARDLOCKUP_DETECTOR_PERF
+ if (unlikely(left <= -(period * 3 / 5))) {
+#else
if (unlikely(left <= -period)) {
+#endif
left = period;
local64_set(&hwc->period_left, left);
hwc->last_period = period;
--
2.16.2

2019-10-10 09:47:33

by Peter Zijlstra

Subject: Re: [PATCH][RFC] perf/x86: avoid false-positives hard lockup

On Thu, Oct 10, 2019 at 04:41:38PM +0800, Li RongQing wrote:
> if perf counter is used as nmi watchdog, and twice nmi in soft
> watchdog sample period will trigger hard lockup
>
> make sure left time is not less than soft watchdog period by
> compared with 3/5 period to skip forward, since soft watchdog
> sample period is 2/5 of watchdog_thresh, nmi watchdog sample
> period, computed by set_sample_period
>
> Signed-off-by: Li RongQing <[email protected]>
> ---
> arch/x86/events/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 7b21455d7504..1f5309456d4c 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1196,7 +1196,11 @@ int x86_perf_event_set_period(struct perf_event *event)
> /*
> * If we are way outside a reasonable range then just skip forward:
> */
> +#ifdef CONFIG_HARDLOCKUP_DETECTOR_PERF
> + if (unlikely(left <= -(period * 3 / 5))) {
> +#else
> if (unlikely(left <= -period)) {
> +#endif

NAK. This is 100% the wrong place to do anything like that.