[PATCH v6] drm/i915: Fix NULL pointer dereference in capture_engine

[Andi Shyti]

Hi Jani,

On Wed, Dec 04, 2024 at 12:51:56PM +0200, Jani Nikula wrote:
> On Wed, 04 Dec 2024, Andi Shyti <andi.shyti at linux.intel.com> wrote:
> > Hi Michal,
> >
> >> > +	if (rq && !i915_request_started(rq)) {
> >> > +		/*
> >> > +		* We want to know also what is the gcu_id of the context,
> >> 
> >> typo: guc_id
> >> 
> >> > +		* but if we don't have the context reference, then skip
> >> > +		* printing it.
> >> > +		*/
> >> 
> >> but IMO this comment is redundant as it's quite obvious that without
> >> context pointer you can't print guc_id member
> >
> > I recommended to add a comment because there is some code
> > redundancy that, I think, needs some explanation; someone might
> > not spot immediately the need for ce, unless goes a the end of
> > the drm_info parameter's list.
> >
> >> > +		if (ce)
> >> > +			drm_info(&engine->gt->i915->drm,
> >> > +				"Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n",
> >> > +				engine->name, rq->fence.context, rq->fence.seqno, ce->guc_id.id);
> >> > +		else
> >> > +			drm_info(&engine->gt->i915->drm,
> >> > +				"Got hung context on %s with active request %lld:%lld not yet started\n",
> >> > +				engine->name, rq->fence.context, rq->fence.seqno);
> >> 
> >> since you are touching drm_info() where we use engine->gt then maybe
> >> it's good time to switch to gt_info() to get better per-GT message?
> >
> > I think the original reason for using drm_info is because we are
> > outside the GT. But, because the engine belongs to the GT, it
> > makes also sense to use gt_info(), I don't oppose.
> >
> > It would make more sense to move this function completely into
> > gt/.
> 
> Can we converge on the patch instead of diverge, please?
> 
> It's a Cc: stable null pointer dereference fix.
> 
> It's already been iterated for two weeks to reach v6.
> 
> Fix the comment typo while applying, but there's nothing inherently
> wrong here AFAICT. Merge it and move on.

Thanks for the feedback, will go ahead and merge.

All other gt/ adjustments can be done later.

Andi