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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx