| Message ID | 1460697046-23781-10-git-send-email-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 15, 2016 at 03:10:40PM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > 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@redhat.com> 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@ffwll.ch> > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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; }