On Mon, Oct 31, 2016 at 05:35:50PM +0000, Tvrtko Ursulin wrote:
>
> On 31/10/2016 10:26, Chris Wilson wrote:
> >Whilst waiting on a request, we may do so without holding any locks or
> >any guards beyond a reference to the request. In order to avoid taking
> >locks within request deallocation, we drop references to its timeline
> >(via the context and ppgtt) upon retirement. We should avoid chasing
>
> Couldn't find that there is a reference taken (or dropped) on the
> timeline when stored in a request. It looks like a borrowed pointer
> to me?
The timeline is owned by the address space which is owned by either the
context or the device. The request holds a reference on the context (and
so indirectly onto the timeline, except for the device's which outlives
the request) up until we retire the request. (Retiring holds
struct_mutex so is earlier than freeing.)
> >+static inline u32 intel_engine_last_submit(struct intel_engine_cs *engine)
> >+{
> >+ return READ_ONCE(engine->timeline->last_submitted_seqno);
> >+}
> >+
>
> Don't like that READ_ONCE gets sprinkled all over the place via call
> sites. It should be extremely well defined and controlled from where
> it is used. Otherwise it suggests READ_ONCE is not really
> appropriate.
It actually is appropriate. last_submitted_seqno is under the timeline
spinlock, none of the callers take the appropriate guard. This is
trying to document that these callers don't call about fully
synchronising the last_submitted_seqno with the request list or last
request pointer. And we don't care to go full seqlock, since we really
are only peeking.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
