Commit 152ef5fa authored by Daniel Vetter's avatar Daniel Vetter Committed by Dave Airlie

drm: Switch blobs to the new generic modeset obj refcounting

Need to move the free function around a bit, but otherwise mostly
just removing code.

Specifically we can nuke all the _locked variants since the weak idr
reference is now protected by the idr_mutex, which we never hold
anywhere expect in the lookup/reg/unreg functions. And those never
call anything else.

Another benefit of this is that this patch switches the weak reference
logic from kref_put_mutex to kref_get_unless_zero. And the later is in
general more flexible wrt accomodating multiple weak references
protected by different locks, which might or might not come handy
eventually.

But one consequence of that switch is that we need to acquire the
blob_lock from the free function for the list_del calls. That's a bit
tricky to pull off, but works well if we pick the exact same scheme as
is already used for framebuffers. Most important changes:

- filp list is maintainer by create/destroy_blob ioctls directly
  (already the case, so we can just remove the redundant list_del from
  the free function).

- filp close handler walks the filp-private list lockless - works
  because we know no one else can access it. I copied the same comment
  from the fb code over to explain this.

- Otherwise we need to sufficiently restrict blob_lock critical
  sections to avoid all the unreference calls. Easy to do once the
  blob_lock only protects the list, and no longer the weak reference.

