Hi
On Wed, Feb 12, 2014 at 2:25 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Jan 29, 2014 at 03:01:52PM +0100, David Herrmann wrote:
Lets not trick ourselves into thinking "drm_device" objects are not ref-counted. That's just utterly stupid. We manage "drm_minor" objects on each drm-device and each minor can have an unlimited number of open handles. Each of these handles has the drm_minor (and thus the drm_device) as private-data in the file-handle. Therefore, we may not destroy "drm_device" until all these handles are closed.
It is *not* possible to reset all these pointers atomically and restrict access to them, and this is *not* how this is done! Instead, we use ref-counts to make sure the object is valid and not freed.
Note that we currently use "dev->open_count" for that, which is *exactly* the same as a reference-count, just open coded. So this patch doesn't change any semantics on DRM devices (well, this patch just introduces the ref-count, anyway. Follow-up patches will replace open_count by it).
Also note that generic VFS revoke support could allow us to drop this ref-count again. We could then just synchronously disable any fops->xy() calls. However, this is not the case, yet, and no such patches are in sight (and I seriously question the idea of dropping the ref-cnt again).
Signed-off-by: David Herrmann dh.herrmann@gmail.com
[snip]
+/**
- drm_dev_ref - Take reference of a DRM device
- @dev: device to take reference of or NULL
- This increases the ref-count of @dev by one. You *must* already own a
- reference when calling this. Use drm_dev_unref() to drop this reference
- again.
- This function never fails. However, this function does not provide *any*
- guarantee whether the device is alive or running. It only provides a
- reference to the object and the memory associated with it.
- */
+void drm_dev_ref(struct drm_device *dev) +{
if (dev)This check here (and below in the unref code) look funny. What's the reason for it? Trying to grab/drop a ref on a NULL pointer sounds like a pretty serious bug to me. This is in contrast to kfree(NULL) which imo makes sense - freeing nothing is a legitimate operation imo.
I added it mainly to simplify cleanup-code paths. You can then just call unref() and set it to NULL regardless whether you actually hold a reference or not. For ref() I don't really care but I think the NULL-test doesn't hurt either.
I copied this behavior from get_device() and put_device(), btw. Similar to these functions, I think a lot more will go wrong if the NULL pointer is not intentional. Imo, ref-counting on a NULL object just means "no object", so it shouldn't do anything.
Thanks David