From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 05/13] drm: provide device-refcount Date: Wed, 12 Feb 2014 14:25:41 +0100 Message-ID: <20140212132541.GU17001@phenom.ffwll.local> References: <1391004120-687-1-git-send-email-dh.herrmann@gmail.com> <1391004120-687-6-git-send-email-dh.herrmann@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47]) by gabe.freedesktop.org (Postfix) with ESMTP id 4F89FFAE88 for ; Wed, 12 Feb 2014 05:25:51 -0800 (PST) Received: by mail-ee0-f47.google.com with SMTP id d49so4329937eek.34 for ; Wed, 12 Feb 2014 05:25:47 -0800 (PST) Content-Disposition: inline In-Reply-To: <1391004120-687-6-git-send-email-dh.herrmann@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: David Herrmann Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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 [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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch