drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference

Peter Zijlstra peterz at infradead.org
Thu Aug 11 09:44:08 UTC 2016


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.

> +++ 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