Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1201039 > unrolled thread
| Started by | Shaohua Li <shli@fb.com> |
|---|---|
| First post | 2015-08-05 20:20 +0200 |
| Last post | 2015-08-17 18:30 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/2] clocksource: improve unstable clocksource detection Shaohua Li <shli@fb.com> - 2015-08-05 20:20 +0200
Re: [PATCH 1/2] clocksource: improve unstable clocksource detection Shaohua Li <shli@fb.com> - 2015-08-17 18:20 +0200
Re: [PATCH 1/2] clocksource: improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-17 18:30 +0200
| From | Shaohua Li <shli@fb.com> |
|---|---|
| Date | 2015-08-05 20:20 +0200 |
| Subject | [PATCH 1/2] clocksource: improve unstable clocksource detection |
| Message-ID | <pUbse-IU-5@gated-at.bofh.it> |
From time to time we saw TSC is marked as unstable in our systems, while
the CPUs declare to have stable TSC. Looking at the clocksource unstable
detection, there are two problems:
- watchdog clock source wrap. HPET is the most common watchdog clock
source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
can wrap in about 5 minutes.
- threshold isn't scaled against interval. The threshold is 0.0625s in
0.5s interval. What if the actual interval is bigger than 0.5s?
The watchdog runs in a timer bh, so hard/soft irq can defer its running.
Heavy network stack softirq can hog a cpu. IPMI driver can disable
interrupt for a very long time. The first problem is mostly we are
suffering I think.
Here is a simple patch to fix the issues. If the waterdog doesn't run
for a long time, we ignore the detection. This should work for the two
problems. For the second one, we probably doen't need to scale if the
interval isn't very long.
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
kernel/time/clocksource.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 841b72f..8417c83 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
static void __clocksource_change_rating(struct clocksource *cs, int rating);
/*
- * Interval: 0.5sec Threshold: 0.0625s
+ * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
*/
#define WATCHDOG_INTERVAL (HZ >> 1)
+#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
#define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
static void clocksource_watchdog_work(struct work_struct *work)
@@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
continue;
/* Check the deviation from the watchdog clocksource. */
- if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
+ if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
+ cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
+ wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
pr_warn("timekeeping watchdog: Marking clocksource '%s' as unstable because the skew is too large:\n",
cs->name);
pr_warn(" '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
--
1.8.5.6
--
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/
[toc] | [next] | [standalone]
| From | Shaohua Li <shli@fb.com> |
|---|---|
| Date | 2015-08-17 18:20 +0200 |
| Message-ID | <pYviG-402-13@gated-at.bofh.it> |
| In reply to | #1201039 |
ping, any comments?
On Wed, Aug 05, 2015 at 11:12:53AM -0700, Shaohua Li wrote:
> From time to time we saw TSC is marked as unstable in our systems, while
> the CPUs declare to have stable TSC. Looking at the clocksource unstable
> detection, there are two problems:
> - watchdog clock source wrap. HPET is the most common watchdog clock
> source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
> can wrap in about 5 minutes.
> - threshold isn't scaled against interval. The threshold is 0.0625s in
> 0.5s interval. What if the actual interval is bigger than 0.5s?
>
> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
> Heavy network stack softirq can hog a cpu. IPMI driver can disable
> interrupt for a very long time. The first problem is mostly we are
> suffering I think.
>
> Here is a simple patch to fix the issues. If the waterdog doesn't run
> for a long time, we ignore the detection. This should work for the two
> problems. For the second one, we probably doen't need to scale if the
> interval isn't very long.
>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
> kernel/time/clocksource.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 841b72f..8417c83 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
> static void __clocksource_change_rating(struct clocksource *cs, int rating);
>
> /*
> - * Interval: 0.5sec Threshold: 0.0625s
> + * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
> */
> #define WATCHDOG_INTERVAL (HZ >> 1)
> +#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
> #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
>
> static void clocksource_watchdog_work(struct work_struct *work)
> @@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
> continue;
>
> /* Check the deviation from the watchdog clocksource. */
> - if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
> + if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
> + cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
> + wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
> pr_warn("timekeeping watchdog: Marking clocksource '%s' as unstable because the skew is too large:\n",
> cs->name);
> pr_warn(" '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
> --
> 1.8.5.6
>
--
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/
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2015-08-17 18:30 +0200 |
| Message-ID | <pYvsn-4bw-29@gated-at.bofh.it> |
| In reply to | #1208691 |
On Mon, Aug 17, 2015 at 9:14 AM, Shaohua Li <shli@fb.com> wrote:
> ping, any comments?
>
> On Wed, Aug 05, 2015 at 11:12:53AM -0700, Shaohua Li wrote:
>> From time to time we saw TSC is marked as unstable in our systems, while
>> the CPUs declare to have stable TSC. Looking at the clocksource unstable
>> detection, there are two problems:
>> - watchdog clock source wrap. HPET is the most common watchdog clock
>> source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
>> can wrap in about 5 minutes.
>> - threshold isn't scaled against interval. The threshold is 0.0625s in
>> 0.5s interval. What if the actual interval is bigger than 0.5s?
>>
>> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
>> Heavy network stack softirq can hog a cpu. IPMI driver can disable
>> interrupt for a very long time. The first problem is mostly we are
>> suffering I think.
>>
>> Here is a simple patch to fix the issues. If the waterdog doesn't run
>> for a long time, we ignore the detection. This should work for the two
>> problems. For the second one, we probably doen't need to scale if the
>> interval isn't very long.
>>
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Signed-off-by: Shaohua Li <shli@fb.com>
>> ---
>> kernel/time/clocksource.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
>> index 841b72f..8417c83 100644
>> --- a/kernel/time/clocksource.c
>> +++ b/kernel/time/clocksource.c
>> @@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
>> static void __clocksource_change_rating(struct clocksource *cs, int rating);
>>
>> /*
>> - * Interval: 0.5sec Threshold: 0.0625s
>> + * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
>> */
>> #define WATCHDOG_INTERVAL (HZ >> 1)
>> +#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
>> #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
>>
>> static void clocksource_watchdog_work(struct work_struct *work)
>> @@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
>> continue;
>>
>> /* Check the deviation from the watchdog clocksource. */
>> - if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
>> + if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
>> + cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
>> + wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
>> pr_warn("timekeeping watchdog: Marking clocksource '%s' as unstable because the skew is too large:\n",
>> cs->name);
>> pr_warn(" '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
Sorry. I got back from vacation last week and haven't finished queuing
items from my inbox. It looks reasonable to me. I'm hoping to put
together my patchset for 4.3, today and this hopefully should make it
unless I run into anything concerning.
thanks
-john
--
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/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web