forked from Minki/linux
b9023b91dd
When a cpu requests broadcasting, before starting the tick broadcast
hrtimer, bc_set_next() checks if the timer callback (bc_handler) is active
using hrtimer_try_to_cancel(). But hrtimer_try_to_cancel() does not provide
the required synchronization when the callback is active on other core.
The callback could have already executed tick_handle_oneshot_broadcast()
and could have also returned. But still there is a small time window where
the hrtimer_try_to_cancel() returns -1. In that case bc_set_next() returns
without doing anything, but the next_event of the tick broadcast clock
device is already set to a timeout value.
In the race condition diagram below, CPU #1 is running the timer callback
and CPU #2 is entering idle state and so calls bc_set_next().
In the worst case, the next_event will contain an expiry time, but the
hrtimer will not be started which happens when the racing callback returns
HRTIMER_NORESTART. The hrtimer might never recover if all further requests
from the CPUs to subscribe to tick broadcast have timeout greater than the
next_event of tick broadcast clock device. This leads to cascading of
failures and finally noticed as rcu stall warnings
Here is a depiction of the race condition
CPU #1 (Running timer callback) CPU #2 (Enter idle
and subscribe to
tick broadcast)
--------------------- ---------------------
__run_hrtimer() tick_broadcast_enter()
bc_handler() __tick_broadcast_oneshot_control()
tick_handle_oneshot_broadcast()
raw_spin_lock(&tick_broadcast_lock);
dev->next_event = KTIME_MAX; //wait for tick_broadcast_lock
//next_event for tick broadcast clock
set to KTIME_MAX since no other cores
subscribed to tick broadcasting
raw_spin_unlock(&tick_broadcast_lock);
if (dev->next_event == KTIME_MAX)
return HRTIMER_NORESTART
// callback function exits without
restarting the hrtimer //tick_broadcast_lock acquired
raw_spin_lock(&tick_broadcast_lock);
tick_broadcast_set_event()
clockevents_program_event()
dev->next_event = expires;
bc_set_next()
hrtimer_try_to_cancel()
//returns -1 since the timer
callback is active. Exits without
restarting the timer
cpu_base->running = NULL;
The comment that hrtimer cannot be armed from within the callback is
wrong. It is fine to start the hrtimer from within the callback. Also it is
safe to start the hrtimer from the enter/exit idle code while the broadcast
handler is active. The enter/exit idle code and the broadcast handler are
synchronized using tick_broadcast_lock. So there is no need for the
existing try to cancel logic. All this can be removed which will eliminate
the race condition as well.
Fixes: 5d1638acb9 ("tick: Introduce hrtimer based broadcast")
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20190926135101.12102-2-balasubramani_vivekanandan@mentor.com
112 lines
3.2 KiB
C
112 lines
3.2 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
/*
|
|
* Emulate a local clock event device via a pseudo clock device.
|
|
*/
|
|
#include <linux/cpu.h>
|
|
#include <linux/err.h>
|
|
#include <linux/hrtimer.h>
|
|
#include <linux/interrupt.h>
|
|
#include <linux/percpu.h>
|
|
#include <linux/profile.h>
|
|
#include <linux/clockchips.h>
|
|
#include <linux/sched.h>
|
|
#include <linux/smp.h>
|
|
#include <linux/module.h>
|
|
|
|
#include "tick-internal.h"
|
|
|
|
static struct hrtimer bctimer;
|
|
|
|
static int bc_shutdown(struct clock_event_device *evt)
|
|
{
|
|
/*
|
|
* Note, we cannot cancel the timer here as we might
|
|
* run into the following live lock scenario:
|
|
*
|
|
* cpu 0 cpu1
|
|
* lock(broadcast_lock);
|
|
* hrtimer_interrupt()
|
|
* bc_handler()
|
|
* tick_handle_oneshot_broadcast();
|
|
* lock(broadcast_lock);
|
|
* hrtimer_cancel()
|
|
* wait_for_callback()
|
|
*/
|
|
hrtimer_try_to_cancel(&bctimer);
|
|
return 0;
|
|
}
|
|
|
|
/*
|
|
* This is called from the guts of the broadcast code when the cpu
|
|
* which is about to enter idle has the earliest broadcast timer event.
|
|
*/
|
|
static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
|
|
{
|
|
/*
|
|
* This is called either from enter/exit idle code or from the
|
|
* broadcast handler. In all cases tick_broadcast_lock is held.
|
|
*
|
|
* hrtimer_cancel() cannot be called here neither from the
|
|
* broadcast handler nor from the enter/exit idle code. The idle
|
|
* code can run into the problem described in bc_shutdown() and the
|
|
* broadcast handler cannot wait for itself to complete for obvious
|
|
* reasons.
|
|
*
|
|
* Each caller tries to arm the hrtimer on its own CPU, but if the
|
|
* hrtimer callbback function is currently running, then
|
|
* hrtimer_start() cannot move it and the timer stays on the CPU on
|
|
* which it is assigned at the moment.
|
|
*
|
|
* As this can be called from idle code, the hrtimer_start()
|
|
* invocation has to be wrapped with RCU_NONIDLE() as
|
|
* hrtimer_start() can call into tracing.
|
|
*/
|
|
RCU_NONIDLE( {
|
|
hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED_HARD);
|
|
/*
|
|
* The core tick broadcast mode expects bc->bound_on to be set
|
|
* correctly to prevent a CPU which has the broadcast hrtimer
|
|
* armed from going deep idle.
|
|
*
|
|
* As tick_broadcast_lock is held, nothing can change the cpu
|
|
* base which was just established in hrtimer_start() above. So
|
|
* the below access is safe even without holding the hrtimer
|
|
* base lock.
|
|
*/
|
|
bc->bound_on = bctimer.base->cpu_base->cpu;
|
|
} );
|
|
return 0;
|
|
}
|
|
|
|
static struct clock_event_device ce_broadcast_hrtimer = {
|
|
.name = "bc_hrtimer",
|
|
.set_state_shutdown = bc_shutdown,
|
|
.set_next_ktime = bc_set_next,
|
|
.features = CLOCK_EVT_FEAT_ONESHOT |
|
|
CLOCK_EVT_FEAT_KTIME |
|
|
CLOCK_EVT_FEAT_HRTIMER,
|
|
.rating = 0,
|
|
.bound_on = -1,
|
|
.min_delta_ns = 1,
|
|
.max_delta_ns = KTIME_MAX,
|
|
.min_delta_ticks = 1,
|
|
.max_delta_ticks = ULONG_MAX,
|
|
.mult = 1,
|
|
.shift = 0,
|
|
.cpumask = cpu_possible_mask,
|
|
};
|
|
|
|
static enum hrtimer_restart bc_handler(struct hrtimer *t)
|
|
{
|
|
ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
|
|
|
|
return HRTIMER_NORESTART;
|
|
}
|
|
|
|
void tick_setup_hrtimer_broadcast(void)
|
|
{
|
|
hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
|
|
bctimer.function = bc_handler;
|
|
clockevents_register_device(&ce_broadcast_hrtimer);
|
|
}
|