[PATCH 2/6] drm/i915/pmu: use the expiry injecting hrtimer callback
Andreas Hindborg <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.freedesktop.lists.dri-devel,org.freedesktop.lists.intel-gfx,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
The sampling timer forwards itself from its callback:
i915_sample()
hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
hrtimer callbacks run with the timer base lock dropped, so this
read-modify-write of the expiry races with a concurrent restart of
the timer. Such a restart is possible here: i915_pmu_gt_parked()
clears pmu->timer_enabled, and a subsequent
__i915_pmu_maybe_start_timer() from i915_pmu_gt_unparked() or event
enable on another CPU sees the timer disabled and calls
hrtimer_start_range_ns() - also while the callback is running, since
i915_sample() checks timer_enabled only once at entry and takes no
lock. hrtimer_forward() then operates on an already requeued timer:
it warns and, in the worst case, rewrites the expiry of an enqueued
timer without the base lock.
Convert the timer to the expiry injecting callback variant. The
callback returns the forward request instead of applying it, and the
hrtimer core applies it under the timer base lock. If a concurrent
start requeued the timer while the callback ran, the core discards
the callback's restart request and the start wins, which closes the
park/unpark race without adding any locking to the sampling path.
No functional change in the common case: the timer still forwards by
PERIOD past the sampling timestamp.
Assisted-by: claude-code:claude-fable-5
Signed-off-by: Andreas Hindborg <[email protected]>
---
drivers/gpu/drm/i915/i915_pmu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 1c3bafda9c708..c18587e200c9f 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -502,7 +502,8 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
intel_gt_pm_put_async(gt, wakeref);
}
-static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
+static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer, ktime_t expires,
+ struct hrtimer_forward_args *fwd)
{
struct i915_pmu *pmu = container_of(hrtimer, struct i915_pmu, timer);
struct drm_i915_private *i915 = pmu_to_i915(pmu);
@@ -533,7 +534,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
frequency_sample(gt, period_ns);
}
- hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
+ fwd->now = now;
+ fwd->interval = ns_to_ktime(PERIOD);
return HRTIMER_RESTART;
}
@@ -1157,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
int ret = -ENOMEM;
spin_lock_init(&pmu->lock);
- hrtimer_setup(&pmu->timer, i915_sample, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_setup_ext(&pmu->timer, i915_sample, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
init_rc6(pmu);
if (IS_DGFX(i915)) {
--
2.51.2