linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] MIPS: Loongson: ExtCC clocksource support
@ 2019-02-24  9:36 kernel
  2019-02-24  9:36 ` [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource kernel
  2019-02-24  9:36 ` [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping kernel
  0 siblings, 2 replies; 5+ messages in thread
From: kernel @ 2019-02-24  9:36 UTC (permalink / raw)
  To: linux-mips
  Cc: Wang Xuerui, Huacai Chen, Jiaxun Yang, James Hogan, Paul Burton,
	Ralf Baechle, linux-mips

From: Wang Xuerui <wangxuerui@qiniu.com>

Hi,

This is my WIP patchset to add support for using the on-chip ExtCC
(external counter) register as clocksource, to boost the real-time
performance on Loongson platform. I'm posting this to solicit comments
and get some of the unresolved questions answered.

* I don't have access to multi-socket ccNUMA Loongson boards. The code
  most certainly doesn't work on such hardware, so either someone with
  expertise would teach me how to do this, or get this done on their
  own.
* I'm not sure of the pipeline behavior of the rdhwr instruction used.
  The current implementation has a sync (roughly the x86 way), and I'm
  not sure if it can be safely omitted, or even moved into the delay
  slot on return.
* Clock skew can be a concern, but it seems there's no generic mechanism
  for clocksource calibration. The x86 code has one supporting PIT/HPET-
  based calibration, but that code never got extracted to common
  framework.
* The VDSO performance seems very bad comparing to x86, even after
  adjusting for the cycle frequency difference. But I haven't thoroughly
  profiled this and perhaps won't have enough spare time for doing so
  (kernel work isn't part of my day job).  So maybe someone could
  provide some hints in this area?

Can only come up with so many points for now; I'll add if more ideas
turn up.

P.S. I have to change to my personal address for outgoing mails; my
work address is hosted on QQ Enterprise Mail, which most certainly is
blocked altogether by linux-mips.org.

Wang Xuerui (2):
  MIPS: Loongson: add extcc clocksource
  MIPS: VDSO: support extcc-based timekeeping

 arch/mips/include/asm/clocksource.h           |  1 +
 arch/mips/include/asm/mach-loongson64/extcc.h | 26 +++++++++
 arch/mips/loongson64/Kconfig                  | 13 +++++
 arch/mips/loongson64/common/Makefile          |  5 ++
 arch/mips/loongson64/common/extcc.c           | 54 +++++++++++++++++++
 arch/mips/loongson64/common/time.c            |  7 +++
 arch/mips/vdso/gettimeofday.c                 |  8 +++
 7 files changed, 114 insertions(+)
 create mode 100644 arch/mips/include/asm/mach-loongson64/extcc.h
 create mode 100644 arch/mips/loongson64/common/extcc.c

-- 
2.20.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource
  2019-02-24  9:36 [RFC PATCH 0/2] MIPS: Loongson: ExtCC clocksource support kernel
@ 2019-02-24  9:36 ` kernel
  2019-02-28  3:12   ` Paul Burton
  2019-02-24  9:36 ` [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping kernel
  1 sibling, 1 reply; 5+ messages in thread
From: kernel @ 2019-02-24  9:36 UTC (permalink / raw)
  To: linux-mips
  Cc: Wang Xuerui, Huacai Chen, Jiaxun Yang, James Hogan, Paul Burton,
	Ralf Baechle, linux-mips

From: Wang Xuerui <wangxuerui@qiniu.com>

The ExtCC (external counter) is a Loongson GS464E-specific hardware
register that ticks every internal bus cycle.  Hence, the counter is
shared among cores of the same package and can be considered constant
frequency.  The frequency is same as maximum frequency on all Loongson
chips I have access to, and the resolution is very good.

A previous iteration of the feature directly ripped x86 arch code, thus
unsuitable for mainlining.  This time though it's implemented with the
generic clocksource framework for simplicity, but without HPET-assisted
calibration as in x86.  Instead, the ExtCC frequency is directly taken
from firmware.

Blindly trusting firmware-provided values indeed can lead to clock
drifts, of course, but according to own observation on a 3A3000 board
the drift is somewhat acceptable.  For example, during a certain test
run, the firmware advertised 1400.020 MHz, which was refined to
1400.027 MHz by the old ported calibration code.

The current code is tested on a single-socket Loongson 3A3000 board for
slightly over a week, while the old code incorporating essentially the
same logic was deployed for well over 2 years. Stability is perfect and
responsiveness improved greatly, according to cyclictest.

The cyclictest benchmark figures are to be filled later.

Signed-off-by: Wang Xuerui <wangxuerui@qiniu.com>
Tested-by: Wang Xuerui <wangxuerui@qiniu.com>
Cc: Huacai Chen <chenhc@lemote.com>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/mach-loongson64/extcc.h | 26 +++++++++
 arch/mips/loongson64/Kconfig                  | 13 +++++
 arch/mips/loongson64/common/Makefile          |  5 ++
 arch/mips/loongson64/common/extcc.c           | 55 +++++++++++++++++++
 arch/mips/loongson64/common/time.c            |  7 +++
 5 files changed, 106 insertions(+)
 create mode 100644 arch/mips/include/asm/mach-loongson64/extcc.h
 create mode 100644 arch/mips/loongson64/common/extcc.c

diff --git a/arch/mips/include/asm/mach-loongson64/extcc.h b/arch/mips/include/asm/mach-loongson64/extcc.h
new file mode 100644
index 000000000000..9e70f195441e
--- /dev/null
+++ b/arch/mips/include/asm/mach-loongson64/extcc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MACH_LOONGSON64_EXTCC_H
+#define __ASM_MACH_LOONGSON64_EXTCC_H
+
+extern void extcc_clocksource_init(void);
+
+// TODO: is the sync really needed on GS464E? Or if this is really the case,
+// is a weakly-ordered version of this suitable for coarser granularity,
+// just like in x86?
+static __always_inline u64 __extcc_read_ordered(void)
+{
+	u64 result;
+
+	__asm__ __volatile__(
+		".set	push\n\t"
+		".set	arch=mips32r2\n\t"
+		".set	noreorder\n\t"
+		"sync\n\t"
+		"rdhwr	%0, $30\n\t"
+		".set	pop\n"
+		: "=r"(result));
+
+	return result;
+}
+
+#endif /* __ASM_MACH_LOONGSON64_EXTCC_H */
diff --git a/arch/mips/loongson64/Kconfig b/arch/mips/loongson64/Kconfig
index 4c14a11525f4..a7213a25a6ab 100644
--- a/arch/mips/loongson64/Kconfig
+++ b/arch/mips/loongson64/Kconfig
@@ -123,6 +123,19 @@ config RS780_HPET
 
 	  If unsure, say Yes.
 
+config LOONGSON_EXTCC_CLKSRC
+	bool "GS464E external counter clocksource"
+	depends on LOONGSON3_ENHANCEMENT && GENERIC_SCHED_CLOCK
+	help
+	  This option enables the external counter found on GS464E chips to
+	  be used as clocksource.
+
+	  The external counter is an internal cycle counter independent of
+	  processor cores, and provides very good (nanosecond-scale) time
+	  resolution.
+
+	  If unsure, say Yes.
+
 config LOONGSON_UART_BASE
 	bool
 	default y
diff --git a/arch/mips/loongson64/common/Makefile b/arch/mips/loongson64/common/Makefile
index 684624f61f5a..5416c794123f 100644
--- a/arch/mips/loongson64/common/Makefile
+++ b/arch/mips/loongson64/common/Makefile
@@ -25,3 +25,8 @@ obj-$(CONFIG_CS5536) += cs5536/
 #
 
 obj-$(CONFIG_SUSPEND) += pm.o
+
+#
+# ExtCC clocksource support
+#
+obj-$(CONFIG_LOONGSON_EXTCC_CLKSRC) += extcc.o
diff --git a/arch/mips/loongson64/common/extcc.c b/arch/mips/loongson64/common/extcc.c
new file mode 100644
index 000000000000..702cb389856a
--- /dev/null
+++ b/arch/mips/loongson64/common/extcc.c
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/sched_clock.h>
+
+#include <loongson.h>
+#include <extcc.h>
+
+static u64 notrace extcc_read(struct clocksource *cs)
+{
+	return __extcc_read_ordered();
+}
+
+static u64 notrace extcc_sched_clock(void)
+{
+	return __extcc_read_ordered();
+}
+
+static struct clocksource extcc_clocksource = {
+	.name		= "extcc",
+	.read		= extcc_read,
+	.mask		= CLOCKSOURCE_MASK(64),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_VALID_FOR_HRES,
+	/* TODO later */
+	.archdata	= { .vdso_clock_mode = VDSO_CLOCK_NONE },
+};
+
+void __init extcc_clocksource_init(void)
+{
+	/* trust the firmware-provided frequency */
+	u32 extcc_frequency = cpu_clock_freq;
+	int ret;
+
+	if (extcc_frequency == 0) {
+		pr_err("Frequency not specified\n");
+		return;
+	}
+
+	/*
+	 * As for the rating, 200+ is good while 300+ is desirable.
+	 * Use 1GHz as bar for "desirable"; most Loongson processors with support
+	 * for ExtCC already satisfy this.
+	 */
+	extcc_clocksource.rating = 200 + extcc_frequency / 10000000;
+
+	ret = clocksource_register_hz(&extcc_clocksource, extcc_frequency);
+	if (ret < 0)
+		pr_warn("Unable to register clocksource\n");
+
+	/* mark extcc as sched clock */
+	sched_clock_register(extcc_sched_clock, 64, extcc_frequency);
+}
+
diff --git a/arch/mips/loongson64/common/time.c b/arch/mips/loongson64/common/time.c
index 0ba53c55ff33..afacc50b7501 100644
--- a/arch/mips/loongson64/common/time.c
+++ b/arch/mips/loongson64/common/time.c
@@ -16,6 +16,9 @@
 
 #include <loongson.h>
 #include <cs5536/cs5536_mfgpt.h>
+#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
+#include <extcc.h>
+#endif
 
 void __init plat_time_init(void)
 {
@@ -27,6 +30,10 @@ void __init plat_time_init(void)
 #else
 	setup_mfgpt0_timer();
 #endif
+
+#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
+	extcc_clocksource_init();
+#endif
 }
 
 void read_persistent_clock64(struct timespec64 *ts)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping
  2019-02-24  9:36 [RFC PATCH 0/2] MIPS: Loongson: ExtCC clocksource support kernel
  2019-02-24  9:36 ` [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource kernel
@ 2019-02-24  9:36 ` kernel
  2019-02-28  3:17   ` Paul Burton
  1 sibling, 1 reply; 5+ messages in thread
From: kernel @ 2019-02-24  9:36 UTC (permalink / raw)
  To: linux-mips
  Cc: Wang Xuerui, Huacai Chen, Jiaxun Yang, James Hogan, Paul Burton,
	Ralf Baechle, linux-mips

From: Wang Xuerui <wangxuerui@qiniu.com>

The clocksource bits are ready, just wire things up in VDSO for a
significant user-space timekeeping performance gain.  There are several
ABI problems uncovered by vdsotest though, but daily usage of the test
system didn't expose any of them.  These inconsistencies will be fixed
in a later commit (presently TODO).

According to vdsotest (formatting is manually added, only the affected
figures are shown):

    category                          before  after
    --------                          ------  -----
    clock-gettime-monotonic: syscall: 409     401 nsec/call
    clock-gettime-monotonic:    libc: 476     141 nsec/call
    clock-gettime-monotonic:    vdso: 462     123 nsec/call

    clock-gettime-realtime:  syscall: 405     407 nsec/call
    clock-gettime-realtime:     libc: 474     142 nsec/call
    clock-gettime-realtime:     vdso: 457     125 nsec/call

    gettimeofday:            syscall: 406     407 nsec/call
    gettimeofday:               libc: 455     121 nsec/call
    gettimeofday:               vdso: 440     102 nsec/call

The benchmark was run on a single-socket 3A3000 @ 1.4GHz.  There is
still plenty of headroom for improvements of course, but let's take
care of the micro-optimizations later.

Signed-off-by: Wang Xuerui <wangxuerui@qiniu.com>
Tested-by: Wang Xuerui <wangxuerui@qiniu.com>
Cc: Huacai Chen <chenhc@lemote.com>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/clocksource.h | 1 +
 arch/mips/loongson64/common/extcc.c | 3 +--
 arch/mips/vdso/gettimeofday.c       | 8 ++++++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/clocksource.h b/arch/mips/include/asm/clocksource.h
index 3deb1d0c1a94..6a126a475892 100644
--- a/arch/mips/include/asm/clocksource.h
+++ b/arch/mips/include/asm/clocksource.h
@@ -17,6 +17,7 @@
 #define VDSO_CLOCK_NONE		0	/* No suitable clocksource. */
 #define VDSO_CLOCK_R4K		1	/* Use the coprocessor 0 count. */
 #define VDSO_CLOCK_GIC		2	/* Use the GIC. */
+#define VDSO_CLOCK_EXTCC	3	/* Use the Loongson ExtCC. */
 
 /**
  * struct arch_clocksource_data - Architecture-specific clocksource information.
diff --git a/arch/mips/loongson64/common/extcc.c b/arch/mips/loongson64/common/extcc.c
index 702cb389856a..0f6775099411 100644
--- a/arch/mips/loongson64/common/extcc.c
+++ b/arch/mips/loongson64/common/extcc.c
@@ -23,8 +23,7 @@ static struct clocksource extcc_clocksource = {
 	.read		= extcc_read,
 	.mask		= CLOCKSOURCE_MASK(64),
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_VALID_FOR_HRES,
-	/* TODO later */
-	.archdata	= { .vdso_clock_mode = VDSO_CLOCK_NONE },
+	.archdata	= { .vdso_clock_mode = VDSO_CLOCK_EXTCC },
 };
 
 void __init extcc_clocksource_init(void)
diff --git a/arch/mips/vdso/gettimeofday.c b/arch/mips/vdso/gettimeofday.c
index e22b422f282c..92eef8de36a4 100644
--- a/arch/mips/vdso/gettimeofday.c
+++ b/arch/mips/vdso/gettimeofday.c
@@ -17,6 +17,9 @@
 #include <asm/io.h>
 #include <asm/unistd.h>
 #include <asm/vdso.h>
+#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
+#include <asm/mach-loongson64/extcc.h>
+#endif
 
 #ifdef CONFIG_MIPS_CLOCK_VSYSCALL
 
@@ -148,6 +151,11 @@ static __always_inline u64 get_ns(const union mips_vdso_data *data)
 	case VDSO_CLOCK_GIC:
 		cycle_now = read_gic_count(data);
 		break;
+#endif
+#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
+	case VDSO_CLOCK_EXTCC:
+		cycle_now = __extcc_read_ordered();
+		break;
 #endif
 	default:
 		return 0;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource
  2019-02-24  9:36 ` [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource kernel
@ 2019-02-28  3:12   ` Paul Burton
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Burton @ 2019-02-28  3:12 UTC (permalink / raw)
  To: kernel
  Cc: linux-mips, Wang Xuerui, Huacai Chen, Jiaxun Yang, James Hogan,
	Ralf Baechle, linux-mips

Hi Wang,

Thanks for the patch - comments inline below.

On Sun, Feb 24, 2019 at 05:36:34PM +0800, kernel@xen0n.name wrote:
> From: Wang Xuerui <wangxuerui@qiniu.com>
> 
> The ExtCC (external counter) is a Loongson GS464E-specific hardware
> register that ticks every internal bus cycle.  Hence, the counter is
> shared among cores of the same package and can be considered constant
> frequency.  The frequency is same as maximum frequency on all Loongson
> chips I have access to, and the resolution is very good.
> 
> A previous iteration of the feature directly ripped x86 arch code, thus
> unsuitable for mainlining.  This time though it's implemented with the
> generic clocksource framework for simplicity, but without HPET-assisted
> calibration as in x86.  Instead, the ExtCC frequency is directly taken
> from firmware.
> 
> Blindly trusting firmware-provided values indeed can lead to clock
> drifts, of course, but according to own observation on a 3A3000 board
> the drift is somewhat acceptable.  For example, during a certain test
> run, the firmware advertised 1400.020 MHz, which was refined to
> 1400.027 MHz by the old ported calibration code.
> 
> The current code is tested on a single-socket Loongson 3A3000 board for
> slightly over a week, while the old code incorporating essentially the
> same logic was deployed for well over 2 years. Stability is perfect and
> responsiveness improved greatly, according to cyclictest.
> 
> The cyclictest benchmark figures are to be filled later.
> 
> Signed-off-by: Wang Xuerui <wangxuerui@qiniu.com>
> Tested-by: Wang Xuerui <wangxuerui@qiniu.com>

That should really be a given :) I'd drop the Tested-by: for your own
patch.

> Cc: Huacai Chen <chenhc@lemote.com>
> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> ---
>  arch/mips/include/asm/mach-loongson64/extcc.h | 26 +++++++++
>  arch/mips/loongson64/Kconfig                  | 13 +++++
>  arch/mips/loongson64/common/Makefile          |  5 ++
>  arch/mips/loongson64/common/extcc.c           | 55 +++++++++++++++++++
>  arch/mips/loongson64/common/time.c            |  7 +++
>  5 files changed, 106 insertions(+)
>  create mode 100644 arch/mips/include/asm/mach-loongson64/extcc.h
>  create mode 100644 arch/mips/loongson64/common/extcc.c
> 
> diff --git a/arch/mips/include/asm/mach-loongson64/extcc.h b/arch/mips/include/asm/mach-loongson64/extcc.h
> new file mode 100644
> index 000000000000..9e70f195441e
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-loongson64/extcc.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_MACH_LOONGSON64_EXTCC_H
> +#define __ASM_MACH_LOONGSON64_EXTCC_H
> +
> +extern void extcc_clocksource_init(void);
> +
> +// TODO: is the sync really needed on GS464E? Or if this is really the case,
> +// is a weakly-ordered version of this suitable for coarser granularity,
> +// just like in x86?

Please use the standard kernel comment style:

/*
 * TODO: ...
 */

> +static __always_inline u64 __extcc_read_ordered(void)
> +{
> +	u64 result;
> +
> +	__asm__ __volatile__(
> +		".set	push\n\t"
> +		".set	arch=mips32r2\n\t"

Generally I'd prefer:

	".set\t" MIPS_ISA_LEVEL "\n\t"

> +		".set	noreorder\n\t"

The .set noreorder directive here is redundant - there are no branches
or jumps in this inline asm & thus no delay slots for the assembler to
reorder instructions into.

> +		"sync\n\t"
> +		"rdhwr	%0, $30\n\t"
> +		".set	pop\n"
> +		: "=r"(result));
> +
> +	return result;
> +}
> +
> +#endif /* __ASM_MACH_LOONGSON64_EXTCC_H */
> diff --git a/arch/mips/loongson64/Kconfig b/arch/mips/loongson64/Kconfig
> index 4c14a11525f4..a7213a25a6ab 100644
> --- a/arch/mips/loongson64/Kconfig
> +++ b/arch/mips/loongson64/Kconfig
> @@ -123,6 +123,19 @@ config RS780_HPET
>  
>  	  If unsure, say Yes.
>  
> +config LOONGSON_EXTCC_CLKSRC
> +	bool "GS464E external counter clocksource"
> +	depends on LOONGSON3_ENHANCEMENT && GENERIC_SCHED_CLOCK
> +	help
> +	  This option enables the external counter found on GS464E chips to
> +	  be used as clocksource.
> +
> +	  The external counter is an internal cycle counter independent of
> +	  processor cores, and provides very good (nanosecond-scale) time
> +	  resolution.
> +
> +	  If unsure, say Yes.
> +
>  config LOONGSON_UART_BASE
>  	bool
>  	default y
> diff --git a/arch/mips/loongson64/common/Makefile b/arch/mips/loongson64/common/Makefile
> index 684624f61f5a..5416c794123f 100644
> --- a/arch/mips/loongson64/common/Makefile
> +++ b/arch/mips/loongson64/common/Makefile
> @@ -25,3 +25,8 @@ obj-$(CONFIG_CS5536) += cs5536/
>  #
>  
>  obj-$(CONFIG_SUSPEND) += pm.o
> +
> +#
> +# ExtCC clocksource support
> +#
> +obj-$(CONFIG_LOONGSON_EXTCC_CLKSRC) += extcc.o
> diff --git a/arch/mips/loongson64/common/extcc.c b/arch/mips/loongson64/common/extcc.c
> new file mode 100644
> index 000000000000..702cb389856a
> --- /dev/null
> +++ b/arch/mips/loongson64/common/extcc.c

I think this code belongs under drivers/clocksource/, and the relevant
maintainers should be copied:

$ ./scripts/get_maintainer.pl -f drivers/clocksource
Daniel Lezcano <daniel.lezcano@linaro.org> (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS)
Thomas Gleixner <tglx@linutronix.de> (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS)
linux-kernel@vger.kernel.org (open list:CLOCKSOURCE, CLOCKEVENT DRIVERS)

> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/clocksource.h>
> +#include <linux/init.h>
> +#include <linux/sched_clock.h>
> +
> +#include <loongson.h>
> +#include <extcc.h>
> +
> +static u64 notrace extcc_read(struct clocksource *cs)
> +{
> +	return __extcc_read_ordered();
> +}
> +
> +static u64 notrace extcc_sched_clock(void)
> +{
> +	return __extcc_read_ordered();
> +}
> +
> +static struct clocksource extcc_clocksource = {
> +	.name		= "extcc",
> +	.read		= extcc_read,
> +	.mask		= CLOCKSOURCE_MASK(64),
> +	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_VALID_FOR_HRES,
> +	/* TODO later */
> +	.archdata	= { .vdso_clock_mode = VDSO_CLOCK_NONE },
> +};
> +
> +void __init extcc_clocksource_init(void)
> +{
> +	/* trust the firmware-provided frequency */
> +	u32 extcc_frequency = cpu_clock_freq;
> +	int ret;
> +
> +	if (extcc_frequency == 0) {
> +		pr_err("Frequency not specified\n");
> +		return;
> +	}

Is this actually possible? The loongson64 implementation of
prom_init_env() seems to always assign a non-zero value to
cpu_clock_freq.

> +
> +	/*
> +	 * As for the rating, 200+ is good while 300+ is desirable.
> +	 * Use 1GHz as bar for "desirable"; most Loongson processors with support
> +	 * for ExtCC already satisfy this.
> +	 */
> +	extcc_clocksource.rating = 200 + extcc_frequency / 10000000;
> +
> +	ret = clocksource_register_hz(&extcc_clocksource, extcc_frequency);
> +	if (ret < 0)
> +		pr_warn("Unable to register clocksource\n");
> +
> +	/* mark extcc as sched clock */
> +	sched_clock_register(extcc_sched_clock, 64, extcc_frequency);
> +}
> +
> diff --git a/arch/mips/loongson64/common/time.c b/arch/mips/loongson64/common/time.c
> index 0ba53c55ff33..afacc50b7501 100644
> --- a/arch/mips/loongson64/common/time.c
> +++ b/arch/mips/loongson64/common/time.c
> @@ -16,6 +16,9 @@
>  
>  #include <loongson.h>
>  #include <cs5536/cs5536_mfgpt.h>
> +#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
> +#include <extcc.h>
> +#endif

Drop the #ifdef for header inclusion - it'll be harmless to include the
header even if nothing from it is used.

Thanks,
    Paul

>  
>  void __init plat_time_init(void)
>  {
> @@ -27,6 +30,10 @@ void __init plat_time_init(void)
>  #else
>  	setup_mfgpt0_timer();
>  #endif
> +
> +#ifdef CONFIG_LOONGSON_EXTCC_CLKSRC
> +	extcc_clocksource_init();
> +#endif
>  }
>  
>  void read_persistent_clock64(struct timespec64 *ts)
> -- 
> 2.20.1
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping
  2019-02-24  9:36 ` [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping kernel
@ 2019-02-28  3:17   ` Paul Burton
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Burton @ 2019-02-28  3:17 UTC (permalink / raw)
  To: kernel
  Cc: linux-mips, Wang Xuerui, Huacai Chen, Jiaxun Yang, James Hogan,
	Ralf Baechle, linux-mips

Hi Wang,

On Sun, Feb 24, 2019 at 05:36:35PM +0800, kernel@xen0n.name wrote:
> From: Wang Xuerui <wangxuerui@qiniu.com>
> 
> The clocksource bits are ready, just wire things up in VDSO for a
> significant user-space timekeeping performance gain.  There are several
> ABI problems uncovered by vdsotest though, but daily usage of the test
> system didn't expose any of them.  These inconsistencies will be fixed
> in a later commit (presently TODO).

I'm afraid that's not how this works... :)

We should make sure this works properly before merging it - could you
provide details of the problems vdsotest found?

> According to vdsotest (formatting is manually added, only the affected
> figures are shown):
> 
>     category                          before  after
>     --------                          ------  -----
>     clock-gettime-monotonic: syscall: 409     401 nsec/call
>     clock-gettime-monotonic:    libc: 476     141 nsec/call
>     clock-gettime-monotonic:    vdso: 462     123 nsec/call
> 
>     clock-gettime-realtime:  syscall: 405     407 nsec/call
>     clock-gettime-realtime:     libc: 474     142 nsec/call
>     clock-gettime-realtime:     vdso: 457     125 nsec/call
> 
>     gettimeofday:            syscall: 406     407 nsec/call
>     gettimeofday:               libc: 455     121 nsec/call
>     gettimeofday:               vdso: 440     102 nsec/call

I'd rather be sure the kernel gives the right answer than have it give
the wrong answer 4x faster, so this looks promising but let's fix the
test failures before moving forwards.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-28  3:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-24  9:36 [RFC PATCH 0/2] MIPS: Loongson: ExtCC clocksource support kernel
2019-02-24  9:36 ` [RFC PATCH 1/2] MIPS: Loongson: add extcc clocksource kernel
2019-02-28  3:12   ` Paul Burton
2019-02-24  9:36 ` [RFC PATCH 2/2] MIPS: VDSO: support extcc-based timekeeping kernel
2019-02-28  3:17   ` Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).