[Intel-gfx] [PATCH 01/33] drm/i915: Add smp_rmb() to busy ioctl's RCU dance

[Chris Wilson]

On Tue, Aug 09, 2016 at 09:36:48AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-08-08 at 10:45 +0100, Chris Wilson wrote:
> > On Mon, Aug 08, 2016 at 10:30:25AM +0100, Chris Wilson wrote:
> > > 
> > > On Mon, Aug 08, 2016 at 11:12:59AM +0200, Daniel Vetter wrote:
> > > > 
> > > > On Sun, Aug 07, 2016 at 03:45:09PM +0100, Chris Wilson wrote:
> > > > > 
> > > > > In the debate as to whether the second read of active->request is
> > > > > ordered after the dependent reads of the first read of active->request,
> > > > > just give in and throw a smp_rmb() in there so that ordering of loads is
> > > > > assured.
> > > > > 
> > > > > v2: Explain the manual smp_rmb()
> > > > > 
> > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > r-b confirmed.
> > > It's still fishy that we are implying an SMP effect where we need to
> > > mandate the local processor order (that being the order evaluation of
> > > request = *active; engine = *request; *active). The two *active are
> > > already ordered across SMP, so we are only concered about this cpu. :|
> > More second thoughts. rcu_assign_pointer(NULL) is not visible to
> > rcu_access_pointer on another CPU without the smp_rmb. 
> 
> Should not a RCU read side lock be involved?

Yes, we use rcu read lock here. The question here is about visibility of
the other processor writes vs the local processor order. Before the
other processor can overwrite the request during reallocation, it will
have updated the active->request and gone through a wmb. During busy
ioctl's read of the request, we want to make sure that the values we
read (request->engine, request->seqno) have not been overwritten as we
do so - and we do that by serialising the second pointer check with the
other cpus.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre