Re: [PATCH 5/7] drm/i915: Request watchdog infrastructure

On 26/03/2021 00:00, Daniel Vetter wrote:

On Wed, Mar 24, 2021 at 12:13:33PM +0000, Tvrtko Ursulin wrote:

[snip]

...

+static enum hrtimer_restart __rq_watchdog_expired(struct hrtimer *hrtimer) +{

• struct i915_request *rq =

• container_of(hrtimer, struct i915_request, watchdog.timer);
  

• struct intel_gt *gt = rq->engine->gt;
• if (!i915_request_completed(rq)) {

• if (llist_add(&rq->watchdog.link, &gt->watchdog.list))
  

• 	schedule_work(&gt->watchdog.work);
  

• } else {

• i915_request_put(rq);
  

• }
• return HRTIMER_NORESTART;

+}

+static void __rq_arm_watchdog(struct i915_request *rq) +{

• struct i915_request_watchdog *wdg = &rq->watchdog;
• struct intel_context *ce = rq->context;
• if (!ce->watchdog.timeout_us)

• return;
  

• hrtimer_init(&wdg->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
• wdg->timer.function = __rq_watchdog_expired;
• hrtimer_start_range_ns(&wdg->timer,

• 	       ns_to_ktime(ce->watchdog.timeout_us *
  

• 			   NSEC_PER_USEC),
  

• 	       NSEC_PER_MSEC,
  

• 	       HRTIMER_MODE_REL);
  

• i915_request_get(rq);

Shouldn't we grab the new reference before we arm the timer? Either way since fairly academic I went ahead and applied, but if you agree pls do a follow up patch.

Absolutely true.. my bad.

Regards,

Tvrtko