[PATCH 08/15] drm/mode: reduce lock hold in addfb2

[Daniel Vetter]

On Fri, Apr 15, 2016 at 03:10:39PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> No need to hold the lock while assigning the variable.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>

Not sure why exactly I put that under the lock, but the only thing that
can race here is rmfb while addfb2 is still doing it's thing, with a
correctly guess (easy to do since they're fully deterministic) fb_id.

But that race can't happen because rmfb checks whether the fb is
associated with the drm_file, and if not bails out. And since
mutex_lock/unlock together are a full barrier gcc can't even reorder
things so that it would be possible to return a 0 fb_id.

I think I convinced myself that this is totally safe ;-)

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/drm_crtc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 1863879..21cb998 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3405,11 +3405,11 @@ int drm_mode_addfb2(struct drm_device *dev,
>  	if (IS_ERR(fb))
>  		return PTR_ERR(fb);
>  
> -	/* Transfer ownership to the filp for reaping on close */
> -
>  	DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
> -	mutex_lock(&file_priv->fbs_lock);
>  	r->fb_id = fb->base.id;
> +
> +	/* Transfer ownership to the filp for reaping on close */
> +	mutex_lock(&file_priv->fbs_lock);
>  	list_add(&fb->filp_head, &file_priv->fbs);
>  	mutex_unlock(&file_priv->fbs_lock);
>  
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch