On Fri, Apr 15, 2016 at 03:10:40PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> When we lookup an ref counted object we now take a proper reference
> using kref_get_unless_zero.
>
> Framebuffer lookup no longer needs do this itself.
>
> Convert rmfb to using framebuffer lookup and deal with the fact
> it now gets an extra reference that we have to cleanup. This should
> mean we can avoid holding fb_lock across rmfb. (if I'm wrong let me
> know).
>
> We also now only hold the fbs_lock around the list manipulation.
>
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
Needs the same comment as patch 7 added to the commit message:
"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."
With that: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_crtc.c | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 21cb998..e47c4a2 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -364,6 +364,11 @@ static struct drm_mode_object *_object_find(struct drm_device *dev,
> if (obj &&
> obj->type == DRM_MODE_OBJECT_BLOB)
> obj = NULL;
> +
> + if (obj && obj->free_cb) {
> + if (!kref_get_unless_zero(&obj->refcount))
> + obj = NULL;
> + }
> mutex_unlock(&dev->mode_config.idr_mutex);
>
> return obj;
> @@ -495,11 +500,8 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
>
> mutex_lock(&dev->mode_config.fb_lock);
> obj = _object_find(dev, id, DRM_MODE_OBJECT_FB);
> - if (obj) {
> + if (obj)
> fb = obj_to_fb(obj);
> - if (!kref_get_unless_zero(&fb->base.refcount))
> - fb = NULL;
> - }
> mutex_unlock(&dev->mode_config.fb_lock);
>
> return fb;
> @@ -3434,37 +3436,38 @@ int drm_mode_rmfb(struct drm_device *dev,
> {
> struct drm_framebuffer *fb = NULL;
> struct drm_framebuffer *fbl = NULL;
> - struct drm_mode_object *obj;
> uint32_t *id = data;
> int found = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EINVAL;
>
> + fb = drm_framebuffer_lookup(dev, *id);
> + if (!fb)
> + return -ENOENT;
> +
> mutex_lock(&file_priv->fbs_lock);
> - mutex_lock(&dev->mode_config.fb_lock);
> - obj = _object_find(dev, *id, DRM_MODE_OBJECT_FB);
> - if (!obj)
> - goto fail_lookup;
> - fb = obj_to_fb(obj);
> list_for_each_entry(fbl, &file_priv->fbs, filp_head)
> if (fb == fbl)
> found = 1;
> - if (!found)
> - goto fail_lookup;
> + if (!found) {
> + mutex_unlock(&file_priv->fbs_lock);
> + goto fail_unref;
> + }
>
> list_del_init(&fb->filp_head);
> - mutex_unlock(&dev->mode_config.fb_lock);
> mutex_unlock(&file_priv->fbs_lock);
>
> + /* we now own the reference that was stored in the fbs list */
> drm_framebuffer_unreference(fb);
>
> - return 0;
> + /* drop the reference we picked up in framebuffer lookup */
> + drm_framebuffer_unreference(fb);
>
> -fail_lookup:
> - mutex_unlock(&dev->mode_config.fb_lock);
> - mutex_unlock(&file_priv->fbs_lock);
> + return 0;
>
> +fail_unref:
> + drm_framebuffer_unreference(fb);
> return -ENOENT;
> }
>
> --
> 2.5.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
