Re: [Intel-gfx] [PATCH 5/5] dma-buf: nuke reservation_object seq number

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Aug 05, 2019 at 05:07:41PM +0100, Chris Wilson wrote:
> Quoting Christian König (2019-08-05 16:45:54)
> > @@ -214,16 +214,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >                 return 0;
> >  
> >  retry:
> > -       seq = read_seqcount_begin(&resv->seq);
> >         rcu_read_lock();
> >  
> > +       fence_excl = rcu_dereference(resv->fence_excl);
> >         fobj = rcu_dereference(resv->fence);
> >         if (fobj)
> >                 shared_count = fobj->shared_count;
> >         else
> >                 shared_count = 0;
> > -       fence_excl = rcu_dereference(resv->fence_excl);
> > -       if (read_seqcount_retry(&resv->seq, seq)) {
> > +
> > +       if (rcu_dereference(resv->fence_excl) != fence_excl) {
> 
> If I remember my rules correctly, rcu_dereference is a
> read-data-depends, which only means that a read through the pointer
> returned by rcu_dereference() is after the retrieval of that pointer.
> Nothing orders the retrieval of fence_excl vs shared_count (different
> pointers), so I think the last line should be:
> 
> smp_rmb();
> if (rcu_access_pointer(resv->fence_excl) != fence_excl)

Also, barriers must have a comment, the comment must be on both barriers
(if you don't have two, you're doing it wrong), and each barrier comment
must reference its counterpart.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch