On Fri, Apr 15, 2016 at 03:10:38PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> We don't need to hold the fb lock around the initialisation,
> only around the list manipulaton.
> 
> So do the lock hold only around the register for now.
> Signed-off-by: Dave Airlie <airlied at redhat.com>

This needs a bit more explanation added:

"Previously fb refcounting, and especially the weak reference
(kref_get_unless_zero) used in fb lookups have been protected by fb_lock.
But with the refactoring to share refcounting in the drm_mode_object base
class that switched to being protected by idr_mutex, which means fb_lock
critical sections can be reduced."

I also double-checked that we don't have any outdated comments that point
at the wrong lock, and didn't find any.

With the commit message augmented:

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

> ---
>  drivers/gpu/drm/drm_crtc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 0d75517..1863879 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -458,21 +458,22 @@ int drm_framebuffer_init(struct drm_device *dev, struct 
> drm_framebuffer *fb,
>  {
>       int ret;
>  
> -     mutex_lock(&dev->mode_config.fb_lock);
>       INIT_LIST_HEAD(&fb->filp_head);
>       fb->dev = dev;
>       fb->funcs = funcs;
>  
>       ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
> -                                   true, drm_framebuffer_free);
> +                                   false, drm_framebuffer_free);
>       if (ret)
>               goto out;
>  
> +     mutex_lock(&dev->mode_config.fb_lock);
>       dev->mode_config.num_fb++;
>       list_add(&fb->head, &dev->mode_config.fb_list);
> -out:
> -     mutex_unlock(&dev->mode_config.fb_lock);
>  
> +     drm_mode_object_register(dev, &fb->base);
> +     mutex_unlock(&dev->mode_config.fb_lock);
> +out:
>       return ret;
>  }
>  EXPORT_SYMBOL(drm_framebuffer_init);
> -- 
> 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

Reply via email to