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

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel