On Thu, Oct 13, 2016 at 12:51:26PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> > +static void record_request(struct drm_i915_gem_request *request,
> > + struct drm_i915_error_request *erq)
> > +{
> > + erq->context = request->ctx->hw_id;
> > + erq->seqno = request->fence.seqno;
> > + erq->jiffies = request->emitted_jiffies;
> > + erq->head = request->head;
> > + erq->tail = request->tail;
> > +
> > + rcu_read_lock();
> > + erq->pid = request->ctx->pid ? pid_nr(request->ctx->pid) : 0;
>
> This lock is only for the pid_nr and nothing to do with ctx dereference?
> Not that it was added by this patch...
It's for the struct task lookup inside pid_nr.
But...
> > + for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++)
> > + if (engine->execlist_port[n].request)
> > + record_request(engine->execlist_port[n].request,
> > + &ee->execlist[n]);
>
> Ok even if we get interrupt at around here and reset the ports,
> the pointer should stay in request_list and at that part we should be
> safe.
Note that we don't even get interrupts anymore as we completely stop the
machine whilst capturing. So even rcu_read_lock() above is overkill,
mere documentation.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
