* [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
@ 2021-08-11 9:55 brookxu
2021-08-11 12:44 ` Thomas Gleixner
2021-08-11 13:00 ` kernel test robot
0 siblings, 2 replies; 8+ messages in thread
From: brookxu @ 2021-08-11 9:55 UTC (permalink / raw)
To: john.stultz, tglx, sboyd; +Cc: linux-kernel
From: Chunguang Xu <brookxu@tencent.com>
After patch 1f45f1f3 (clocksource: Make clocksource validation work
for all clocksources), md_nsec may be 0 in some scenarios, such as
the watchdog is delayed for a long time or the watchdog has a
time-warp.
We found a problem when testing nvme disks with fio, when multiple
queue interrupts of a disk were mapped to a single CPU. IO interrupt
processing will cause the watchdog to be delayed for a long time
(155 seconds), the system reports TSC unstable and switches the clock
to hpet. It seems that this scenario cannot be handled by optimizing
softirq. Therefore, when md_nsec returns 0, the machine or watchdog
should be in unstable state,the verification result not unreliable.
Is it possible for us to skip the current check at this time?
1. If the watchdog is delayed because the system is busy, and the
clocksource is switched to hpet due to a wrong judgment, the
performance degradation may directly cause the machine to be
unavailable and cause more problems.
2. If watchdog has time-warp, we should not rely on hpet to directly
mark TSC as unstable.
Later we register watchdog to other CPU, if other CPU is not busy, we
can also check the stability of TSC.
Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
kernel/time/clocksource.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index b89c76e1c02c..9b9014d67f1d 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -399,6 +399,13 @@ static void clocksource_watchdog(struct timer_list *unused)
cs->cs_last = csnow;
cs->wd_last = wdnow;
+ if (!wd_nsec) {
+ pr_warn("timekeeping watchdog on CPU%d seems hung up or unstable:");
+ pr_warn("'%s' wd_now: %llx wd_last: %llx mask: %llx\n",
+ watchdog->name, wdnow, wdlast, watchdog->mask);
+ continue;
+ }
+
if (atomic_read(&watchdog_reset_pending))
continue;
--
2.30.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 9:55 [RFC PATCH] clocksource: skip check while watchdog hung up or unstable brookxu
@ 2021-08-11 12:44 ` Thomas Gleixner
2021-08-11 13:18 ` brookxu
2021-08-11 13:00 ` kernel test robot
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2021-08-11 12:44 UTC (permalink / raw)
To: brookxu, john.stultz, sboyd; +Cc: linux-kernel
On Wed, Aug 11 2021 at 17:55, brookxu wrote:
> From: Chunguang Xu <brookxu@tencent.com>
>
> After patch 1f45f1f3 (clocksource: Make clocksource validation work
> for all clocksources), md_nsec may be 0 in some scenarios, such as
> the watchdog is delayed for a long time or the watchdog has a
> time-warp.
Maybe 0? There is exactly one single possibility for it to be zero:
cs->wd_last == wdnow, i.e. delta = 0 -> wd_nsec = 0
So how does that condition solve any long delay or wrap around of the
watchdog? It's more than unlikely to hit exactly this case where the
readout is identical to the previous readout unless the watchdog stopped
counting.
> We found a problem when testing nvme disks with fio, when multiple
> queue interrupts of a disk were mapped to a single CPU. IO interrupt
> processing will cause the watchdog to be delayed for a long time
> (155 seconds), the system reports TSC unstable and switches the clock
If you hold off the softirq from running for 155 seconds then the TSC
watchdog is the least of your problems.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 9:55 [RFC PATCH] clocksource: skip check while watchdog hung up or unstable brookxu
2021-08-11 12:44 ` Thomas Gleixner
@ 2021-08-11 13:00 ` kernel test robot
1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-08-11 13:00 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7695 bytes --]
Hi brookxu,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/timers/core]
[also build test WARNING on v5.14-rc5 next-20210811]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/brookxu/clocksource-skip-check-while-watchdog-hung-up-or-unstable/20210811-175707
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 1e7f7fbcd40c69d23e3fe641ead9f3dc128fa8aa
config: x86_64-randconfig-c001-20210810 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d39ebdae674c8efc84ebe8dc32716ec353220530)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/085c7bc9e8da6deabad14b2093bd8409cb77b3be
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review brookxu/clocksource-skip-check-while-watchdog-hung-up-or-unstable/20210811-175707
git checkout 085c7bc9e8da6deabad14b2093bd8409cb77b3be
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/time/clocksource.c:403:41: warning: more '%' conversions than data arguments [-Wformat-insufficient-args]
pr_warn("timekeeping watchdog on CPU%d seems hung up or unstable:");
~^
include/linux/printk.h:400:29: note: expanded from macro 'pr_warn'
printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
^~~
kernel/time/clocksource.c:8:41: note: expanded from macro 'pr_fmt'
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
^~~
1 warning generated.
vim +403 kernel/time/clocksource.c
352
353 static void clocksource_watchdog(struct timer_list *unused)
354 {
355 u64 csnow, wdnow, cslast, wdlast, delta;
356 int next_cpu, reset_pending;
357 int64_t wd_nsec, cs_nsec;
358 struct clocksource *cs;
359 u32 md;
360
361 spin_lock(&watchdog_lock);
362 if (!watchdog_running)
363 goto out;
364
365 reset_pending = atomic_read(&watchdog_reset_pending);
366
367 list_for_each_entry(cs, &watchdog_list, wd_list) {
368
369 /* Clocksource already marked unstable? */
370 if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
371 if (finished_booting)
372 schedule_work(&watchdog_work);
373 continue;
374 }
375
376 if (!cs_watchdog_read(cs, &csnow, &wdnow)) {
377 /* Clock readout unreliable, so give it up. */
378 __clocksource_unstable(cs);
379 continue;
380 }
381
382 /* Clocksource initialized ? */
383 if (!(cs->flags & CLOCK_SOURCE_WATCHDOG) ||
384 atomic_read(&watchdog_reset_pending)) {
385 cs->flags |= CLOCK_SOURCE_WATCHDOG;
386 cs->wd_last = wdnow;
387 cs->cs_last = csnow;
388 continue;
389 }
390
391 delta = clocksource_delta(wdnow, cs->wd_last, watchdog->mask);
392 wd_nsec = clocksource_cyc2ns(delta, watchdog->mult,
393 watchdog->shift);
394
395 delta = clocksource_delta(csnow, cs->cs_last, cs->mask);
396 cs_nsec = clocksource_cyc2ns(delta, cs->mult, cs->shift);
397 wdlast = cs->wd_last; /* save these in case we print them */
398 cslast = cs->cs_last;
399 cs->cs_last = csnow;
400 cs->wd_last = wdnow;
401
402 if (!wd_nsec) {
> 403 pr_warn("timekeeping watchdog on CPU%d seems hung up or unstable:");
404 pr_warn("'%s' wd_now: %llx wd_last: %llx mask: %llx\n",
405 watchdog->name, wdnow, wdlast, watchdog->mask);
406 continue;
407 }
408
409 if (atomic_read(&watchdog_reset_pending))
410 continue;
411
412 /* Check the deviation from the watchdog clocksource. */
413 md = cs->uncertainty_margin + watchdog->uncertainty_margin;
414 if (abs(cs_nsec - wd_nsec) > md) {
415 pr_warn("timekeeping watchdog on CPU%d: Marking clocksource '%s' as unstable because the skew is too large:\n",
416 smp_processor_id(), cs->name);
417 pr_warn(" '%s' wd_nsec: %lld wd_now: %llx wd_last: %llx mask: %llx\n",
418 watchdog->name, wd_nsec, wdnow, wdlast, watchdog->mask);
419 pr_warn(" '%s' cs_nsec: %lld cs_now: %llx cs_last: %llx mask: %llx\n",
420 cs->name, cs_nsec, csnow, cslast, cs->mask);
421 if (curr_clocksource == cs)
422 pr_warn(" '%s' is current clocksource.\n", cs->name);
423 else if (curr_clocksource)
424 pr_warn(" '%s' (not '%s') is current clocksource.\n", curr_clocksource->name, cs->name);
425 else
426 pr_warn(" No current clocksource.\n");
427 __clocksource_unstable(cs);
428 continue;
429 }
430
431 if (cs == curr_clocksource && cs->tick_stable)
432 cs->tick_stable(cs);
433
434 if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) &&
435 (cs->flags & CLOCK_SOURCE_IS_CONTINUOUS) &&
436 (watchdog->flags & CLOCK_SOURCE_IS_CONTINUOUS)) {
437 /* Mark it valid for high-res. */
438 cs->flags |= CLOCK_SOURCE_VALID_FOR_HRES;
439
440 /*
441 * clocksource_done_booting() will sort it if
442 * finished_booting is not set yet.
443 */
444 if (!finished_booting)
445 continue;
446
447 /*
448 * If this is not the current clocksource let
449 * the watchdog thread reselect it. Due to the
450 * change to high res this clocksource might
451 * be preferred now. If it is the current
452 * clocksource let the tick code know about
453 * that change.
454 */
455 if (cs != curr_clocksource) {
456 cs->flags |= CLOCK_SOURCE_RESELECT;
457 schedule_work(&watchdog_work);
458 } else {
459 tick_clock_notify();
460 }
461 }
462 }
463
464 /*
465 * We only clear the watchdog_reset_pending, when we did a
466 * full cycle through all clocksources.
467 */
468 if (reset_pending)
469 atomic_dec(&watchdog_reset_pending);
470
471 /*
472 * Cycle through CPUs to check if the CPUs stay synchronized
473 * to each other.
474 */
475 next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
476 if (next_cpu >= nr_cpu_ids)
477 next_cpu = cpumask_first(cpu_online_mask);
478
479 /*
480 * Arm timer if not already pending: could race with concurrent
481 * pair clocksource_stop_watchdog() clocksource_start_watchdog().
482 */
483 if (!timer_pending(&watchdog_timer)) {
484 watchdog_timer.expires += WATCHDOG_INTERVAL;
485 add_timer_on(&watchdog_timer, next_cpu);
486 }
487 out:
488 spin_unlock(&watchdog_lock);
489 }
490
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36329 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 12:44 ` Thomas Gleixner
@ 2021-08-11 13:18 ` brookxu
2021-08-11 14:01 ` Thomas Gleixner
0 siblings, 1 reply; 8+ messages in thread
From: brookxu @ 2021-08-11 13:18 UTC (permalink / raw)
To: Thomas Gleixner, john.stultz, sboyd; +Cc: linux-kernel
Thanks for your time.
Thomas Gleixner wrote on 2021/8/11 8:44 下午:
> On Wed, Aug 11 2021 at 17:55, brookxu wrote:
>> From: Chunguang Xu <brookxu@tencent.com>
>>
>> After patch 1f45f1f3 (clocksource: Make clocksource validation work
>> for all clocksources), md_nsec may be 0 in some scenarios, such as
>> the watchdog is delayed for a long time or the watchdog has a
>> time-warp.
>
> Maybe 0? There is exactly one single possibility for it to be zero:
>
> cs->wd_last == wdnow, i.e. delta = 0 -> wd_nsec = 0
>
> So how does that condition solve any long delay or wrap around of the
> watchdog? It's more than unlikely to hit exactly this case where the
> readout is identical to the previous readout unless the watchdog stopped
> counting.
Maybe I missed something. Like this example, when watchdog run ,hpet have
wrap around:
'hpet' wd_now: d76e5a69 wd_last: f929eb3c mask: ffffffff
We can calculate the number of elapsed cycles:
cycles = wd_now - wd_last = 0xde446f2d
clocksource_delta() uses the MSB to determine an invalid inteval and returns
0, but for 0xde446f2d, this judgment should be wrong.
>> We found a problem when testing nvme disks with fio, when multiple
>> queue interrupts of a disk were mapped to a single CPU. IO interrupt
>> processing will cause the watchdog to be delayed for a long time
>> (155 seconds), the system reports TSC unstable and switches the clock
>
> If you hold off the softirq from running for 155 seconds then the TSC
> watchdog is the least of your problems.
To be precise, we are processing interrupts in handle_edge_irq() for a long
time. Since the interrupts of multiple hardware queues are mapped to a single
CPU, multiple cores are continuously issuing IO, and then a single core is
processing IO. Perhaps the test case can be optimized, but shouldn't this lead
to switching clocks in principle?
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 13:18 ` brookxu
@ 2021-08-11 14:01 ` Thomas Gleixner
2021-08-11 15:26 ` brookxu
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2021-08-11 14:01 UTC (permalink / raw)
To: brookxu, john.stultz, sboyd; +Cc: linux-kernel
On Wed, Aug 11 2021 at 21:18, brookxu wrote:
> Thomas Gleixner wrote on 2021/8/11 8:44 下午:
>> On Wed, Aug 11 2021 at 17:55, brookxu wrote:
>>> From: Chunguang Xu <brookxu@tencent.com>
>>>
>>> After patch 1f45f1f3 (clocksource: Make clocksource validation work
>>> for all clocksources), md_nsec may be 0 in some scenarios, such as
>>> the watchdog is delayed for a long time or the watchdog has a
>>> time-warp.
>>
>> Maybe 0? There is exactly one single possibility for it to be zero:
>>
>> cs->wd_last == wdnow, i.e. delta = 0 -> wd_nsec = 0
>>
>> So how does that condition solve any long delay or wrap around of the
>> watchdog? It's more than unlikely to hit exactly this case where the
>> readout is identical to the previous readout unless the watchdog stopped
>> counting.
>
> Maybe I missed something. Like this example, when watchdog run ,hpet have
> wrap around:
>
> 'hpet' wd_now: d76e5a69 wd_last: f929eb3c mask: ffffffff
>
> We can calculate the number of elapsed cycles:
> cycles = wd_now - wd_last = 0xde446f2d
>
> clocksource_delta() uses the MSB to determine an invalid inteval and returns
> 0, but for 0xde446f2d, this judgment should be wrong.
You're right. I forgot about the MSB check which is enabled on x86.
>>> We found a problem when testing nvme disks with fio, when multiple
>>> queue interrupts of a disk were mapped to a single CPU. IO interrupt
>>> processing will cause the watchdog to be delayed for a long time
>>> (155 seconds), the system reports TSC unstable and switches the clock
>>
>> If you hold off the softirq from running for 155 seconds then the TSC
>> watchdog is the least of your problems.
>
> To be precise, we are processing interrupts in handle_edge_irq() for a long
> time. Since the interrupts of multiple hardware queues are mapped to a single
> CPU, multiple cores are continuously issuing IO, and then a single core is
> processing IO. Perhaps the test case can be optimized, but shouldn't this lead
> to switching clocks in principle?
The clocksource watchdog failure is only _ONE_ consequence. Processing
hard interrupts for 155 seconds straight will trigger lockup detectors
of all sorts if you have them enabled.
So just papering over the clocksource watchdog does not solve anything,
really. Next week you have to add similar hacks to the lockup detectors,
RCU and whatever.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 14:01 ` Thomas Gleixner
@ 2021-08-11 15:26 ` brookxu
2021-08-12 10:53 ` Thomas Gleixner
0 siblings, 1 reply; 8+ messages in thread
From: brookxu @ 2021-08-11 15:26 UTC (permalink / raw)
To: Thomas Gleixner, john.stultz, sboyd; +Cc: linux-kernel
Thanks for your time.
Thomas Gleixner wrote on 2021/8/11 22:01:
> On Wed, Aug 11 2021 at 21:18, brookxu wrote:
>> Thomas Gleixner wrote on 2021/8/11 8:44 下午:
>>> On Wed, Aug 11 2021 at 17:55, brookxu wrote:
>>>> From: Chunguang Xu <brookxu@tencent.com>
>>>>
>>>> After patch 1f45f1f3 (clocksource: Make clocksource validation work
>>>> for all clocksources), md_nsec may be 0 in some scenarios, such as
>>>> the watchdog is delayed for a long time or the watchdog has a
>>>> time-warp.
>>>
>>> Maybe 0? There is exactly one single possibility for it to be zero:
>>>
>>> cs->wd_last == wdnow, i.e. delta = 0 -> wd_nsec = 0
>>>
>>> So how does that condition solve any long delay or wrap around of the
>>> watchdog? It's more than unlikely to hit exactly this case where the
>>> readout is identical to the previous readout unless the watchdog stopped
>>> counting.
>>
>> Maybe I missed something. Like this example, when watchdog run ,hpet have
>> wrap around:
>>
>> 'hpet' wd_now: d76e5a69 wd_last: f929eb3c mask: ffffffff
>>
>> We can calculate the number of elapsed cycles:
>> cycles = wd_now - wd_last = 0xde446f2d
>>
>> clocksource_delta() uses the MSB to determine an invalid inteval and returns
>> 0, but for 0xde446f2d, this judgment should be wrong.
>
> You're right. I forgot about the MSB check which is enabled on x86.
>
>>>> We found a problem when testing nvme disks with fio, when multiple
>>>> queue interrupts of a disk were mapped to a single CPU. IO interrupt
>>>> processing will cause the watchdog to be delayed for a long time
>>>> (155 seconds), the system reports TSC unstable and switches the clock
>>>
>>> If you hold off the softirq from running for 155 seconds then the TSC
>>> watchdog is the least of your problems.
>>
>> To be precise, we are processing interrupts in handle_edge_irq() for a long
>> time. Since the interrupts of multiple hardware queues are mapped to a single
>> CPU, multiple cores are continuously issuing IO, and then a single core is
>> processing IO. Perhaps the test case can be optimized, but shouldn't this lead
>> to switching clocks in principle?
>
> The clocksource watchdog failure is only _ONE_ consequence. Processing
> hard interrupts for 155 seconds straight will trigger lockup detectors
> of all sorts if you have them enabled.
>
> So just papering over the clocksource watchdog does not solve anything,
> really. Next week you have to add similar hacks to the lockup detectors,
> RCU and whatever.
Yeah, we have observed soft lockup and RCU stall, but these behaviors are
expected because the current CPU scheduling is disabled. However, marking
TSC unstable is inconsistent with the actual situation. The worst problem
is that after the clocksource switched to hpet, the abnormal time will be
greatly prolonged due to the degradation of performance. We have not found
that soft lockup and RCU stall will affect the machine for a long time in
this test. Aside from these, as the watchdog is scheduled periodically, when
wd_nsec is 0, it means that something maybe abnormal, do we readlly still
need to continue to verify TSC? and how to ensure the correctness of the
results?
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-11 15:26 ` brookxu
@ 2021-08-12 10:53 ` Thomas Gleixner
2021-08-13 0:54 ` brookxu
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2021-08-12 10:53 UTC (permalink / raw)
To: brookxu, john.stultz, sboyd; +Cc: linux-kernel
On Wed, Aug 11 2021 at 23:26, brookxu wrote:
> Thomas Gleixner wrote on 2021/8/11 22:01:
>>> To be precise, we are processing interrupts in handle_edge_irq() for a long
>>> time. Since the interrupts of multiple hardware queues are mapped to a single
>>> CPU, multiple cores are continuously issuing IO, and then a single core is
>>> processing IO. Perhaps the test case can be optimized, but shouldn't this lead
>>> to switching clocks in principle?
>>
>> The clocksource watchdog failure is only _ONE_ consequence. Processing
>> hard interrupts for 155 seconds straight will trigger lockup detectors
>> of all sorts if you have them enabled.
>>
>> So just papering over the clocksource watchdog does not solve anything,
>> really. Next week you have to add similar hacks to the lockup detectors,
>> RCU and whatever.
>
> Yeah, we have observed soft lockup and RCU stall, but these behaviors are
> expected because the current CPU scheduling is disabled. However, marking
> TSC unstable is inconsistent with the actual situation. The worst problem
> is that after the clocksource switched to hpet, the abnormal time will be
> greatly prolonged due to the degradation of performance. We have not found
> that soft lockup and RCU stall will affect the machine for a long time in
> this test. Aside from these, as the watchdog is scheduled periodically, when
> wd_nsec is 0, it means that something maybe abnormal, do we readlly still
> need to continue to verify TSC? and how to ensure the correctness of the
> results?
Sorry no. While softlockups and RCU stalls might have no long term
effect in the first place, this argumentation vs. the clocksource
watchdog is just a strawman. You're abusing the system in a way which
causes it to malfunction so you have to live with the consequences.
Aside of that this 'workaround' is just duct taping a particular part of
the problem. What guarantees that after the interrupt storm subsided the
clocksource delta of the watchdog becomes 0 (negative)?
Absolutely nothing. The delta can be positive, but then the watchdog and
the TSC are not in sync anymore which will disable the TSC as well.
A 24MHz HPET has a wraparound time of ~178s which means during:
89s < tdelta < 178s
your hack papers over the problem. Any interrupt storm time outside of
that window results in fail.
Now run the same test on a machine with a 14MHz HPET and you get
153s < tdelta < 306s
so your 155s interrupt storm barely fits. And what are you doing with
your next test which runs only 80 seconds?
Not to talk about the fact that you wreckage detection of a watchdog
clocksource going stale.
So no, we are not adding hacks to support abuse.
What we really want to do is to add detection for interrupt storms of
this sort and shut those interrupts down for good.
Thanks,
tglx
---
Patient: "Doctor, it hurts when I hammer on my toe."
Doctor: "Don't do that then!"
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH] clocksource: skip check while watchdog hung up or unstable
2021-08-12 10:53 ` Thomas Gleixner
@ 2021-08-13 0:54 ` brookxu
0 siblings, 0 replies; 8+ messages in thread
From: brookxu @ 2021-08-13 0:54 UTC (permalink / raw)
To: Thomas Gleixner, john.stultz, sboyd; +Cc: linux-kernel
Thomas Gleixner wrote on 2021/8/12 6:53 下午:
> On Wed, Aug 11 2021 at 23:26, brookxu wrote:
>> Thomas Gleixner wrote on 2021/8/11 22:01:
>>>> To be precise, we are processing interrupts in handle_edge_irq() for a long
>>>> time. Since the interrupts of multiple hardware queues are mapped to a single
>>>> CPU, multiple cores are continuously issuing IO, and then a single core is
>>>> processing IO. Perhaps the test case can be optimized, but shouldn't this lead
>>>> to switching clocks in principle?
>>>
>>> The clocksource watchdog failure is only _ONE_ consequence. Processing
>>> hard interrupts for 155 seconds straight will trigger lockup detectors
>>> of all sorts if you have them enabled.
>>>
>>> So just papering over the clocksource watchdog does not solve anything,
>>> really. Next week you have to add similar hacks to the lockup detectors,
>>> RCU and whatever.
>>
>> Yeah, we have observed soft lockup and RCU stall, but these behaviors are
>> expected because the current CPU scheduling is disabled. However, marking
>> TSC unstable is inconsistent with the actual situation. The worst problem
>> is that after the clocksource switched to hpet, the abnormal time will be
>> greatly prolonged due to the degradation of performance. We have not found
>> that soft lockup and RCU stall will affect the machine for a long time in
>> this test. Aside from these, as the watchdog is scheduled periodically, when
>> wd_nsec is 0, it means that something maybe abnormal, do we readlly still
>> need to continue to verify TSC? and how to ensure the correctness of the
>> results?
>
> Sorry no. While softlockups and RCU stalls might have no long term
> effect in the first place, this argumentation vs. the clocksource
> watchdog is just a strawman. You're abusing the system in a way which
> causes it to malfunction so you have to live with the consequences.
>
> Aside of that this 'workaround' is just duct taping a particular part of
> the problem. What guarantees that after the interrupt storm subsided the
> clocksource delta of the watchdog becomes 0 (negative)?
>
> Absolutely nothing. The delta can be positive, but then the watchdog and
> the TSC are not in sync anymore which will disable the TSC as well.
>
> A 24MHz HPET has a wraparound time of ~178s which means during:
>
> 89s < tdelta < 178s
>
> your hack papers over the problem. Any interrupt storm time outside of
> that window results in fail.
>
> Now run the same test on a machine with a 14MHz HPET and you get
>
> 153s < tdelta < 306s
>
> so your 155s interrupt storm barely fits. And what are you doing with
> your next test which runs only 80 seconds?
>
> Not to talk about the fact that you wreckage detection of a watchdog
> clocksource going stale.
>
> So no, we are not adding hacks to support abuse.
>
> What we really want to do is to add detection for interrupt storms of
> this sort and shut those interrupts down for good.
ok, thanks for your suggestion.
> Thanks,
>
> tglx
> ---
> Patient: "Doctor, it hurts when I hammer on my toe."
> Doctor: "Don't do that then!"
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-13 0:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 9:55 [RFC PATCH] clocksource: skip check while watchdog hung up or unstable brookxu
2021-08-11 12:44 ` Thomas Gleixner
2021-08-11 13:18 ` brookxu
2021-08-11 14:01 ` Thomas Gleixner
2021-08-11 15:26 ` brookxu
2021-08-12 10:53 ` Thomas Gleixner
2021-08-13 0:54 ` brookxu
2021-08-11 13:00 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.