From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: Don't reference objects in the flink name idr Date: Tue, 3 Dec 2013 16:26:22 +0100 Message-ID: <20131203152622.GE27344@phenom.ffwll.local> References: <1386034577-9195-1-git-send-email-krh@bitplanet.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f49.google.com (mail-wg0-f49.google.com [74.125.82.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 05AF5FB462 for ; Tue, 3 Dec 2013 07:25:39 -0800 (PST) Received: by mail-wg0-f49.google.com with SMTP id x12so13911566wgg.16 for ; Tue, 03 Dec 2013 07:25:39 -0800 (PST) Content-Disposition: inline In-Reply-To: <1386034577-9195-1-git-send-email-krh@bitplanet.net> 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: Kristian =?iso-8859-1?Q?H=F8gsberg?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Mon, Dec 02, 2013 at 05:36:17PM -0800, Kristian H=F8gsberg wrote: > There's no reason to keep a reference to objects in the name idr. Each > handle to an object has a reference to the object and just before we > destroy the last handle we take the object out of the name idr. Thus, > if an object is in the name idr, there's at least one reference to the > object. > = > Or to put it another way, the name idr reference will never keep the > object alive. It just looks like it, which is confusing. > = > Signed-off-by: Kristian H=F8gsberg I expect this to blow up when you race gem_close ioctl calls with flink open. i-g-t/gem_flink_close tests actually have been written specifically to exercise these races. -Daniel > --- > drivers/gpu/drm/drm_gem.c | 15 --------------- > 1 file changed, 15 deletions(-) > = > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 4761ade..3ff39bb 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -175,11 +175,6 @@ drm_gem_remove_prime_handles(struct drm_gem_object *= obj, struct drm_file *filp) > mutex_unlock(&filp->prime.lock); > } > = > -static void drm_gem_object_ref_bug(struct kref *list_kref) > -{ > - BUG(); > -} > - > /** > * Called after the last handle to the object has been closed > * > @@ -195,13 +190,6 @@ static void drm_gem_object_handle_free(struct drm_ge= m_object *obj) > if (obj->name) { > idr_remove(&dev->object_name_idr, obj->name); > obj->name =3D 0; > - /* > - * The object name held a reference to this object, drop > - * that now. > - * > - * This cannot be the last reference, since the handle holds one too. > - */ > - kref_put(&obj->refcount, drm_gem_object_ref_bug); > } > } > = > @@ -602,9 +590,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *dat= a, > goto err; > = > obj->name =3D ret; > - > - /* Allocate a reference for the name table. */ > - drm_gem_object_reference(obj); > } > = > args->name =3D (uint64_t) obj->name; > -- = > 1.8.3.1 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch