linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] MIPS: clocksource cumulative enhancements
@ 2024-06-12  8:54 Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 1/7] MIPS: csrc-r4k: Refine rating computation Jiaxun Yang
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang,
	Philippe Mathieu-Daudé

Hi all,

This series combined many enhancements for MIPS clocksource subsystems,
It improved r4k count synchronisation process, clock source rating for
selection, sched_clock eligibility and so on.

It seems fixed random RCU stall issue on Loongson 3A4000 multi-node
system and some boot failures on QEMU.

Please review.

Thanks 

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Changes in v2:
- Fix number of zeros in rating computation (Maciej)
- Only select HAVE_UNSTABLE_SCHED_CLOCK for SMP (Maciej)
- Link to v1: https://lore.kernel.org/r/20240511-mips-clks-v1-0-ddb4a10ee9f9@flygoat.com

---
Jiaxun Yang (7):
      MIPS: csrc-r4k: Refine rating computation
      MIPS: csrc-r4k: Apply verification clocksource flags
      MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
      MIPS: csrc-r4k: Don't register as sched_clock if unfit
      MIPS: sync-r4k: Rework based on x86 tsc_sync
      clocksource: mips-gic-timer: Refine rating computation
      clocksource: mips-gic-timer: Correct sched_clock width

 arch/mips/Kconfig                    |   1 +
 arch/mips/include/asm/r4k-timer.h    |   5 -
 arch/mips/kernel/csrc-r4k.c          |  24 ++-
 arch/mips/kernel/smp.c               |   2 -
 arch/mips/kernel/sync-r4k.c          | 281 +++++++++++++++++++++++++----------
 drivers/clocksource/mips-gic-timer.c |  20 ++-
 6 files changed, 234 insertions(+), 99 deletions(-)
---
base-commit: 704ba27ac55579704ba1289392448b0c66b56258
change-id: 20240509-mips-clks-9001264fcfbe

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>


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

* [PATCH v2 1/7] MIPS: csrc-r4k: Refine rating computation
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags Jiaxun Yang
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

Increase frequency addend dividend to 10000000 (10MHz) to
reasonably accommodate multi GHz level mips_hpt_frequency.

Cap rating of csrc-r4k into 299 to ensure it doesn't go into
"Desired" range, given all the drama we have with CP0 count
registers (SMP sync, behaviour on wait etc).

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
v2: Fix up number of zeros for 10 MHz (Maciej)
---
 arch/mips/kernel/csrc-r4k.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index edc4afc080fa..f02ae333f4f9 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -111,7 +111,8 @@ int __init init_r4k_clocksource(void)
 		return -ENXIO;
 
 	/* Calculate a somewhat reasonable rating value */
-	clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
+	clocksource_mips.rating = 200;
+	clocksource_mips.rating += clamp(mips_hpt_frequency / 10000000, 0, 99);
 
 	/*
 	 * R2 onwards makes the count accessible to user mode so it can be used

-- 
2.43.0


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

* [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 1/7] MIPS: csrc-r4k: Refine rating computation Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-08-06  4:09   ` Guenter Roeck
  2024-06-12  8:54 ` [PATCH v2 3/7] MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT Jiaxun Yang
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

CP0 counter suffers from various problems like SMP sync,
behaviour on wait.

Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
as what x86 did to TSC, to let kernel test it before use.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/kernel/csrc-r4k.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index f02ae333f4f9..055747a7417d 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -21,7 +21,9 @@ static struct clocksource clocksource_mips = {
 	.name		= "MIPS",
 	.read		= c0_hpt_read,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS |
+				  CLOCK_SOURCE_MUST_VERIFY |
+				  CLOCK_SOURCE_VERIFY_PERCPU,
 };
 
 static u64 __maybe_unused notrace r4k_read_sched_clock(void)

-- 
2.43.0


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

* [PATCH v2 3/7] MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 1/7] MIPS: csrc-r4k: Refine rating computation Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 4/7] MIPS: csrc-r4k: Don't register as sched_clock if unfit Jiaxun Yang
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

csrc-r4k suffers from SMP synchronization overhead.

Select HAVE_UNSTABLE_SCHED_CLOCK to workaround drift
between the CPUs on the system. HAVE_UNSTABLE_SCHED_CLOCK
requires cmpxchg64, so enable it for 64 bits only.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
v2: Depends on SMP as well
---
 arch/mips/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index f1aa1bf11166..656df5d4097d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1083,6 +1083,7 @@ config CSRC_IOASIC
 
 config CSRC_R4K
 	select CLOCKSOURCE_WATCHDOG if CPU_FREQ
+	select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
 	bool
 
 config CSRC_SB1250

-- 
2.43.0


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

* [PATCH v2 4/7] MIPS: csrc-r4k: Don't register as sched_clock if unfit
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (2 preceding siblings ...)
  2024-06-12  8:54 ` [PATCH v2 3/7] MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 5/7] MIPS: sync-r4k: Rework based on x86 tsc_sync Jiaxun Yang
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

When we have more than one CPU in system, counter synchronisation
overhead can lead to a scenario that sched_clock goes backward when
being read from different CPUs.

This is accommodated by CONFIG_HAVE_UNSTABLE_SCHED_CLOCK, but it's
unavailable on 32bit kernel.

We don't want to risk sched_clock correctness, so if we have multiple
CPU in system and CONFIG_HAVE_UNSTABLE_SCHED_CLOCK is not set, we just
don't use counter as sched_clock source.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/kernel/csrc-r4k.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index 055747a7417d..bdb1fa8931f4 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -68,6 +68,18 @@ static bool rdhwr_count_usable(void)
 	return false;
 }
 
+static inline __init bool count_can_be_sched_clock(void)
+{
+	if (IS_ENABLED(CONFIG_CPU_FREQ))
+		return false;
+
+	if (num_possible_cpus() > 1 &&
+			!IS_ENABLED(CONFIG_HAVE_UNSTABLE_SCHED_CLOCK))
+		return false;
+
+	return true;
+}
+
 #ifdef CONFIG_CPU_FREQ
 
 static bool __read_mostly r4k_clock_unstable;
@@ -125,9 +137,8 @@ int __init init_r4k_clocksource(void)
 
 	clocksource_register_hz(&clocksource_mips, mips_hpt_frequency);
 
-#ifndef CONFIG_CPU_FREQ
-	sched_clock_register(r4k_read_sched_clock, 32, mips_hpt_frequency);
-#endif
+	if (count_can_be_sched_clock())
+		sched_clock_register(r4k_read_sched_clock, 32, mips_hpt_frequency);
 
 	return 0;
 }

-- 
2.43.0


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

* [PATCH v2 5/7] MIPS: sync-r4k: Rework based on x86 tsc_sync
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (3 preceding siblings ...)
  2024-06-12  8:54 ` [PATCH v2 4/7] MIPS: csrc-r4k: Don't register as sched_clock if unfit Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation Jiaxun Yang
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

The original sync-r4k did a good job on reducing jitter by determine
the "next time value", but it has a limitation that when synchronization
being performed too many times due to high core count or CPU hotplug,
the timewrap on CPU0 will become unaccpetable.

Rework the mechanism based on latest x86 tsc_sync. (It seems like
the original implementation is based on tsc_sync at that time,
so it's just a refresh.) To improve overall performance.

Tesed on Loongson64, Boston, QEMU.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/r4k-timer.h |   5 -
 arch/mips/kernel/smp.c            |   2 -
 arch/mips/kernel/sync-r4k.c       | 281 +++++++++++++++++++++++++++-----------
 3 files changed, 202 insertions(+), 86 deletions(-)

diff --git a/arch/mips/include/asm/r4k-timer.h b/arch/mips/include/asm/r4k-timer.h
index 6e7361629348..432e61dd5204 100644
--- a/arch/mips/include/asm/r4k-timer.h
+++ b/arch/mips/include/asm/r4k-timer.h
@@ -12,15 +12,10 @@
 
 #ifdef CONFIG_SYNC_R4K
 
-extern void synchronise_count_master(int cpu);
 extern void synchronise_count_slave(int cpu);
 
 #else
 
-static inline void synchronise_count_master(int cpu)
-{
-}
-
 static inline void synchronise_count_slave(int cpu)
 {
 }
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 0b53d35a116e..0362fc5df7b0 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -462,8 +462,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 		return -EIO;
 	}
 
-	synchronise_count_master(cpu);
-
 	/* Wait for CPU to finish startup & mark itself online before return */
 	wait_for_completion(&cpu_running);
 	return 0;
