On Thu, Oct 13, 2016 at 12:51:26PM +0300, Mika Kuoppala wrote: > Chris Wilson <[email protected]> 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
