On Thu, Nov 27, 2014 at 4:49 AM, Daniel Vetter daniel@ffwll.ch wrote:

On Wed, Nov 26, 2014 at 07:08:23PM -0500, Rob Clark wrote:

...

_object_find() is not supposed to check for refcnt'd objects, that is done in drm_mode_object_find().

Signed-off-by: Rob Clark robdclark@gmail.com

drivers/gpu/drm/drm_crtc.c | 3 --- 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9972cc8..3ba84df 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -356,9 +356,6 @@ static struct drm_mode_object *_object_find(struct drm_device *dev, obj = NULL; if (obj && obj->id != id) obj = NULL;

• /* don't leak out unref'd fb's */
  

• if (obj && (obj->type == DRM_MODE_OBJECT_FB))
  

•         obj = NULL;
  mutex_unlock(&dev->mode_config.idr_mutex);
  

Can't we just create a new _object_exists which returns bool? I know that this is safe, but it's also really tricky. And we have to get rid of the fairly useful comment explaining why this is dangerous.

that is basically the point of _object_find() :-)

I'll add a comment that this doesn't take a ref so for refcnt'd mode objects returning non-null only means the object did exist

BR, -R

It's a bit of copypasted code, but imo that's much better here. Especially if there's a comment about refcounted objects. So still nack for this one.

-Daniel

Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch