From patchwork Fri Nov 30 09:10:15 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Inki Dae X-Patchwork-Id: 1824221 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 39FA9DF24C for ; Fri, 30 Nov 2012 09:30:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0AA77E66B9 for ; Fri, 30 Nov 2012 01:30:59 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mailout2.samsung.com (mailout2.samsung.com [203.254.224.25]) by gabe.freedesktop.org (Postfix) with ESMTP id 220B9E5F6A for ; Fri, 30 Nov 2012 01:30:44 -0800 (PST) Received: from epcpsbgm2.samsung.com (epcpsbgm2 [203.254.230.27]) by mailout2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MEA00FEFMSO6NN0@mailout2.samsung.com> for dri-devel@lists.freedesktop.org; Fri, 30 Nov 2012 18:10:16 +0900 (KST) Received: from epcpsbgm2.samsung.com ( [203.254.230.42]) by epcpsbgm2.samsung.com (EPCPMTA) with SMTP id 7A.95.12699.8F778B05; Fri, 30 Nov 2012 18:10:16 +0900 (KST) X-AuditID: cbfee61b-b7f616d00000319b-93-50b877f82be1 Received: from epmmp1.local.host ( [203.254.227.16]) by epcpsbgm2.samsung.com (EPCPMTA) with SMTP id 3A.95.12699.8F778B05; Fri, 30 Nov 2012 18:10:16 +0900 (KST) Received: from daeinki-desktop.10.32.193.11 ([10.90.51.53]) by mmp1.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MEA002QVMT49W50@mmp1.samsung.com> for dri-devel@lists.freedesktop.org; Fri, 30 Nov 2012 18:10:16 +0900 (KST) From: Inki Dae To: airlied@linux.ie, dri-devel@lists.freedesktop.org Subject: [RFC v1] drm: add reference count of gem object name Date: Fri, 30 Nov 2012 18:10:15 +0900 Message-id: <1354266615-14013-1-git-send-email-inki.dae@samsung.com> X-Mailer: git-send-email 1.7.4.1 DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrCLMWRmVeSWpSXmKPExsVy+t8zLd0f5TsCDHr/Klhc+fqezYHR4373 caYAxigum5TUnMyy1CJ9uwSujIPPX7MUbNesWHkiv4HxiUIXIyeHhICJxJoXXUwQtpjEhXvr 2boYuTiEBJYxSmzddIsVpujs03nMEIlFjBKT96xihXDWM0m82vGDHaSKTUBVYuKK+2wgtoiA qUTHpKUsIDazgLLExf6lzCC2sICtxOu+U4wgNgtQfd+ev2A1vAIuEkv2fWSB2KYgseDeWzaI GgGJb5MPAcU5gOKyEpsOgB0hIbCDTWJpTyPU2ZISB1fcYJnAKLiAkWEVo2hqQXJBcVJ6rpFe cWJucWleul5yfu4mRkhQSe9gXNVgcYhRgINRiYeXYc32ACHWxLLiytxDjBIczEoivPsEdwQI 8aYkVlalFuXHF5XmpBYfYvQBumQis5Rocj4w4PNK4g2NDYwNDS0NzUwtTQ1wCCuJ8zZ7pAQI CaQnlqRmp6YWpBbBjGPi4JRqYCz5duhHoqHQ1r9vv804WHDs+mTTPusv+59cuCusH8QrXLjA 5N6NTeeyQ29uEv3qLKLv0tXdnX07dKniQ7N7nBu0BVbM2Hl+yb6HR182aV7fab38aqDdwgMh h1pfXxJlrnqYcD6l623r+iuHtC3eXfr42VA9otbDL9u4TKauXK6t4unfP5p+D78osRRnJBpq MRcVJwIAyGBGc1cCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrMLMWRmVeSWpSXmKPExsVy+t9jAd0f5TsCDCY/FLe48vU9mwOjx/3u 40wBjFENjDYZqYkpqUUKqXnJ+SmZeem2St7B8c7xpmYGhrqGlhbmSgp5ibmptkouPgG6bpk5 QGOVFMoSc0qBQgGJxcVK+naYJoSGuOlawDRG6PqGBMH1GBmggYR1jBkHn79mKdiuWbHyRH4D 4xOFLkZODgkBE4mzT+cxQ9hiEhfurWfrYuTiEBJYxCgxec8qVghnPZPEqx0/2EGq2ARUJSau uM8GYosImEp0TFrKAmIzCyhLXOxfCjZJWMBW4nXfKUYQmwWovm/PX7AaXgEXiSX7PrJAbFOQ WHDvLdsERu4FjAyrGEVTC5ILipPSc430ihNzi0vz0vWS83M3MYJD9pn0DsZVDRaHGAU4GJV4 eBnWbA8QYk0sK67MPcQowcGsJMK7T3BHgBBvSmJlVWpRfnxRaU5q8SFGH6DtE5mlRJPzgfGU VxJvaGxiZmRpZGZsYm5sjENYSZy32SMlQEggPbEkNTs1tSC1CGYcEwenVAPjiu/n55nIlexz PrdgxesXs3IU8kU/Ncx5FlEmIxJ4VPF20tbD4k9cdp3bFOgxtfbtQ662S0+3ufEpOHhovGMp v/n4cPHWzYyZmwJSK/fFfBKfzVTR1FVuwvh85gn+VU+2/jyhcTt2+/x9L5OWMQaFSnB6r6kL 08wVl1skFMnpskaYO3XSxt/ZSizFGYmGWsxFxYkAB4CGi4YCAAA= X-CFilter-Loop: Reflected 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 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 | 41 ++++++++++++++++++++++++++++++++++++++--- include/drm/drmP.h | 12 ++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 24efae4..16e4b75 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,19 @@ 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 and not same process. + */ + if (obj->name) { + if (file_priv->pid != current->pid) + atomic_inc(&obj->obj_name_count); + + 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 +476,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 +518,11 @@ 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); + if (file_priv->pid != current->pid) + atomic_inc(&obj->obj_name_count); + } spin_unlock(&dev->object_name_lock); if (!obj) return -ENOENT; @@ -612,9 +631,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;