drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference
Paul E. McKenney
paulmck at linux.vnet.ibm.com
Thu Aug 11 17:57:01 UTC 2016
On Thu, Aug 11, 2016 at 11:44:08AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 22, 2016 at 08:46:12AM +0100, Chris Wilson wrote:
> > We are only documenting that the read is outside of the lock, and do not
> > require strict ordering on the operation. In this case the more relaxed
> > lockless_dereference() will suffice.
>
> No, no, no... This is 'broken'. lockless_dereference() is _stronger_
> than READ_ONCE(), not weaker.
>
> lockless_dereference() is a wrapper around smp_read_barrier_depends()
> and is used to form read dependencies. There is no read dependency here,
> therefore using lockless_dereference() is entirely pointless.
>
> Look at the definition of lockless_dereference(), it does a READ_ONCE()
> and then smp_read_barrier_depends().
>
> Also, clue is in the name: 'dereference', you don't actually dereference
> the pointer here, only load it.
What Peter said!
If you are just checking the value of an RCU-protected pointer,
that is, one on which you would use rcu_dereference() rather than
lockless_dereference(), rcu_access_pointer() does the job of READ_ONCE()
while keeping sparse happy.
Thanx, Paul
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> >
> > /* Sometimes user space wants everything disabled, so don't steal the
> > * display if there's a master. */
> > - if (READ_ONCE(dev->master))
> > + if (lockless_dereference(dev->master))
> > return false;
> >
> > drm_for_each_crtc(crtc, dev) {
>
More information about the dri-devel
mailing list