diff --git a/arch/mips/kernel/sync-r4k.c b/arch/mips/kernel/sync-r4k.c
index abdd7aaa3311..39156592582e 100644
--- a/arch/mips/kernel/sync-r4k.c
+++ b/arch/mips/kernel/sync-r4k.c
@@ -2,121 +2,244 @@
 /*
  * Count register synchronisation.
  *
- * All CPUs will have their count registers synchronised to the CPU0 next time
- * value. This can cause a small timewarp for CPU0. All other CPU's should
- * not have done anything significant (but they may have had interrupts
- * enabled briefly - prom_smp_finish() should not be responsible for enabling
- * interrupts...)
+ * Derived from arch/x86/kernel/tsc_sync.c
+ * Copyright (C) 2006, Red Hat, Inc., Ingo Molnar
  */
 
 #include <linux/kernel.h>
 #include <linux/irqflags.h>
 #include <linux/cpumask.h>
+#include <linux/atomic.h>
+#include <linux/nmi.h>
+#include <linux/smp.h>
+#include <linux/spinlock.h>
 
 #include <asm/r4k-timer.h>
-#include <linux/atomic.h>
-#include <asm/barrier.h>
 #include <asm/mipsregs.h>
+#include <asm/time.h>
 
-static unsigned int initcount = 0;
-static atomic_t count_count_start = ATOMIC_INIT(0);
-static atomic_t count_count_stop = ATOMIC_INIT(0);
-
-#define COUNTON 100
-#define NR_LOOPS 3
-
-void synchronise_count_master(int cpu)
-{
-	int i;
-	unsigned long flags;
-
-	pr_info("Synchronize counters for CPU %u: ", cpu);
+#define COUNTON		100
+#define NR_LOOPS	3
+#define LOOP_TIMEOUT	20
 
-	local_irq_save(flags);
+/*
+ * Entry/exit counters that make sure that both CPUs
+ * run the measurement code at once:
+ */
+static atomic_t start_count;
+static atomic_t stop_count;
+static atomic_t test_runs;
 
-	/*
-	 * We loop a few times to get a primed instruction cache,
-	 * then the last pass is more or less synchronised and
-	 * the master and slaves each set their cycle counters to a known
-	 * value all at once. This reduces the chance of having random offsets
-	 * between the processors, and guarantees that the maximum
-	 * delay between the cycle counters is never bigger than
-	 * the latency of information-passing (cachelines) between
-	 * two CPUs.
-	 */
+/*
+ * We use a raw spinlock in this exceptional case, because
+ * we want to have the fastest, inlined, non-debug version
+ * of a critical section, to be able to prove counter time-warps:
+ */
+static arch_spinlock_t sync_lock = __ARCH_SPIN_LOCK_UNLOCKED;
 
-	for (i = 0; i < NR_LOOPS; i++) {
-		/* slaves loop on '!= 2' */
-		while (atomic_read(&count_count_start) != 1)
-			mb();
-		atomic_set(&count_count_stop, 0);
-		smp_wmb();
+static uint32_t last_counter;
+static uint32_t max_warp;
+static int nr_warps;
+static int random_warps;
 
-		/* Let the slave writes its count register */
-		atomic_inc(&count_count_start);
+/*
+ * Counter warp measurement loop running on both CPUs.
+ */
+static uint32_t check_counter_warp(void)
+{
+	uint32_t start, now, prev, end, cur_max_warp = 0;
+	int i, cur_warps = 0;
 
-		/* Count will be initialised to current timer */
-		if (i == 1)
-			initcount = read_c0_count();
+	start = read_c0_count();
+	end = start + (uint32_t) mips_hpt_frequency / 1000 * LOOP_TIMEOUT;
 
+	for (i = 0; ; i++) {
 		/*
-		 * Everyone initialises count in the last loop:
+		 * We take the global lock, measure counter, save the
+		 * previous counter that was measured (possibly on
+		 * another CPU) and update the previous counter timestamp.
 		 */
-		if (i == NR_LOOPS-1)
-			write_c0_count(initcount);
+		arch_spin_lock(&sync_lock);
+		prev = last_counter;
+		now = read_c0_count();
+		last_counter = now;
+		arch_spin_unlock(&sync_lock);
 
 		/*
-		 * Wait for slave to leave the synchronization point:
+		 * Be nice every now and then (and also check whether
+		 * measurement is done [we also insert a 10 million
+		 * loops safety exit, so we dont lock up in case the
+		 * counter is totally broken]):
 		 */
-		while (atomic_read(&count_count_stop) != 1)
-			mb();
-		atomic_set(&count_count_start, 0);
-		smp_wmb();
-		atomic_inc(&count_count_stop);
+		if (unlikely(!(i & 7))) {
+			if (now > end || i > 10000000)
+				break;
+			cpu_relax();
+			touch_nmi_watchdog();
+		}
+		/*
+		 * Outside the critical section we can now see whether
+		 * we saw a time-warp of the counter going backwards:
+		 */
+		if (unlikely(prev > now)) {
+			arch_spin_lock(&sync_lock);
+			max_warp = max(max_warp, prev - now);
+			cur_max_warp = max_warp;
+			/*
+			 * Check whether this bounces back and forth. Only
+			 * one CPU should observe time going backwards.
+			 */
+			if (cur_warps != nr_warps)
+				random_warps++;
+			nr_warps++;
+			cur_warps = nr_warps;
+			arch_spin_unlock(&sync_lock);
+		}
+	}
+	WARN(!(now-start),
+		"Warning: zero counter calibration delta: %d [max: %d]\n",
+			now-start, end-start);
+	return cur_max_warp;
+}
+
+/*
+ * The freshly booted CPU initiates this via an async SMP function call.
+ */
+static void check_counter_sync_source(void *__cpu)
+{
+	unsigned int cpu = (unsigned long)__cpu;
+	int cpus = 2;
+
+	atomic_set(&test_runs, NR_LOOPS);
+retry:
+	/* Wait for the target to start. */
+	while (atomic_read(&start_count) != cpus - 1)
+		cpu_relax();
+
+	/*
+	 * Trigger the target to continue into the measurement too:
+	 */
+	atomic_inc(&start_count);
+
+	check_counter_warp();
+
+	while (atomic_read(&stop_count) != cpus-1)
+		cpu_relax();
+
+	/*
+	 * If the test was successful set the number of runs to zero and
+	 * stop. If not, decrement the number of runs an check if we can
+	 * retry. In case of random warps no retry is attempted.
+	 */
+	if (!nr_warps) {
+		atomic_set(&test_runs, 0);
+
+		pr_info("Counter synchronization [CPU#%d -> CPU#%u]: passed\n",
+			smp_processor_id(), cpu);
+	} else if (atomic_dec_and_test(&test_runs) || random_warps) {
+		/* Force it to 0 if random warps brought us here */
+		atomic_set(&test_runs, 0);
+
+		pr_info("Counter synchronization [CPU#%d -> CPU#%u]:\n",
+			smp_processor_id(), cpu);
+		pr_info("Measured %d cycles counter warp between CPUs", max_warp);
+		if (random_warps)
+			pr_warn("Counter warped randomly between CPUs\n");
 	}
-	/* Arrange for an interrupt in a short while */
-	write_c0_compare(read_c0_count() + COUNTON);
 
-	local_irq_restore(flags);
+	/*
+	 * Reset it - just in case we boot another CPU later:
+	 */
+	atomic_set(&start_count, 0);
+	random_warps = 0;
+	nr_warps = 0;
+	max_warp = 0;
+	last_counter = 0;
+
+	/*
+	 * Let the target continue with the bootup:
+	 */
+	atomic_inc(&stop_count);
 
 	/*
-	 * i386 code reported the skew here, but the
-	 * count registers were almost certainly out of sync
-	 * so no point in alarming people
+	 * Retry, if there is a chance to do so.
 	 */
-	pr_cont("done.\n");
+	if (atomic_read(&test_runs) > 0)
+		goto retry;
 }
 
+/*
+ * Freshly booted CPUs call into this:
+ */
 void synchronise_count_slave(int cpu)
 {
-	int i;
-	unsigned long flags;
+	uint32_t cur_max_warp, gbl_max_warp, count;
+	int cpus = 2;
 
-	local_irq_save(flags);
+	if (!cpu_has_counter || !mips_hpt_frequency)
+		return;
 
+	/* Kick the control CPU into the counter synchronization function */
+	smp_call_function_single(cpumask_first(cpu_online_mask),
+				 check_counter_sync_source,
+				 (unsigned long *)(unsigned long)cpu, 0);
+retry:
 	/*
-	 * Not every cpu is online at the time this gets called,
-	 * so we first wait for the master to say everyone is ready
+	 * Register this CPU's participation and wait for the
+	 * source CPU to start the measurement:
 	 */
+	atomic_inc(&start_count);
+	while (atomic_read(&start_count) != cpus)
+		cpu_relax();
 
-	for (i = 0; i < NR_LOOPS; i++) {
-		atomic_inc(&count_count_start);
-		while (atomic_read(&count_count_start) != 2)
-			mb();
+	cur_max_warp = check_counter_warp();
 
-		/*
-		 * Everyone initialises count in the last loop:
-		 */
-		if (i == NR_LOOPS-1)
-			write_c0_count(initcount);
+	/*
+	 * Store the maximum observed warp value for a potential retry:
+	 */
+	gbl_max_warp = max_warp;
+
+	/*
+	 * Ok, we are done:
+	 */
+	atomic_inc(&stop_count);
+
+	/*
+	 * Wait for the source CPU to print stuff:
+	 */
+	while (atomic_read(&stop_count) != cpus)
+		cpu_relax();
 
-		atomic_inc(&count_count_stop);
-		while (atomic_read(&count_count_stop) != 2)
-			mb();
+	/*
+	 * Reset it for the next sync test:
+	 */
+	atomic_set(&stop_count, 0);
+
+	/*
+	 * Check the number of remaining test runs. If not zero, the test
+	 * failed and a retry with adjusted counter is possible. If zero the
+	 * test was either successful or failed terminally.
+	 */
+	if (!atomic_read(&test_runs)) {
+		/* Arrange for an interrupt in a short while */
+		write_c0_compare(read_c0_count() + COUNTON);
+		return;
 	}
-	/* Arrange for an interrupt in a short while */
-	write_c0_compare(read_c0_count() + COUNTON);
 
-	local_irq_restore(flags);
+	/*
+	 * If the warp value of this CPU is 0, then the other CPU
+	 * observed time going backwards so this counter was ahead and
+	 * needs to move backwards.
+	 */
+	if (!cur_max_warp)
+		cur_max_warp = -gbl_max_warp;
+
+	count = read_c0_count();
+	count += cur_max_warp;
+	write_c0_count(count);
+
+	pr_debug("Counter compensate: CPU%u observed %d warp\n", cpu, cur_max_warp);
+
+	goto retry;
+
 }
-#undef NR_LOOPS

-- 
2.43.0


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

* [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (4 preceding siblings ...)
  2024-06-12  8:54 ` [PATCH v2 5/7] MIPS: sync-r4k: Rework based on x86 tsc_sync Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-06-21 11:18   ` Jiaxun Yang
  2024-06-12  8:54 ` [PATCH v2 7/7] clocksource: mips-gic-timer: Correct sched_clock width Jiaxun Yang
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang

It is a good clocksource which usually go as fast as CPU core
and have a low access latency, so raise the base of rating
from Good to desired when we know that it has a stable frequency.

Increase frequency addend dividend to 10000000 (10MHz) to
reasonably accommodate multi GHz level clock, also cap rating
within current level.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
v2: Fix number of zeros for 10 MHz
---
 drivers/clocksource/mips-gic-timer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index b3ae38f36720..7a03d94c028a 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -197,7 +197,11 @@ static int __init __gic_clocksource_init(void)
 	gic_clocksource.mask = CLOCKSOURCE_MASK(count_width);
 
 	/* Calculate a somewhat reasonable rating value. */
-	gic_clocksource.rating = 200 + gic_frequency / 10000000;
+	if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ))
+		gic_clocksource.rating = 300; /* Good when frequecy is stable */
+	else
+		gic_clocksource.rating = 200;
+	gic_clocksource.rating += clamp(gic_frequency / 10000000, 0, 99);
 
 	ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
 	if (ret < 0)

-- 
2.43.0


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

* [PATCH v2 7/7] clocksource: mips-gic-timer: Correct sched_clock width
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (5 preceding siblings ...)
  2024-06-12  8:54 ` [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation Jiaxun Yang
@ 2024-06-12  8:54 ` Jiaxun Yang
  2024-07-03  5:59 ` [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
  2024-07-03 15:24 ` Thomas Bogendoerfer
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-12  8:54 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips, linux-kernel, Jiaxun Yang,
	Philippe Mathieu-Daudé

Counter width of GIC is configurable and can be read from a
register.

Use width value from the register for sched_clock.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 drivers/clocksource/mips-gic-timer.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 7a03d94c028a..110347707ff9 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -19,6 +19,7 @@
 static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
 static int gic_timer_irq;
 static unsigned int gic_frequency;
+static unsigned int gic_count_width;
 static bool __read_mostly gic_clock_unstable;
 
 static void gic_clocksource_unstable(char *reason);
@@ -186,15 +187,14 @@ static void gic_clocksource_unstable(char *reason)
 
 static int __init __gic_clocksource_init(void)
 {
-	unsigned int count_width;
 	int ret;
 
 	/* Set clocksource mask. */
-	count_width = read_gic_config() & GIC_CONFIG_COUNTBITS;
-	count_width >>= __ffs(GIC_CONFIG_COUNTBITS);
-	count_width *= 4;
-	count_width += 32;
-	gic_clocksource.mask = CLOCKSOURCE_MASK(count_width);
+	gic_count_width = read_gic_config() & GIC_CONFIG_COUNTBITS;
+	gic_count_width >>= __ffs(GIC_CONFIG_COUNTBITS);
+	gic_count_width *= 4;
+	gic_count_width += 32;
+	gic_clocksource.mask = CLOCKSOURCE_MASK(gic_count_width);
 
 	/* Calculate a somewhat reasonable rating value. */
 	if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ))
@@ -264,7 +264,7 @@ static int __init gic_clocksource_of_init(struct device_node *node)
 	if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
 		sched_clock_register(mips_cm_is64 ?
 				     gic_read_count_64 : gic_read_count_2x32,
-				     64, gic_frequency);
+				     gic_count_width, gic_frequency);
 	}
 
 	return 0;

-- 
2.43.0


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

* Re: [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation
  2024-06-12  8:54 ` [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation Jiaxun Yang
@ 2024-06-21 11:18   ` Jiaxun Yang
  0 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-06-21 11:18 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	Thomas Bogendoerfer, Serge Semin, Thomas Gleixner



在2024年6月12日六月 上午9:54,Jiaxun Yang写道:
> It is a good clocksource which usually go as fast as CPU core
> and have a low access latency, so raise the base of rating
> from Good to desired when we know that it has a stable frequency.
>
> Increase frequency addend dividend to 10000000 (10MHz) to
> reasonably accommodate multi GHz level clock, also cap rating
> within current level.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

Hi Daniel,

Can I get a Review or Ack for this series? As it's mainly clocksource related.

Thanks.
- Jiaxun

> ---
> v2: Fix number of zeros for 10 MHz
> ---
>  drivers/clocksource/mips-gic-timer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/mips-gic-timer.c 
> b/drivers/clocksource/mips-gic-timer.c
> index b3ae38f36720..7a03d94c028a 100644
> --- a/drivers/clocksource/mips-gic-timer.c
> +++ b/drivers/clocksource/mips-gic-timer.c
> @@ -197,7 +197,11 @@ static int __init __gic_clocksource_init(void)
>  	gic_clocksource.mask = CLOCKSOURCE_MASK(count_width);
> 
>  	/* Calculate a somewhat reasonable rating value. */
> -	gic_clocksource.rating = 200 + gic_frequency / 10000000;
> +	if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ))
> +		gic_clocksource.rating = 300; /* Good when frequecy is stable */
> +	else
> +		gic_clocksource.rating = 200;
> +	gic_clocksource.rating += clamp(gic_frequency / 10000000, 0, 99);
> 
>  	ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
>  	if (ret < 0)
>
> -- 
> 2.43.0

-- 
- Jiaxun

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

* Re: [PATCH v2 0/7] MIPS: clocksource cumulative enhancements
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (6 preceding siblings ...)
  2024-06-12  8:54 ` [PATCH v2 7/7] clocksource: mips-gic-timer: Correct sched_clock width Jiaxun Yang
@ 2024-07-03  5:59 ` Jiaxun Yang
  2024-07-03 15:24 ` Thomas Bogendoerfer
  8 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-07-03  5:59 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner
  Cc: Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	Philippe Mathieu-Daudé



在2024年6月12日六月 下午4:54,Jiaxun Yang写道:
> Hi all,
>
> This series combined many enhancements for MIPS clocksource subsystems,
> It improved r4k count synchronisation process, clock source rating for
> selection, sched_clock eligibility and so on.
>
> It seems fixed random RCU stall issue on Loongson 3A4000 multi-node
> system and some boot failures on QEMU.

Ping :-)

>
> Please review.
>
> Thanks 
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> Changes in v2:
> - Fix number of zeros in rating computation (Maciej)
> - Only select HAVE_UNSTABLE_SCHED_CLOCK for SMP (Maciej)
> - Link to v1: 
> https://lore.kernel.org/r/20240511-mips-clks-v1-0-ddb4a10ee9f9@flygoat.com
>
> ---
> Jiaxun Yang (7):
>       MIPS: csrc-r4k: Refine rating computation
>       MIPS: csrc-r4k: Apply verification clocksource flags
>       MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
>       MIPS: csrc-r4k: Don't register as sched_clock if unfit
>       MIPS: sync-r4k: Rework based on x86 tsc_sync
>       clocksource: mips-gic-timer: Refine rating computation
>       clocksource: mips-gic-timer: Correct sched_clock width
>
>  arch/mips/Kconfig                    |   1 +
>  arch/mips/include/asm/r4k-timer.h    |   5 -
>  arch/mips/kernel/csrc-r4k.c          |  24 ++-
>  arch/mips/kernel/smp.c               |   2 -
>  arch/mips/kernel/sync-r4k.c          | 281 +++++++++++++++++++++++++----------
>  drivers/clocksource/mips-gic-timer.c |  20 ++-
>  6 files changed, 234 insertions(+), 99 deletions(-)
> ---
> base-commit: 704ba27ac55579704ba1289392448b0c66b56258
> change-id: 20240509-mips-clks-9001264fcfbe
>
> Best regards,
> -- 
> Jiaxun Yang <jiaxun.yang@flygoat.com>

-- 
- Jiaxun

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

* Re: [PATCH v2 0/7] MIPS: clocksource cumulative enhancements
  2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
                   ` (7 preceding siblings ...)
  2024-07-03  5:59 ` [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
@ 2024-07-03 15:24 ` Thomas Bogendoerfer
  2024-07-08 16:40   ` Daniel Lezcano
  8 siblings, 1 reply; 17+ messages in thread
From: Thomas Bogendoerfer @ 2024-07-03 15:24 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: Serge Semin, Daniel Lezcano, Thomas Gleixner, Maciej W. Rozycki,
	linux-mips, linux-kernel, Philippe Mathieu-Daudé

On Wed, Jun 12, 2024 at 09:54:27AM +0100, Jiaxun Yang wrote:
> Hi all,
> 
> This series combined many enhancements for MIPS clocksource subsystems,
> It improved r4k count synchronisation process, clock source rating for
> selection, sched_clock eligibility and so on.
> 
> It seems fixed random RCU stall issue on Loongson 3A4000 multi-node
> system and some boot failures on QEMU.
> 
> Please review.
> 
> Thanks 
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> Changes in v2:
> - Fix number of zeros in rating computation (Maciej)
> - Only select HAVE_UNSTABLE_SCHED_CLOCK for SMP (Maciej)
> - Link to v1: https://lore.kernel.org/r/20240511-mips-clks-v1-0-ddb4a10ee9f9@flygoat.com
> 
> ---
> Jiaxun Yang (7):
>       MIPS: csrc-r4k: Refine rating computation
>       MIPS: csrc-r4k: Apply verification clocksource flags
>       MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
>       MIPS: csrc-r4k: Don't register as sched_clock if unfit
>       MIPS: sync-r4k: Rework based on x86 tsc_sync

applied these patches to mips-next.

>       clocksource: mips-gic-timer: Refine rating computation
>       clocksource: mips-gic-timer: Correct sched_clock width

looks like the remaining patches don't have any dependency to the other
five patches, so they could just go via clocksource tree. BTW it would
be good to split series in such cases.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH v2 0/7] MIPS: clocksource cumulative enhancements
  2024-07-03 15:24 ` Thomas Bogendoerfer
@ 2024-07-08 16:40   ` Daniel Lezcano
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Lezcano @ 2024-07-08 16:40 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Jiaxun Yang
  Cc: Serge Semin, Thomas Gleixner, Maciej W. Rozycki, linux-mips,
	linux-kernel, Philippe Mathieu-Daudé

On 03/07/2024 17:24, Thomas Bogendoerfer wrote:
> On Wed, Jun 12, 2024 at 09:54:27AM +0100, Jiaxun Yang wrote:
>> Hi all,
>>
>> This series combined many enhancements for MIPS clocksource subsystems,
>> It improved r4k count synchronisation process, clock source rating for
>> selection, sched_clock eligibility and so on.
>>
>> It seems fixed random RCU stall issue on Loongson 3A4000 multi-node
>> system and some boot failures on QEMU.
>>
>> Please review.
>>
>> Thanks
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>> Changes in v2:
>> - Fix number of zeros in rating computation (Maciej)
>> - Only select HAVE_UNSTABLE_SCHED_CLOCK for SMP (Maciej)
>> - Link to v1: https://lore.kernel.org/r/20240511-mips-clks-v1-0-ddb4a10ee9f9@flygoat.com
>>
>> ---
>> Jiaxun Yang (7):
>>        MIPS: csrc-r4k: Refine rating computation
>>        MIPS: csrc-r4k: Apply verification clocksource flags
>>        MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
>>        MIPS: csrc-r4k: Don't register as sched_clock if unfit
>>        MIPS: sync-r4k: Rework based on x86 tsc_sync
> 
> applied these patches to mips-next.
> 
>>        clocksource: mips-gic-timer: Refine rating computation
>>        clocksource: mips-gic-timer: Correct sched_clock width
> 
> looks like the remaining patches don't have any dependency to the other
> five patches, so they could just go via clocksource tree. BTW it would
> be good to split series in such cases.

Applied patches 6 and 7

Thanks

   -- Daniel

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


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

* Re: [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-06-12  8:54 ` [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags Jiaxun Yang
@ 2024-08-06  4:09   ` Guenter Roeck
  2024-08-06  5:06     ` Jiaxun Yang
  0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2024-08-06  4:09 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner,
	Maciej W. Rozycki, linux-mips, linux-kernel

Hi,

On Wed, Jun 12, 2024 at 09:54:29AM +0100, Jiaxun Yang wrote:
> CP0 counter suffers from various problems like SMP sync,
> behaviour on wait.
> 
> Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
> as what x86 did to TSC, to let kernel test it before use.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

With this patch in the mainline kernel, about one in five qemu
boot attempts with e1000 Ethernet controller fail to activate
the network interface (specifically, the dhcp client is unable to
get an IP address for the interface). Bisect log is attached below.

For reference, here is an example command line.

qemu-system-mips64 -kernel vmlinux -M malta -cpu 5KEc \
	-initrd rootfs-n32.cpio \
	-device e1000,netdev=net0 -netdev user,id=net0 \
	-vga cirrus -no-reboot -m 256 \
	--append "rdinit=/sbin/init mem=256M console=ttyS0 console=tty " \
	-nographic

Reverting this patch fixes the probem.

Thanks,
Guenter

---
# bad: [de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed] Linux 6.11-rc2
# good: [0c3836482481200ead7b416ca80c68a29cfdaabd] Linux 6.10
git bisect start 'HEAD' 'v6.10'
# good: [280e36f0d5b997173d014c07484c03a7f7750668] nsfs: use cleanup guard
git bisect good 280e36f0d5b997173d014c07484c03a7f7750668
# good: [a4f9285520584977127946a22eab2adfbc87d1bf] Merge tag 'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
git bisect good a4f9285520584977127946a22eab2adfbc87d1bf
# bad: [8e313211f7d46d42b6aa7601b972fe89dcc4a076] Merge tag 'pinctrl-v6.11-1' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
git bisect bad 8e313211f7d46d42b6aa7601b972fe89dcc4a076
# good: [acc5965b9ff8a1889f5b51466562896d59c6e1b9] Merge tag 'char-misc-6.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
git bisect good acc5965b9ff8a1889f5b51466562896d59c6e1b9
# bad: [d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d] Merge tag 'mips_6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
git bisect bad d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d
# good: [45659274e60864f9acabba844468e405362bdc8c] Merge branch 'pci/misc'
git bisect good 45659274e60864f9acabba844468e405362bdc8c
# good: [8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74] Merge tag 'input-for-v6.11-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
git bisect good 8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74
# good: [3c3ff7be9729959699eb6cbc7fd7303566d74069] Merge tag 'powerpc-6.11-1' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect good 3c3ff7be9729959699eb6cbc7fd7303566d74069
# good: [3de96d810ffd712b7ad2bd764c1390fac2436551] dt-bindings: mips: brcm: Document brcm,bmips-cbr-reg property
git bisect good 3de96d810ffd712b7ad2bd764c1390fac2436551
# bad: [9c7a86c935074525f24cc20e78a7d5150e4600e3] MIPS: lantiq: improve USB initialization
git bisect bad 9c7a86c935074525f24cc20e78a7d5150e4600e3
# bad: [580724fce27f2b71b3e4d58bbe6d83b671929b33] MIPS: sync-r4k: Rework based on x86 tsc_sync
git bisect bad 580724fce27f2b71b3e4d58bbe6d83b671929b33
# good: [c171186c177970d3ec22dd814f2693f1f7fc1e7d] MIPS: csrc-r4k: Refine rating computation
git bisect good c171186c177970d3ec22dd814f2693f1f7fc1e7d
# bad: [426fa8e4fe7bb914b5977cbce453a9926bf5b2e6] MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
git bisect bad 426fa8e4fe7bb914b5977cbce453a9926bf5b2e6
# bad: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS: csrc-r4k: Apply verification clocksource flags
git bisect bad 7190401fc56fb5f02ee3d04476778ab000bbaf32
# first bad commit: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS: csrc-r4k: Apply verification clocksource flags

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

* Re: [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-08-06  4:09   ` Guenter Roeck
@ 2024-08-06  5:06     ` Jiaxun Yang
  2024-08-06  5:13       ` Guenter Roeck
  0 siblings, 1 reply; 17+ messages in thread
From: Jiaxun Yang @ 2024-08-06  5:06 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner,
	Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	regressions



在2024年8月6日八月 下午12:09,Guenter Roeck写道:
> Hi,
>
> On Wed, Jun 12, 2024 at 09:54:29AM +0100, Jiaxun Yang wrote:
>> CP0 counter suffers from various problems like SMP sync,
>> behaviour on wait.
>> 
>> Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
>> as what x86 did to TSC, to let kernel test it before use.
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

Hi Guenter,

Thanks for the report, it makes no sense to me though....

I can't reproduce it with QEMU git master, do you mind specifying your QEMU
version for me? Also is it possible to have a copy of dmesg when failure happens.

If I'm unable to resolve it in a couple of days, I'll send a patch to revert this change.

Thanks

>
> With this patch in the mainline kernel, about one in five qemu
> boot attempts with e1000 Ethernet controller fail to activate
> the network interface (specifically, the dhcp client is unable to
> get an IP address for the interface). Bisect log is attached below.
>
> For reference, here is an example command line.
>
> qemu-system-mips64 -kernel vmlinux -M malta -cpu 5KEc \
> 	-initrd rootfs-n32.cpio \
> 	-device e1000,netdev=net0 -netdev user,id=net0 \
> 	-vga cirrus -no-reboot -m 256 \
> 	--append "rdinit=/sbin/init mem=256M console=ttyS0 console=tty " \
> 	-nographic
>
> Reverting this patch fixes the probem.
>
> Thanks,
> Guenter
>
> ---
> # bad: [de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed] Linux 6.11-rc2
> # good: [0c3836482481200ead7b416ca80c68a29cfdaabd] Linux 6.10
> git bisect start 'HEAD' 'v6.10'
> # good: [280e36f0d5b997173d014c07484c03a7f7750668] nsfs: use cleanup 
> guard
> git bisect good 280e36f0d5b997173d014c07484c03a7f7750668
> # good: [a4f9285520584977127946a22eab2adfbc87d1bf] Merge tag 
> 'clk-for-linus' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
> git bisect good a4f9285520584977127946a22eab2adfbc87d1bf
> # bad: [8e313211f7d46d42b6aa7601b972fe89dcc4a076] Merge tag 
> 'pinctrl-v6.11-1' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
> git bisect bad 8e313211f7d46d42b6aa7601b972fe89dcc4a076
> # good: [acc5965b9ff8a1889f5b51466562896d59c6e1b9] Merge tag 
> 'char-misc-6.11-rc1' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
> git bisect good acc5965b9ff8a1889f5b51466562896d59c6e1b9
> # bad: [d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d] Merge tag 'mips_6.11' 
> of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
> git bisect bad d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d
> # good: [45659274e60864f9acabba844468e405362bdc8c] Merge branch 
> 'pci/misc'
> git bisect good 45659274e60864f9acabba844468e405362bdc8c
> # good: [8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74] Merge tag 
> 'input-for-v6.11-rc0' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
> git bisect good 8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74
> # good: [3c3ff7be9729959699eb6cbc7fd7303566d74069] Merge tag 
> 'powerpc-6.11-1' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect good 3c3ff7be9729959699eb6cbc7fd7303566d74069
> # good: [3de96d810ffd712b7ad2bd764c1390fac2436551] dt-bindings: mips: 
> brcm: Document brcm,bmips-cbr-reg property
> git bisect good 3de96d810ffd712b7ad2bd764c1390fac2436551
> # bad: [9c7a86c935074525f24cc20e78a7d5150e4600e3] MIPS: lantiq: improve 
> USB initialization
> git bisect bad 9c7a86c935074525f24cc20e78a7d5150e4600e3
> # bad: [580724fce27f2b71b3e4d58bbe6d83b671929b33] MIPS: sync-r4k: 
> Rework based on x86 tsc_sync
> git bisect bad 580724fce27f2b71b3e4d58bbe6d83b671929b33
> # good: [c171186c177970d3ec22dd814f2693f1f7fc1e7d] MIPS: csrc-r4k: 
> Refine rating computation
> git bisect good c171186c177970d3ec22dd814f2693f1f7fc1e7d
> # bad: [426fa8e4fe7bb914b5977cbce453a9926bf5b2e6] MIPS: csrc-r4k: 
> Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
> git bisect bad 426fa8e4fe7bb914b5977cbce453a9926bf5b2e6
> # bad: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS: csrc-r4k: Apply 
> verification clocksource flags
> git bisect bad 7190401fc56fb5f02ee3d04476778ab000bbaf32
> # first bad commit: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS: 
> csrc-r4k: Apply verification clocksource flags

-- 
- Jiaxun

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

* Re: [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-08-06  5:06     ` Jiaxun Yang
@ 2024-08-06  5:13       ` Guenter Roeck
  2024-08-06 15:06         ` Guenter Roeck
  0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2024-08-06  5:13 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner,
	Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	regressions

On 8/5/24 22:06, Jiaxun Yang wrote:
> 
> 
> 在2024年8月6日八月 下午12:09,Guenter Roeck写道:
>> Hi,
>>
>> On Wed, Jun 12, 2024 at 09:54:29AM +0100, Jiaxun Yang wrote:
>>> CP0 counter suffers from various problems like SMP sync,
>>> behaviour on wait.
>>>
>>> Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
>>> as what x86 did to TSC, to let kernel test it before use.
>>>
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> 
> Hi Guenter,
> 
> Thanks for the report, it makes no sense to me though....
> 
> I can't reproduce it with QEMU git master, do you mind specifying your QEMU
> version for me? Also is it possible to have a copy of dmesg when failure happens.
> 

I currently use v9.0.2. I'll try with some other versions tomorrow.
A complete log is at
https://kerneltests.org/builders/qemu-mips64-master/builds/241/steps/qemubuildcommand/logs/stdio

Are you trying to instantiate an e1000 (or a variant of it) ? So far
I have only seen the problem with that controller. There is no specific
error message, the network interface just doesn't get an IP address.

Thanks,
Guenter

> If I'm unable to resolve it in a couple of days, I'll send a patch to revert this change.
> 
> Thanks
> 
>>
>> With this patch in the mainline kernel, about one in five qemu
>> boot attempts with e1000 Ethernet controller fail to activate
>> the network interface (specifically, the dhcp client is unable to
>> get an IP address for the interface). Bisect log is attached below.
>>
>> For reference, here is an example command line.
>>
>> qemu-system-mips64 -kernel vmlinux -M malta -cpu 5KEc \
>> 	-initrd rootfs-n32.cpio \
>> 	-device e1000,netdev=net0 -netdev user,id=net0 \
>> 	-vga cirrus -no-reboot -m 256 \
>> 	--append "rdinit=/sbin/init mem=256M console=ttyS0 console=tty " \
>> 	-nographic
>>
>> Reverting this patch fixes the probem.
>>
>> Thanks,
>> Guenter
>>
>> ---
>> # bad: [de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed] Linux 6.11-rc2
>> # good: [0c3836482481200ead7b416ca80c68a29cfdaabd] Linux 6.10
>> git bisect start 'HEAD' 'v6.10'
>> # good: [280e36f0d5b997173d014c07484c03a7f7750668] nsfs: use cleanup
>> guard
>> git bisect good 280e36f0d5b997173d014c07484c03a7f7750668
>> # good: [a4f9285520584977127946a22eab2adfbc87d1bf] Merge tag
>> 'clk-for-linus' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
>> git bisect good a4f9285520584977127946a22eab2adfbc87d1bf
>> # bad: [8e313211f7d46d42b6aa7601b972fe89dcc4a076] Merge tag
>> 'pinctrl-v6.11-1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
>> git bisect bad 8e313211f7d46d42b6aa7601b972fe89dcc4a076
>> # good: [acc5965b9ff8a1889f5b51466562896d59c6e1b9] Merge tag
>> 'char-misc-6.11-rc1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
>> git bisect good acc5965b9ff8a1889f5b51466562896d59c6e1b9
>> # bad: [d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d] Merge tag 'mips_6.11'
>> of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
>> git bisect bad d2be38b9a5514dbc7dc0c96a2a7f619fcddce00d
>> # good: [45659274e60864f9acabba844468e405362bdc8c] Merge branch
>> 'pci/misc'
>> git bisect good 45659274e60864f9acabba844468e405362bdc8c
>> # good: [8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74] Merge tag
>> 'input-for-v6.11-rc0' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
>> git bisect good 8e5c0abfa02d85b9cd2419567ad2d73ed8fe4b74
>> # good: [3c3ff7be9729959699eb6cbc7fd7303566d74069] Merge tag
>> 'powerpc-6.11-1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
>> git bisect good 3c3ff7be9729959699eb6cbc7fd7303566d74069
>> # good: [3de96d810ffd712b7ad2bd764c1390fac2436551] dt-bindings: mips:
>> brcm: Document brcm,bmips-cbr-reg property
>> git bisect good 3de96d810ffd712b7ad2bd764c1390fac2436551
>> # bad: [9c7a86c935074525f24cc20e78a7d5150e4600e3] MIPS: lantiq: improve
>> USB initialization
>> git bisect bad 9c7a86c935074525f24cc20e78a7d5150e4600e3
>> # bad: [580724fce27f2b71b3e4d58bbe6d83b671929b33] MIPS: sync-r4k:
>> Rework based on x86 tsc_sync
>> git bisect bad 580724fce27f2b71b3e4d58bbe6d83b671929b33
>> # good: [c171186c177970d3ec22dd814f2693f1f7fc1e7d] MIPS: csrc-r4k:
>> Refine rating computation
>> git bisect good c171186c177970d3ec22dd814f2693f1f7fc1e7d
>> # bad: [426fa8e4fe7bb914b5977cbce453a9926bf5b2e6] MIPS: csrc-r4k:
>> Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT
>> git bisect bad 426fa8e4fe7bb914b5977cbce453a9926bf5b2e6
>> # bad: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS: csrc-r4k: Apply
>> verification clocksource flags
>> git bisect bad 7190401fc56fb5f02ee3d04476778ab000bbaf32
>> # first bad commit: [7190401fc56fb5f02ee3d04476778ab000bbaf32] MIPS:
>> csrc-r4k: Apply verification clocksource flags
> 


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

* Re: [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-08-06  5:13       ` Guenter Roeck
@ 2024-08-06 15:06         ` Guenter Roeck
  2024-08-08  7:35           ` Jiaxun Yang
  0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2024-08-06 15:06 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner,
	Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	regressions

On 8/5/24 22:13, Guenter Roeck wrote:
> On 8/5/24 22:06, Jiaxun Yang wrote:
>>
>>
>> 在2024年8月6日八月 下午12:09,Guenter Roeck写道:
>>> Hi,
>>>
>>> On Wed, Jun 12, 2024 at 09:54:29AM +0100, Jiaxun Yang wrote:
>>>> CP0 counter suffers from various problems like SMP sync,
>>>> behaviour on wait.
>>>>
>>>> Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
>>>> as what x86 did to TSC, to let kernel test it before use.
>>>>
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>
>> Hi Guenter,
>>
>> Thanks for the report, it makes no sense to me though....
>>
>> I can't reproduce it with QEMU git master, do you mind specifying your QEMU
>> version for me? Also is it possible to have a copy of dmesg when failure happens.
>>
> 
> I currently use v9.0.2. I'll try with some other versions tomorrow.
> A complete log is at
> https://kerneltests.org/builders/qemu-mips64-master/builds/241/steps/qemubuildcommand/logs/stdio
> 
> Are you trying to instantiate an e1000 (or a variant of it) ? So far
> I have only seen the problem with that controller. There is no specific
> error message, the network interface just doesn't get an IP address.
> 

I am able to reproduce the problem with qemu 6.2.0 (Debian build).
http://server.roeck-us.net/qemu/mips64/ should have everything needed to
reproduce it. "repeat.sh" repeats the test until it fails.

Hope this helps,
Guenter


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

* Re: [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags
  2024-08-06 15:06         ` Guenter Roeck
@ 2024-08-08  7:35           ` Jiaxun Yang
  0 siblings, 0 replies; 17+ messages in thread
From: Jiaxun Yang @ 2024-08-08  7:35 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Thomas Bogendoerfer, Serge Semin, Daniel Lezcano, Thomas Gleixner,
	Maciej W. Rozycki, linux-mips@vger.kernel.org, linux-kernel,
	regressions



在2024年8月6日八月 下午4:06,Guenter Roeck写道:
> On 8/5/24 22:13, Guenter Roeck wrote:
>> On 8/5/24 22:06, Jiaxun Yang wrote:
>>>
>>>
>>> 在2024年8月6日八月 下午12:09,Guenter Roeck写道:
>>>> Hi,
>>>>
>>>> On Wed, Jun 12, 2024 at 09:54:29AM +0100, Jiaxun Yang wrote:
>>>>> CP0 counter suffers from various problems like SMP sync,
>>>>> behaviour on wait.
>>>>>
>>>>> Set CLOCK_SOURCE_MUST_VERIFY and CLOCK_SOURCE_VERIFY_PERCPU,
>>>>> as what x86 did to TSC, to let kernel test it before use.
>>>>>
>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>
>>> Hi Guenter,
>>>
>>> Thanks for the report, it makes no sense to me though....
>>>
>>> I can't reproduce it with QEMU git master, do you mind specifying your QEMU
>>> version for me? Also is it possible to have a copy of dmesg when failure happens.
>>>
>> 
>> I currently use v9.0.2. I'll try with some other versions tomorrow.
>> A complete log is at
>> https://kerneltests.org/builders/qemu-mips64-master/builds/241/steps/qemubuildcommand/logs/stdio
>> 
>> Are you trying to instantiate an e1000 (or a variant of it) ? So far
>> I have only seen the problem with that controller. There is no specific
>> error message, the network interface just doesn't get an IP address.
>> 
>
> I am able to reproduce the problem with qemu 6.2.0 (Debian build).
> http://server.roeck-us.net/qemu/mips64/ should have everything needed to
> reproduce it. "repeat.sh" repeats the test until it fails.

Thanks for the info, I'm able to reproduce that. It can be reproduced faster
on system with lower CPU performance.

So the actual failure is:

clocksource: timekeeping watchdog on CPU0: Marking clocksource 'MIPS' as unstable because the skew is too large:
clocksource:                       'jiffies' wd_nsec: 500000000 wd_now: ffff8bde wd_last: ffff8bac mask: ffffffff
clocksource:                       'MIPS' cs_nsec: 940634468 cs_now: 310181c4 cs_last: 28090a09 mask: ffffffff
clocksource:                       Clocksource 'MIPS' skewed 440634468 ns (440 ms) over watchdog 'jiffies' interval of 500000000 ns (500 ms)
clocksource:                       'MIPS' is current clocksource.

Jiffies is not an ideal clocksource as watchdog base, really....
I guess clocksource selection process needs to be improved, let me think about it.

>
> Hope this helps,
> Guenter

-- 
- Jiaxun

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

end of thread, other threads:[~2024-08-08  7:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-12  8:54 [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 1/7] MIPS: csrc-r4k: Refine rating computation Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 2/7] MIPS: csrc-r4k: Apply verification clocksource flags Jiaxun Yang
2024-08-06  4:09   ` Guenter Roeck
2024-08-06  5:06     ` Jiaxun Yang
2024-08-06  5:13       ` Guenter Roeck
2024-08-06 15:06         ` Guenter Roeck
2024-08-08  7:35           ` Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 3/7] MIPS: csrc-r4k: Select HAVE_UNSTABLE_SCHED_CLOCK if SMP && 64BIT Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 4/7] MIPS: csrc-r4k: Don't register as sched_clock if unfit Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 5/7] MIPS: sync-r4k: Rework based on x86 tsc_sync Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 6/7] clocksource: mips-gic-timer: Refine rating computation Jiaxun Yang
2024-06-21 11:18   ` Jiaxun Yang
2024-06-12  8:54 ` [PATCH v2 7/7] clocksource: mips-gic-timer: Correct sched_clock width Jiaxun Yang
2024-07-03  5:59 ` [PATCH v2 0/7] MIPS: clocksource cumulative enhancements Jiaxun Yang
2024-07-03 15:24 ` Thomas Bogendoerfer
2024-07-08 16:40   ` Daniel Lezcano

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).