Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1450924 > unrolled thread
| Started by | Kyle Walker <kwalker@redhat.com> |
|---|---|
| First post | 2016-07-26 23:30 +0200 |
| Last post | 2016-08-05 21:10 +0200 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH resend] clocksource: Defer override invalidation unless clock is unstable Kyle Walker <kwalker@redhat.com> - 2016-07-26 23:30 +0200
Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable John Stultz <john.stultz@linaro.org> - 2016-07-26 23:40 +0200
Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable Kyle Walker <kwalker@redhat.com> - 2016-07-27 16:30 +0200
Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable Kyle Walker <kwalker@redhat.com> - 2016-08-05 20:40 +0200
Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable John Stultz <john.stultz@linaro.org> - 2016-08-05 21:10 +0200
| From | Kyle Walker <kwalker@redhat.com> |
|---|---|
| Date | 2016-07-26 23:30 +0200 |
| Subject | [PATCH resend] clocksource: Defer override invalidation unless clock is unstable |
| Message-ID | <rZi5j-7eZ-17@gated-at.bofh.it> |
The clock_select() operation will attempt to use the clocksource override
to apply the desired clocksource when the "clocksource=" boot parameter is
supplied. However, in the event that "clocksource=tsc" is used on a system
where there is a more desireable clocksource available, the boot parameter
fails. This is due to the TSC clocksource being installed unvalidated, but
the override being invalidated during the initial run through
clocksource_done_booting().
To address this condition, the override_name is only invalidated for
unstable clocksources. Otherwise, the override is left intact until after
the watchdog has validated the clocksource as stable/unstable.
Signed-off-by: Kyle Walker <kwalker@redhat.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
---
Notes:
Resend due to no feedback on the initial submit. Thank you in advance!
kernel/time/clocksource.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 56ece14..4c1bb2a 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -600,9 +600,18 @@ static void __clocksource_select(bool skipcur)
*/
if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
/* Override clocksource cannot be used. */
- pr_warn("Override clocksource %s is not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
- cs->name);
- override_name[0] = 0;
+ if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
+ pr_warn("Override clocksource %s is unstable and not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
+ cs->name);
+ override_name[0] = 0;
+ } else {
+ /*
+ * The override cannot be currently verified.
+ * Deferring to let the watchdog check.
+ */
+ pr_info("Override clocksource %s is not currently HRT compatible - deferring\n",
+ cs->name);
+ }
} else
/* Override clocksource can be used. */
best = cs;
--
2.5.5
[toc] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-07-26 23:40 +0200 |
| Subject | Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable |
| Message-ID | <rZieZ-7i5-13@gated-at.bofh.it> |
| In reply to | #1450924 |
Sorry for not getting back to you. This has been in my to-look-at list.
On Tue, Jul 26, 2016 at 2:24 PM, Kyle Walker <kwalker@redhat.com> wrote:
> The clock_select() operation will attempt to use the clocksource override
> to apply the desired clocksource when the "clocksource=" boot parameter is
> supplied. However, in the event that "clocksource=tsc" is used on a system
> where there is a more desireable clocksource available, the boot parameter
> fails. This is due to the TSC clocksource being installed unvalidated, but
> the override being invalidated during the initial run through
> clocksource_done_booting().
I've read this a few times, and I'm not sure I really understand it.
Can you give an example of a "more desirable clocksource" then the
TSC? Especially when the TSC was specified as a boot argument?
>
> To address this condition, the override_name is only invalidated for
> unstable clocksources. Otherwise, the override is left intact until after
> the watchdog has validated the clocksource as stable/unstable.
>
> Signed-off-by: Kyle Walker <kwalker@redhat.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> ---
>
> Notes:
> Resend due to no feedback on the initial submit. Thank you in advance!
>
> kernel/time/clocksource.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 56ece14..4c1bb2a 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -600,9 +600,18 @@ static void __clocksource_select(bool skipcur)
> */
> if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
> /* Override clocksource cannot be used. */
> - pr_warn("Override clocksource %s is not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
> - cs->name);
> - override_name[0] = 0;
> + if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
> + pr_warn("Override clocksource %s is unstable and not HRT compatible - cannot switch while in HRT/NOHZ mode\n",
> + cs->name);
> + override_name[0] = 0;
> + } else {
> + /*
> + * The override cannot be currently verified.
> + * Deferring to let the watchdog check.
> + */
> + pr_info("Override clocksource %s is not currently HRT compatible - deferring\n",
> + cs->name);
> + }
The logic here is confusing as well. So.. if the override is not HRT
compatible, we check if its stable or not? Once we're in HRT there's
not much likelyhood of us going into non HRT mode. I'm not sure what
the stability has to do with it here.
Sorry, could you explain the case you're running into in some further detail?
thanks
-john
[toc] | [prev] | [next] | [standalone]
| From | Kyle Walker <kwalker@redhat.com> |
|---|---|
| Date | 2016-07-27 16:30 +0200 |
| Subject | Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable |
| Message-ID | <rZy0u-y1-11@gated-at.bofh.it> |
| In reply to | #1450931 |
I'm so sorry for the duplicate, gmail managed to sneak in some HTML. Resending due to the mailing list correctly blocking the initial send. On Tue, Jul 26, 2016 at 5:36 PM, John Stultz <john.stultz@linaro.org> wrote: > Sorry for not getting back to you. This has been in my to-look-at list. No problem at all. Thanks for taking a look! > On Tue, Jul 26, 2016 at 2:24 PM, Kyle Walker <kwalker@redhat.com> wrote: >> The clock_select() operation will attempt to use the clocksource override >> to apply the desired clocksource when the "clocksource=" boot parameter is >> supplied. However, in the event that "clocksource=tsc" is used on a system >> where there is a more desireable clocksource available, the boot parameter >> fails. This is due to the TSC clocksource being installed unvalidated, but >> the override being invalidated during the initial run through >> clocksource_done_booting(). > > I've read this a few times, and I'm not sure I really understand it. > > Can you give an example of a "more desirable clocksource" then the > TSC? Especially when the TSC was specified as a boot argument? > I apologize for the confusion. By "more desireable", I mean that there is another clocksource that the system is wanting to use by default. For example, on Xen platforms, the "xen" clocksource is the "best" as determined by clocksource_find_best(), being at the top of the list. crash> list -s clocksource.name,rating clocksource.list -H clocksource_list ffffffff81c0cb00 name = 0xffffffff817ad585 "xen" rating = 400 ffffffff81a98540 name = 0xffffffff817c2873 "tsc" rating = 300 ffffffff81aa6580 name = 0xffffffff817b118a "hpet" rating = 250 ffffffff81b22b00 name = 0xffffffff817c02b7 "acpi_pm" rating = 120 ffffffff81ab48c0 name = 0xffffffff817c05c6 "jiffies" rating = 1 In that scenario, the xen clocksource would be used if no override was specified. > > The logic here is confusing as well. So.. if the override is not HRT > compatible, we check if its stable or not? Once we're in HRT there's > not much likelyhood of us going into non HRT mode. I'm not sure what > the stability has to do with it here. > > Sorry, could you explain the case you're running into in some further detail? The issue I'm running into is that the override is not HRT compatible yet. Though it will be later in the boot process, unless the clocksource watchdog marks the clocksource as unstable. The issue with the current implementation is that the override_name value is disabled when the tsc is first checked, before the watchdog has a chance to check it and mark it stable or unstable. Without patch: $ dmesg | grep -e clocksource <snip> clocksource: refined-jiffies: <snip> Kernel command line: <snip> clocksource=tsc clocksource: hpet: <snip> clocksource: xen: <snip> clocksource: jiffies: <snip> clocksource: Switched to clocksource xen clocksource: acpi_pm: <snip> tsc: Refined TSC clocksource calibration: 2394.399 MHz clocksource: tsc: <snip> clocksource: Override clocksource tsc is not HRT compatible - <snip> With patch: $ dmesg | grep -e clocksource <snip> clocksource: refined-jiffies:<snip> Kernel command line: <snip> clocksource=tsc clocksource: hpet: <snip> clocksource: xen: <snip> clocksource: jiffies: <snip> clocksource: Switched to clocksource xen clocksource: acpi_pm: <snip> tsc: Refined TSC clocksource calibration: 2394.461 MHz clocksource: tsc: <snip> clocksource: Override clocksource tsc is not currently HRT compatible - deferring clocksource: Switched to clocksource tsc Please let me know if there is any further clarification needed. Have a good one! -- Kyle Walker
[toc] | [prev] | [next] | [standalone]
| From | Kyle Walker <kwalker@redhat.com> |
|---|---|
| Date | 2016-08-05 20:40 +0200 |
| Subject | Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable |
| Message-ID | <s2Sch-2Ir-1@gated-at.bofh.it> |
| In reply to | #1451304 |
Good evening John, On Wed, Jul 27, 2016 at 10:29 AM, Kyle Walker <kwalker@redhat.com> wrote: > The issue I'm running into is that the override is not HRT compatible yet. > Though it will be later in the boot process, unless the clocksource watchdog > marks the clocksource as unstable. > > The issue with the current implementation is that the override_name value is > disabled when the tsc is first checked, before the watchdog has a chance to > check it and mark it stable or unstable. > > Without patch: > $ dmesg | grep -e clocksource > <snip> > clocksource: refined-jiffies: <snip> > Kernel command line: <snip> clocksource=tsc > clocksource: hpet: <snip> > clocksource: xen: <snip> > clocksource: jiffies: <snip> > clocksource: Switched to clocksource xen > clocksource: acpi_pm: <snip> > tsc: Refined TSC clocksource calibration: 2394.399 MHz > > clocksource: tsc: <snip> > clocksource: Override clocksource tsc is not HRT compatible - <snip> > > > With patch: > $ dmesg | grep -e clocksource > <snip> > clocksource: refined-jiffies:<snip> > Kernel command line: <snip> clocksource=tsc > clocksource: hpet: <snip> > clocksource: xen: <snip> > > clocksource: jiffies: <snip> > clocksource: Switched to clocksource xen > clocksource: acpi_pm: <snip> > tsc: Refined TSC clocksource calibration: 2394.461 MHz > clocksource: tsc: <snip> > clocksource: Override clocksource tsc is not currently HRT compatible > - deferring > clocksource: Switched to clocksource tsc > Is there anything else needed from my end? Please let me know if there is any further information or clarification I can provide. Have a great evening! -- Kyle Walker
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-08-05 21:10 +0200 |
| Subject | Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable |
| Message-ID | <s2SFj-3aN-11@gated-at.bofh.it> |
| In reply to | #1450931 |
On Wed, Jul 27, 2016 at 6:50 AM, Kyle Walker <kwalker@redhat.com> wrote: > On Tue, Jul 26, 2016 at 5:36 PM, John Stultz <john.stultz@linaro.org> wrote: >> The logic here is confusing as well. So.. if the override is not HRT >> compatible, we check if its stable or not? Once we're in HRT there's >> not much likelyhood of us going into non HRT mode. I'm not sure what >> the stability has to do with it here. >> >> Sorry, could you explain the case you're running into in some further >> detail? > > The issue I'm running into is that the override is not HRT compatible yet. > Though it will be later in the boot process, unless the clocksource watchdog > marks the clocksource as unstable. > > The issue with the current implementation is that the override_name value is > disabled when the tsc is first checked, before the watchdog has a chance to > check it and mark it stable or unstable. Hrm. Ok. I've missed that the setting of a clocksource to being VALID_FOR_HRES happens by the watchdog and was thinking it was more like the IS_CONTINUOUS flag. So with that detail made clear, the patch makes more sense. I'd definitely clear up the change log to explain that detail: "Clocksources don't get the VALID_FOR_HRES flag until they have been checked by a watchdog. However, when using an override, the clocksource_select logic will clear the override value if the clocksources is not marked VALID_FOR_HRES on that check. When using the boot arguments clocksource=<foo>, this selection can run before the watchdog, and can cause the override to be incorrectly cleared." Sorry for the slow response, keeping up with the merge window the last two weeks has been a little crazy. thanks -john
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web