linux-kernel - [PATCH 1/3] clocksource/mips-gic-timer: Fix rcu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2017 15:01:12 +0100
From:   Matt Redfearn <matt.redfearn@...s.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
CC:     <linux-mips@...ux-mips.org>,
        Matt Redfearn <matt.redfearn@...s.com>,
        "Matt Redfearn" <matt.redfearn@...tec.com>,
        "# v3 . 19 +" <stable@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: [PATCH 1/3] clocksource/mips-gic-timer: Fix rcu_sched timeouts from multithreading

When the MIPS GIC clockevent code was written, it appears to have
inherited the 0x300 cycle min delta from the MIPS CPU timer driver. This
is suboptimal for two reasons.

Firstly, the CPU timer counts once every other cycle (i.e. half the
clock rate). The GIC counts once per clock. Assuming that the GIC and
CPU share the same clock this means the GIC is counting twice as fast,
and so the min delta should be (at least) doubled. Fix this by doubling
the min delta to 0x600.

Secondly, the fixed min delta ignores the fact that with MIPS
multithreading active, execution resource within a core is shared
between the hardware threads within that core. An inconvenienly timed
switch of executing thread within gic_next_event, between the read and
write of updated count, can result in the CPU writing an event in the
past, and subsequently not receiving a tick interrupt until the counter
wraps. This stalls the CPU from the RCU scheduler. Other CPUs detect
this and print rcu_sched timeout messages in  the kernel log. It can
lead to other issues as well if the CPU is holding locks or other
resources at the point at which it stalls. Fix this by scaling the min
delta for the timer based on the number of threads in the core
(smp_num_siblings). This accounts for the greater average runtime of
CPUs within a multithreading core.

Signed-off-by: Matt Redfearn <matt.redfearn@...tec.com>
Fixes: b695d8e6ad6f ("clocksource: mips-gic: Use clockevents_config_and_register")
Cc: <stable@...r.kernel.org> # v3.19 +

Signed-off-by: Matt Redfearn <matt.redfearn@...s.com>
---

 drivers/clocksource/mips-gic-timer.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index ae3167c28b12..6c94a74682a2 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -72,6 +72,9 @@ struct irqaction gic_compare_irqaction = {
 static void gic_clockevent_cpu_init(unsigned int cpu,
 				    struct clock_event_device *cd)
 {
+	unsigned long max_d = 0x7fffffff;
+	unsigned long min_d = 0x600;
+
 	cd->name		= "MIPS GIC";
 	cd->features		= CLOCK_EVT_FEAT_ONESHOT |
 				  CLOCK_EVT_FEAT_C3STOP;
@@ -81,7 +84,15 @@ static void gic_clockevent_cpu_init(unsigned int cpu,
 	cd->cpumask		= cpumask_of(cpu);
 	cd->set_next_event	= gic_next_event;

-	clockevents_config_and_register(cd, gic_frequency, 0x300, 0x7fffffff);
+	/*
+	 * The min_delta is sensitive to the number of hardware threads in
+	 * the core. With more threads each thread will, on average, get
+	 * less instructions executed per clock. To account for this, we
+	 * scale the min delta based on the number of threads per core.
+	 */
+	min_d *= smp_num_siblings;
+
+	clockevents_config_and_register(cd, gic_frequency, min_d, max_d);

 	enable_percpu_irq(gic_timer_irq, IRQ_TYPE_NONE);
 }
-- 
2.7.4