Skip to content

Commit bc4394e

Browse files
Kan LiangPeter Zijlstra
authored andcommitted
perf: Fix the throttle error of some clock events
Both ARM and IBM CI reports RCU stall, which can be reproduced by the below perf command. perf record -a -e cpu-clock -- sleep 2 The issue is introduced by the generic throttle patch set, which unconditionally invoke the event_stop() when throttle is triggered. The cpu-clock and task-clock are two special SW events, which rely on the hrtimer. The throttle is invoked in the hrtimer handler. The event_stop()->hrtimer_cancel() waits for the handler to finish, which is a deadlock. Instead of invoking the stop(), the HRTIMER_NORESTART should be used to stop the timer. There may be two ways to fix it: - Introduce a PMU flag to track the case. Avoid the event_stop in perf_event_throttle() if the flag is detected. It has been implemented in the https://lore.kernel.org/lkml/[email protected]/ The new flag was thought to be an overkill for the issue. - Add a check in the event_stop. Return immediately if the throttle is invoked in the hrtimer handler. Rely on the existing HRTIMER_NORESTART method to stop the timer. The latter is implemented here. Move event->hw.interrupts = MAX_INTERRUPTS before the stop(). It makes the order the same as perf_event_unthrottle(). Except the patch, no one checks the hw.interrupts in the stop(). There is no impact from the order change. When stops in the throttle, the event should not be updated, stop(event, 0). But the cpu_clock_event_stop() doesn't handle the flag. In logic, it's wrong. But it didn't bring any problems with the old code, because the stop() was not invoked when handling the throttle. Checking the flag before updating the event. Fixes: 9734e25 ("perf: Fix the throttle logic for a group") Closes: https://lore.kernel.org/lkml/[email protected]/ Closes: https://lore.kernel.org/lkml/djxlh5fx326gcenwrr52ry3pk4wxmugu4jccdjysza7tlc5fef@ktp4rffawgcw/ Closes: https://lore.kernel.org/lkml/[email protected]/ Closes: https://lore.kernel.org/lkml/[email protected]/ Reported-by: Leo Yan <[email protected]> Reported-by: Aishwarya TCV <[email protected]> Reported-by: Alexei Starovoitov <[email protected]> Reported-by: Venkat Rao Bagalkote <[email protected]> Reported-by: Vince Weaver <[email protected]> Signed-off-by: Kan Liang <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Ian Rogers <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 49b393a commit bc4394e

File tree

1 file changed

+11
-4
lines changed

1 file changed

+11
-4
lines changed

kernel/events/core.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2674,8 +2674,8 @@ static void perf_event_unthrottle(struct perf_event *event, bool start)
26742674

26752675
static void perf_event_throttle(struct perf_event *event)
26762676
{
2677-
event->pmu->stop(event, 0);
26782677
event->hw.interrupts = MAX_INTERRUPTS;
2678+
event->pmu->stop(event, 0);
26792679
if (event == event->group_leader)
26802680
perf_log_throttle(event, 0);
26812681
}
@@ -11774,7 +11774,12 @@ static void perf_swevent_cancel_hrtimer(struct perf_event *event)
1177411774
{
1177511775
struct hw_perf_event *hwc = &event->hw;
1177611776

11777-
if (is_sampling_event(event)) {
11777+
/*
11778+
* The throttle can be triggered in the hrtimer handler.
11779+
* The HRTIMER_NORESTART should be used to stop the timer,
11780+
* rather than hrtimer_cancel(). See perf_swevent_hrtimer()
11781+
*/
11782+
if (is_sampling_event(event) && (hwc->interrupts != MAX_INTERRUPTS)) {
1177811783
ktime_t remaining = hrtimer_get_remaining(&hwc->hrtimer);
1177911784
local64_set(&hwc->period_left, ktime_to_ns(remaining));
1178011785

@@ -11829,7 +11834,8 @@ static void cpu_clock_event_start(struct perf_event *event, int flags)
1182911834
static void cpu_clock_event_stop(struct perf_event *event, int flags)
1183011835
{
1183111836
perf_swevent_cancel_hrtimer(event);
11832-
cpu_clock_event_update(event);
11837+
if (flags & PERF_EF_UPDATE)
11838+
cpu_clock_event_update(event);
1183311839
}
1183411840

1183511841
static int cpu_clock_event_add(struct perf_event *event, int flags)
@@ -11907,7 +11913,8 @@ static void task_clock_event_start(struct perf_event *event, int flags)
1190711913
static void task_clock_event_stop(struct perf_event *event, int flags)
1190811914
{
1190911915
perf_swevent_cancel_hrtimer(event);
11910-
task_clock_event_update(event, event->ctx->time);
11916+
if (flags & PERF_EF_UPDATE)
11917+
task_clock_event_update(event, event->ctx->time);
1191111918
}
1191211919

1191311920
static int task_clock_event_add(struct perf_event *event, int flags)

0 commit comments

Comments
 (0)