Cc: Dave Airlie <airlied@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
parent b0b5511b
...@@ -360,10 +360,6 @@ static struct drm_mode_object *_object_find(struct drm_device *dev, ...@@ -360,10 +360,6 @@ static struct drm_mode_object *_object_find(struct drm_device *dev,
obj = NULL; obj = NULL;
if (obj && obj->id != id) if (obj && obj->id != id)
obj = NULL; obj = NULL;
/* don't leak out unref'd fb's */
if (obj &&
obj->type == DRM_MODE_OBJECT_BLOB)
obj = NULL;
if (obj && obj->free_cb) { if (obj && obj->free_cb) {
if (!kref_get_unless_zero(&obj->refcount)) if (!kref_get_unless_zero(&obj->refcount))
...@@ -389,7 +385,6 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, ...@@ -389,7 +385,6 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
{ {
struct drm_mode_object *obj = NULL; struct drm_mode_object *obj = NULL;
WARN_ON(type == DRM_MODE_OBJECT_BLOB);
obj = _object_find(dev, id, type); obj = _object_find(dev, id, type);
return obj; return obj;
} }
...@@ -4259,6 +4254,20 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev, ...@@ -4259,6 +4254,20 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
return ret; return ret;
} }
static void drm_property_free_blob(struct kref *kref)
{
struct drm_property_blob *blob =
container_of(kref, struct drm_property_blob, base.refcount);
mutex_lock(&blob->dev->mode_config.blob_lock);
list_del(&blob->head_global);
mutex_unlock(&blob->dev->mode_config.blob_lock);
drm_mode_object_unregister(blob->dev, &blob->base);
kfree(blob);
}
/** /**
* drm_property_create_blob - Create new blob property * drm_property_create_blob - Create new blob property
* *
...@@ -4296,47 +4305,22 @@ drm_property_create_blob(struct drm_device *dev, size_t length, ...@@ -4296,47 +4305,22 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
if (data) if (data)
memcpy(blob->data, data, length); memcpy(blob->data, data, length);
mutex_lock(&dev->mode_config.blob_lock); ret = drm_mode_object_get_reg(dev, &blob->base, DRM_MODE_OBJECT_BLOB,
true, drm_property_free_blob);
ret = drm_mode_object_get(dev, &blob->base, DRM_MODE_OBJECT_BLOB);
if (ret) { if (ret) {
kfree(blob); kfree(blob);
mutex_unlock(&dev->mode_config.blob_lock);
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
} }
kref_init(&blob->refcount); mutex_lock(&dev->mode_config.blob_lock);
list_add_tail(&blob->head_global, list_add_tail(&blob->head_global,
&dev->mode_config.property_blob_list); &dev->mode_config.property_blob_list);
mutex_unlock(&dev->mode_config.blob_lock); mutex_unlock(&dev->mode_config.blob_lock);
return blob; return blob;
} }
EXPORT_SYMBOL(drm_property_create_blob); EXPORT_SYMBOL(drm_property_create_blob);
/**
* drm_property_free_blob - Blob property destructor
*
* Internal free function for blob properties; must not be used directly.
*
* @kref: Reference
*/
static void drm_property_free_blob(struct kref *kref)
{
struct drm_property_blob *blob =
container_of(kref, struct drm_property_blob, refcount);
WARN_ON(!mutex_is_locked(&blob->dev->mode_config.blob_lock));
list_del(&blob->head_global);
list_del(&blob->head_file);
drm_mode_object_unregister(blob->dev, &blob->base);
kfree(blob);
}
/** /**
* drm_property_unreference_blob - Unreference a blob property * drm_property_unreference_blob - Unreference a blob property
* *
...@@ -4346,41 +4330,13 @@ static void drm_property_free_blob(struct kref *kref) ...@@ -4346,41 +4330,13 @@ static void drm_property_free_blob(struct kref *kref)
*/ */
void drm_property_unreference_blob(struct drm_property_blob *blob) void drm_property_unreference_blob(struct drm_property_blob *blob)
{ {
struct drm_device *dev;
if (!blob) if (!blob)
return; return;
dev = blob->dev; drm_mode_object_unreference(&blob->base);
DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount));
if (kref_put_mutex(&blob->refcount, drm_property_free_blob,
&dev->mode_config.blob_lock))
mutex_unlock(&dev->mode_config.blob_lock);
else
might_lock(&dev->mode_config.blob_lock);
} }
EXPORT_SYMBOL(drm_property_unreference_blob); EXPORT_SYMBOL(drm_property_unreference_blob);
/**
* drm_property_unreference_blob_locked - Unreference a blob property with blob_lock held
*
* Drop a reference on a blob property. May free the object. This must be
* called with blob_lock held.
*
* @blob: Pointer to blob property
*/
static void drm_property_unreference_blob_locked(struct drm_property_blob *blob)
{
if (!blob)
return;
DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount));
kref_put(&blob->refcount, drm_property_free_blob);
}
/** /**
* drm_property_destroy_user_blobs - destroy all blobs created by this client * drm_property_destroy_user_blobs - destroy all blobs created by this client
* @dev: DRM device * @dev: DRM device
...@@ -4391,14 +4347,14 @@ void drm_property_destroy_user_blobs(struct drm_device *dev, ...@@ -4391,14 +4347,14 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
{ {
struct drm_property_blob *blob, *bt; struct drm_property_blob *blob, *bt;
mutex_lock(&dev->mode_config.blob_lock); /*
* When the file gets released that means no one else can access the
* blob list any more, so no need to grab dev->blob_lock.
*/
list_for_each_entry_safe(blob, bt, &file_priv->blobs, head_file) { list_for_each_entry_safe(blob, bt, &file_priv->blobs, head_file) {
list_del_init(&blob->head_file); list_del_init(&blob->head_file);
drm_property_unreference_blob_locked(blob); drm_property_unreference_blob(blob);
} }
mutex_unlock(&dev->mode_config.blob_lock);
} }
/** /**
...@@ -4410,35 +4366,11 @@ void drm_property_destroy_user_blobs(struct drm_device *dev, ...@@ -4410,35 +4366,11 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
*/ */
struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob) struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
{ {
DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount)); drm_mode_object_reference(&blob->base);
kref_get(&blob->refcount);
return blob; return blob;
} }
EXPORT_SYMBOL(drm_property_reference_blob); EXPORT_SYMBOL(drm_property_reference_blob);
/*
* Like drm_property_lookup_blob, but does not return an additional reference.
* Must be called with blob_lock held.
*/
static struct drm_property_blob *__drm_property_lookup_blob(struct drm_device *dev,
uint32_t id)
{
struct drm_mode_object *obj = NULL;
struct drm_property_blob *blob;
WARN_ON(!mutex_is_locked(&dev->mode_config.blob_lock));
mutex_lock(&dev->mode_config.idr_mutex);
obj = idr_find(&dev->mode_config.crtc_idr, id);
if (!obj || (obj->type != DRM_MODE_OBJECT_BLOB) || (obj->id != id))
blob = NULL;
else
blob = obj_to_blob(obj);
mutex_unlock(&dev->mode_config.idr_mutex);
return blob;
}
/** /**
* drm_property_lookup_blob - look up a blob property and take a reference * drm_property_lookup_blob - look up a blob property and take a reference
* @dev: drm device * @dev: drm device
...@@ -4451,16 +4383,12 @@ static struct drm_property_blob *__drm_property_lookup_blob(struct drm_device *d ...@@ -4451,16 +4383,12 @@ static struct drm_property_blob *__drm_property_lookup_blob(struct drm_device *d
struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev, struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev,
uint32_t id) uint32_t id)
{ {
struct drm_property_blob *blob; struct drm_mode_object *obj;
struct drm_property_blob *blob = NULL;
mutex_lock(&dev->mode_config.blob_lock);
blob = __drm_property_lookup_blob(dev, id);
if (blob) {
if (!kref_get_unless_zero(&blob->refcount))
blob = NULL;
}
mutex_unlock(&dev->mode_config.blob_lock);
obj = _object_find(dev, id, DRM_MODE_OBJECT_BLOB);
if (obj)
blob = obj_to_blob(obj);
return blob; return blob;
} }
EXPORT_SYMBOL(drm_property_lookup_blob); EXPORT_SYMBOL(drm_property_lookup_blob);
...@@ -4565,26 +4493,21 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, ...@@ -4565,26 +4493,21 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET)) if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL; return -EINVAL;
drm_modeset_lock_all(dev); blob = drm_property_lookup_blob(dev, out_resp->blob_id);
mutex_lock(&dev->mode_config.blob_lock); if (!blob)
blob = __drm_property_lookup_blob(dev, out_resp->blob_id); return -ENOENT;
if (!blob) {
ret = -ENOENT;
goto done;
}
if (out_resp->length == blob->length) { if (out_resp->length == blob->length) {
blob_ptr = (void __user *)(unsigned long)out_resp->data; blob_ptr = (void __user *)(unsigned long)out_resp->data;
if (copy_to_user(blob_ptr, blob->data, blob->length)) { if (copy_to_user(blob_ptr, blob->data, blob->length)) {
ret = -EFAULT; ret = -EFAULT;
goto done; goto unref;
} }
} }
out_resp->length = blob->length; out_resp->length = blob->length;
unref:
drm_property_unreference_blob(blob);
done:
mutex_unlock(&dev->mode_config.blob_lock);
drm_modeset_unlock_all(dev);
return ret; return ret;
} }
...@@ -4663,13 +4586,11 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev, ...@@ -4663,13 +4586,11 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET)) if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL; return -EINVAL;
mutex_lock(&dev->mode_config.blob_lock); blob = drm_property_lookup_blob(dev, out_resp->blob_id);
blob = __drm_property_lookup_blob(dev, out_resp->blob_id); if (!blob)
if (!blob) { return -ENOENT;
ret = -ENOENT;
goto err;
}
mutex_lock(&dev->mode_config.blob_lock);
/* Ensure the property was actually created by this user. */ /* Ensure the property was actually created by this user. */
list_for_each_entry(bt, &file_priv->blobs, head_file) { list_for_each_entry(bt, &file_priv->blobs, head_file) {
if (bt == blob) { if (bt == blob) {
...@@ -4686,13 +4607,18 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev, ...@@ -4686,13 +4607,18 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
/* We must drop head_file here, because we may not be the last /* We must drop head_file here, because we may not be the last
* reference on the blob. */ * reference on the blob. */
list_del_init(&blob->head_file); list_del_init(&blob->head_file);
drm_property_unreference_blob_locked(blob);
mutex_unlock(&dev->mode_config.blob_lock); mutex_unlock(&dev->mode_config.blob_lock);
/* One reference from lookup, and one from the filp. */
drm_property_unreference_blob(blob);
drm_property_unreference_blob(blob);
return 0; return 0;
err: err:
mutex_unlock(&dev->mode_config.blob_lock); mutex_unlock(&dev->mode_config.blob_lock);
drm_property_unreference_blob(blob);
return ret; return ret;
} }
......
...@@ -250,7 +250,6 @@ struct drm_framebuffer { ...@@ -250,7 +250,6 @@ struct drm_framebuffer {
struct drm_property_blob { struct drm_property_blob {
struct drm_mode_object base; struct drm_mode_object base;
struct drm_device *dev; struct drm_device *dev;
struct kref refcount;
struct list_head head_global; struct list_head head_global;
struct list_head head_file; struct list_head head_file;
size_t length; size_t length;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment