Quoting Tvrtko Ursulin (2018-01-17 12:22:33)
>
> On 15/01/2018 21:24, Chris Wilson wrote:
> > Avoid calling dma_fence_signal() from inside the interrupt if we haven't
> > enabled signaling on the request.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_gem_request.c | 2 +-
> > drivers/gpu/drm/i915/i915_irq.c | 3 ++-
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ++---
> > 3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> > index 08bbd56277e5..46848aef1648 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -1214,7 +1214,7 @@ long i915_wait_request(struct drm_i915_gem_request *req,
> > if (flags & I915_WAIT_LOCKED)
> > add_wait_queue(errq, &reset);
> >
> > - intel_wait_init(&wait, req);
> > + intel_wait_init(&wait);
> >
> > restart:
> > do {
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index e5f76d580010..ea290f102784 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1093,7 +1093,8 @@ static void notify_ring(struct intel_engine_cs *engine)
> > if (i915_seqno_passed(seqno, wait->seqno)) {
> > struct drm_i915_gem_request *waiter = wait->request;
> >
> > - if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > + if (waiter &&
> > + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > &waiter->fence.flags) &&
> > intel_wait_check_request(wait, waiter))
> > rq = i915_gem_request_get(waiter);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index f406d0ff4612..cea2092d25d9 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -874,11 +874,10 @@ static inline u32 intel_hws_preempt_done_address(struct intel_engine_cs *engine)
> > /* intel_breadcrumbs.c -- user interrupt bottom-half for waiters */
> > int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
> >
> > -static inline void intel_wait_init(struct intel_wait *wait,
> > - struct drm_i915_gem_request *rq)
> > +static inline void intel_wait_init(struct intel_wait *wait)
> > {
> > wait->tsk = current;
> > - wait->request = rq;
> > + wait->request = NULL;
>
> Unless it is in this patch series, I can't see that this now leaves any
> code path which sets wait->request?
The signaler is now the only entity that sets up wait.request.
> It's strange anyway - is this guarding against null ptr deref or what?
Uninitialized pointer deref in the irq handler, yup.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
