From patchwork Fri Nov 30 13:58:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Inki Dae X-Patchwork-Id: 1825401 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 854C53FC23 for ; Fri, 30 Nov 2012 13:59:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 75E98E5E9F for ; Fri, 30 Nov 2012 05:59:46 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pa0-f49.google.com (mail-pa0-f49.google.com [209.85.220.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 760CBE67C4 for ; Fri, 30 Nov 2012 05:59:11 -0800 (PST) Received: by mail-pa0-f49.google.com with SMTP id bi1so327688pad.36 for ; Fri, 30 Nov 2012 05:59:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=tLdqrxzxsksvuXPmGratp52dPTZ001+DtdKBLBY8w5g=; b=LzXaXnbI2ql7p10b18eA5kUNdpWXJhHRmYuveoLF6bJbbb8IQtzlEbmlEQGudpiJw1 Mall3NqDC7k7hyKTXEsQMBMTWVRjFUkCOIXMxgI6hfpMYEd+yAl7YcpzGMdeDH4vvBT0 DnumMXj6V2leW6Gz9eJIZZVHaJffJoqZaM8kCg4Gp5mmKXrhBlDEHCRnaaiiGbJlZ8Ht q5ES2J/dCiOZyDeaoPNhPPUoJ16x6XDlogScAyh2Djdt0v1yl/Gxek6U4W7wMRIrh2rp BgiOtO7jtUj6H1cg42mRgCT2RmXDH35vy0OIW+uEtoAUEddZL6icl/WofkvsqDFp6CvF DXTg== Received: by 10.68.132.232 with SMTP id ox8mr5607557pbb.46.1354283951245; Fri, 30 Nov 2012 05:59:11 -0800 (PST) Received: from localhost.localdomain ([222.117.162.7]) by mx.google.com with ESMTPS id u5sm2966719pax.6.2012.11.30.05.59.08 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 30 Nov 2012 05:59:09 -0800 (PST) From: inki.dae@samsung.com To: airlied@linux.ie, dri-devel@lists.freedesktop.org Subject: [RESEND][RFC v1] drm: add reference count of gem object name Date: Fri, 30 Nov 2012 05:58:44 -0800 Message-Id: <1354283924-3621-1-git-send-email-inki.dae@samsung.com> X-Mailer: git-send-email 1.8.0.rc3.16.g8ead1bf In-Reply-To: <1354266615-14013-1-git-send-email-inki.dae@samsung.com> References: <1354266615-14013-1-git-send-email-inki.dae@samsung.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org From: Inki Dae Hello, all. This patch introduces reference count of gem object name and resolves the below issue. In case that proces A shares its own gem object with process B, process B opens a gem object name from process A to get its own gem handle. But if process A closes the gem handle, the gem object name to this is also released. So the gem object name that process B referring to becomes invalid. I'm not sure that this is issue or not but at least, gem object name IS NOT process-unique but IS gem object-unique. So I think the gem object name shared by process A should be preserved as long as the gem object has not been released. Below is simple example. at P1: 1. gem create => obj_refcount = 1 2. gem flink => obj_refcount = 2 (allocate obj_name) 5. gem close a. obj_refcount = 2 and release the obj_name b. obj_refcount = 1 once the obj_name release 3. and share it with P2 at P2: 4. gem open => obj_refcount = 3 6. the obj_name from P1 is invalid. 7. gem close => obj_refcount = 0(released) And with this patch, at P1: 1. gem create => obj_refcount = 1, name_refcount = 0 2. gem flink => obj_refcount = 2, name_refcount = 1 (allocate obj_name) 5. gem close => obj_refcount = 2, name_refcount = 1 3. and share it with P2 at P2: 4. gem open => obj_refcount = 3, name_refcount = 2 6. the gem object name from P1 is valid. 7. gem close a. obj_refcount = 1, name_refcount = 0(released) b. obj_refcount = 0(released) once name_ref_count = 0 There may be my missing point so please give me any advice. Thanks, Inki Dae Signed-off-by: Inki Dae --- drivers/gpu/drm/drm_gem.c | 37 ++++++++++++++++++++++++++++++++++--- include/drm/drmP.h | 12 ++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 24efae4..946f46c 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -145,6 +145,7 @@ int drm_gem_object_init(struct drm_device *dev, kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0); + atomic_set(&obj->obj_name_count, 0); obj->size = size; return 0; @@ -166,6 +167,7 @@ int drm_gem_private_object_init(struct drm_device *dev, kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0); + atomic_set(&obj->obj_name_count, 0); obj->size = size; return 0; @@ -452,6 +454,16 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, return -ENOENT; again: + /* + * return current object name increasing reference count of + * this object name if exists already. + */ + if (obj->name) { + args->name = (uint64_t)obj->name; + drm_gem_object_unreference_unlocked(obj); + return 0; + } + if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) { ret = -ENOMEM; goto err; @@ -461,6 +473,7 @@ again: if (!obj->name) { ret = idr_get_new_above(&dev->object_name_idr, obj, 1, &obj->name); + atomic_set(&obj->obj_name_count, 1); args->name = (uint64_t) obj->name; spin_unlock(&dev->object_name_lock); @@ -502,8 +515,10 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data, spin_lock(&dev->object_name_lock); obj = idr_find(&dev->object_name_idr, (int) args->name); - if (obj) + if (obj) { drm_gem_object_reference(obj); + atomic_inc(&obj->obj_name_count); + } spin_unlock(&dev->object_name_lock); if (!obj) return -ENOENT; @@ -612,9 +627,25 @@ void drm_gem_object_handle_free(struct drm_gem_object *obj) /* Remove any name for this object */ spin_lock(&dev->object_name_lock); if (obj->name) { - idr_remove(&dev->object_name_idr, obj->name); - obj->name = 0; + /* + * The name of this object could being referenced + * by another process so remove the name after checking + * the obj_name_count of this object. + */ + if (atomic_dec_and_test(&obj->obj_name_count)) { + idr_remove(&dev->object_name_idr, obj->name); + obj->name = 0; + } else { + /* + * this object name is being referenced by other yet + * so do not unreference this. + */ + spin_unlock(&dev->object_name_lock); + return; + } + spin_unlock(&dev->object_name_lock); + /* * The object name held a reference to this object, drop * that now. diff --git a/include/drm/drmP.h b/include/drm/drmP.h index fad21c9..27657b8 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -628,6 +628,18 @@ struct drm_gem_object { /** Handle count of this object. Each handle also holds a reference */ atomic_t handle_count; /* number of handles on this object */ + /* + * Name count of this object. + * + * This count is initialized as 0 with drm_gem_object_init or + * drm_gem_private_object_init call. After that, this count is + * increased if the name of this object exists already + * otherwise is set to 1 at flink. And finally, the name of + * this object will be released when this count reaches 0 + * by gem close. + */ + atomic_t obj_name_count; + /** Related drm device */ struct drm_device *dev;