From 4e731f60f5893960c50905e7fac5699cb8deadaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=8B=87?= Date: Tue, 16 Oct 2018 01:54:29 +0800 Subject: [PATCH] =?UTF-8?q?1.=20=E4=BF=AE=E5=A4=8Damdgpu=5Fbo=E6=B3=84?= =?UTF-8?q?=E9=9C=B2=E7=9A=84=E9=97=AE=E9=A2=98=20=20=20=20=20amdgpu=5Fbo?= =?UTF-8?q?=E7=94=B1libdrm=E7=AE=A1=E7=90=86=EF=BC=8C=E4=BD=BF=E7=94=A8?= =?UTF-8?q?=E5=BC=95=E7=94=A8=E8=AE=A1=E6=95=B0=E3=80=82=20=20=20=20=20amd?= =?UTF-8?q?gpu=5Fwinsys=5Fbo=E7=94=B1mesa=E7=AE=A1=E7=90=86=EF=BC=8C?= =?UTF-8?q?=E5=90=8C=E6=A0=B7=E4=BD=BF=E7=94=A8=E5=BC=95=E7=94=A8=E8=AE=A1?= =?UTF-8?q?=E6=95=B0=E3=80=82=E5=BD=93=E5=B0=9D=E8=AF=95=E5=B0=86fd?= =?UTF-8?q?=E5=AF=BC=E5=85=A5mesa=E5=BE=97=E5=88=B0pipe=5Fresource?= =?UTF-8?q?=E6=97=B6=EF=BC=8C=E4=BC=9A=E8=B0=83=E7=94=A8amdgpu=5Fbo=5Ffrom?= =?UTF-8?q?=5Fhandle,=E6=AD=A4=E5=87=BD=E6=95=B0=E4=BC=9A=E8=B0=83?= =?UTF-8?q?=E7=94=A8amdgpu=5Fbo=5Fimport=E5=AE=9E=E7=8E=B0=E5=B0=86fd?= =?UTF-8?q?=E8=BD=AC=E5=8C=96=E4=B8=BAamdgpu=5Fbo=E3=80=82=20=20=20=20=20b?= =?UTF-8?q?ug=E5=9C=A8=E4=BA=8E=E6=AF=8F=E6=AC=A1=E5=AF=BC=E5=85=A5fd?= =?UTF-8?q?=E7=9A=84=E6=97=B6=E5=80=99=E9=83=BD=E4=BC=9A=E8=B0=83=E7=94=A8?= =?UTF-8?q?amdgpu=5Fbo=5Ffrom=5Fhandle=E5=92=8Camdgpu=5Fbo=5Fimport?= =?UTF-8?q?=EF=BC=8C=E5=A6=82=E6=9E=9C=E5=90=8C=E4=B8=80=E4=B8=AAfd?= =?UTF-8?q?=E8=B0=83=E7=94=A8=E5=A4=9A=E6=AC=A1=EF=BC=8C=E5=B0=B1=E4=BC=9A?= =?UTF-8?q?=E5=AF=BC=E8=87=B4amdgpu=5Fbo=5Fimport=E8=A2=AB=E8=B0=83?= =?UTF-8?q?=E7=94=A8=E4=BA=86=E5=A4=9A=E6=AC=A1=E3=80=82=E7=94=B1=E4=BA=8E?= =?UTF-8?q?amdgpu=5Fbo=E5=9C=A8=E5=86=85=E9=83=A8=E7=BB=B4=E6=8A=A4?= =?UTF-8?q?=E4=BA=86=E5=BC=95=E7=94=A8=E8=AE=A1=E6=95=B0=EF=BC=8C=20=20=20?= =?UTF-8?q?=20=20=E5=BF=85=E9=A1=BB=E8=B0=83=E7=94=A8=E5=90=8C=E7=AD=89?= =?UTF-8?q?=E6=AC=A1=E6=95=B0=E7=9A=84amdgpu=5Fbo=5Ffree=E6=89=8D=E5=8F=AF?= =?UTF-8?q?=E4=BB=A5=E7=9C=9F=E6=AD=A3=E9=87=8A=E6=94=BE=E3=80=82=E8=80=8C?= =?UTF-8?q?=E7=94=B1=E4=BA=8Eamdgpu=5Fwinsys=5Fbo=E4=B9=9F=E9=87=87?= =?UTF-8?q?=E7=94=A8=E4=BA=86=E5=BC=95=E7=94=A8=E8=AE=A1=E6=95=B0=EF=BC=8C?= =?UTF-8?q?=E5=AF=B9=E4=BA=8E=E5=90=8C=E4=B8=80=E4=B8=AAfd=EF=BC=8C?= =?UTF-8?q?=E4=B8=8D=E8=AE=BA=E8=B0=83=E7=94=A8=E5=A4=9A=E5=B0=91=E6=AC=A1?= =?UTF-8?q?amdgpu=5Fbo=5Ffrom=5Fhandle,=20=E6=9C=80=E5=90=8E=E9=83=BD?= =?UTF-8?q?=E5=8F=AA=E4=BC=9A=E8=B0=83=E7=94=A8=E4=B8=80=E6=AC=A1amdgpu=5F?= =?UTF-8?q?bo=5Ffree=EF=BC=8C=20=20=20=20=20=E8=BF=99=E5=B0=B1=E5=AF=BC?= =?UTF-8?q?=E8=87=B4=E4=BA=86=E5=BC=95=E7=94=A8=E8=AE=A1=E6=95=B0=E6=B3=84?= =?UTF-8?q?=E9=9C=B2=E3=80=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复的方法是改动amdgpu_bo_from_handle，如果fd对应的amdgpu_bo是已知的amdgpu_bo，则调用一次amdgpu_bo_free，减去多余的引用，解决引用计数的问题。 2. 修复st_drawable在android平台上不能正确释放导致泄露的问题 根据egl规范，试图删除一个正在被绑定的eglsurface不会立刻导致此surface被销毁，而是会延迟直到此surface不再被使用的时候被销毁。 因此st_framebuffer会包含一个指针，指向st_framebuffer_iface。st_manager中维护了一个当前活跃的framebuffer的hashtable，key和value都是st_framebuffer_iface的内存地址， 销毁framebuffer时会首先从hashtable删除st_framebuffer_iface，真正销毁st_framebuffer则是发生在后续调用make_current时。 make_current会调用st_framebuffers_purge，此函数会遍历winsys_buffers，检查每一个st_framebuffer的st_framebuffer_iface是否存在于hashtable中，如果不存在，则释放此framebuffer。 bug在于此hashtable的key和value是st_framebuffer_iface，作者假设不同的st_framebuffer_iface不会被分配到相同的内存地址，因此可以安全的作为key。 但android系统的malloc实现并非如此，假设首先删除一个st_framebuffer_iface，随后再次malloc一个新的st_framebuffer_iface对象，在android平台上，新的对象很可能与老的对象拥有相同的地址， 这就导致在后续make_current调用st_framebuffers_purge时不会正确释放本应被释放的st_framebuffer，最终出现了内存泄露。 有两种修复方式 1. 在向hash_table添加iface时，检查是否有相同的key存在，如果有的话，释放对应的st_framebuffer 2. 使用st_framebuffer_iface->ID作为hash key，根据文档，ID是全局唯一的。 --- src/egl/Android.mk | 7 ++----- src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 1 + src/mesa/state_tracker/st_manager.c | 22 +++++++++------------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 84455d0..5b440c3 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -45,9 +45,7 @@ LOCAL_CFLAGS := \ LOCAL_C_INCLUDES := \ $(MESA_TOP)/include/drm-uapi \ $(MESA_TOP)/src/egl/main \ - $(MESA_TOP)/src/egl/drivers/dri2 \ - $(MESA_TOP)/src/gbm/main \ - $(MESA_TOP)/src/gbm/backends/dri + $(MESA_TOP)/src/egl/drivers/dri2 LOCAL_STATIC_LIBRARIES := \ libmesa_util \ @@ -59,8 +57,7 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libsync \ - libgbm + libsync ifeq ($(BOARD_USES_DRM_GRALLOC),true) LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c index 80563d3..076e7eb 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c @@ -1306,6 +1306,7 @@ static struct pb_buffer *amdgpu_bo_from_handle(struct radeon_winsys *rws, * counter and return it. */ if (bo) { + amdgpu_bo_free(result.buf_handle); p_atomic_inc(&bo->base.reference.count); simple_mtx_unlock(&ws->bo_export_table_lock); return &bo->base; diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c index 69286b5..23bc348 100644 --- a/src/mesa/state_tracker/st_manager.c +++ b/src/mesa/state_tracker/st_manager.c @@ -523,13 +523,12 @@ st_framebuffer_iface_hash(const void *key) static bool st_framebuffer_iface_equal(const void *a, const void *b) { - return (struct st_framebuffer_iface *)a == (struct st_framebuffer_iface *)b; + return a == b; } static boolean -st_framebuffer_iface_lookup(struct st_manager *smapi, - const struct st_framebuffer_iface *stfbi) +st_framebuffer_iface_lookup(struct st_manager *smapi, uint32_t ID) { struct st_manager_private *smPriv = (struct st_manager_private *)smapi->st_manager_private; @@ -539,7 +538,7 @@ st_framebuffer_iface_lookup(struct st_manager *smapi, assert(smPriv->stfbi_ht); mtx_lock(&smPriv->st_mutex); - entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi); + entry = _mesa_hash_table_search(smPriv->stfbi_ht, (const void *)ID); mtx_unlock(&smPriv->st_mutex); return entry != NULL; @@ -558,7 +557,7 @@ st_framebuffer_iface_insert(struct st_manager *smapi, assert(smPriv->stfbi_ht); mtx_lock(&smPriv->st_mutex); - entry = _mesa_hash_table_insert(smPriv->stfbi_ht, stfbi, stfbi); + entry = _mesa_hash_table_insert(smPriv->stfbi_ht, (const void *)stfbi->ID, stfbi); mtx_unlock(&smPriv->st_mutex); return entry != NULL; @@ -566,8 +565,7 @@ st_framebuffer_iface_insert(struct st_manager *smapi, static void -st_framebuffer_iface_remove(struct st_manager *smapi, - struct st_framebuffer_iface *stfbi) +st_framebuffer_iface_remove(struct st_manager *smapi, uint32_t ID) { struct st_manager_private *smPriv = (struct st_manager_private *)smapi->st_manager_private; @@ -577,7 +575,7 @@ st_framebuffer_iface_remove(struct st_manager *smapi, return; mtx_lock(&smPriv->st_mutex); - entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi); + entry = _mesa_hash_table_search(smPriv->stfbi_ht, (const void *)ID); if (!entry) goto unlock; @@ -599,7 +597,7 @@ st_api_destroy_drawable(struct st_api *stapi, if (!stfbi) return; - st_framebuffer_iface_remove(stfbi->state_manager, stfbi); + st_framebuffer_iface_remove(stfbi->state_manager, stfbi->ID); } @@ -617,9 +615,7 @@ st_framebuffers_purge(struct st_context *st) assert(smapi); LIST_FOR_EACH_ENTRY_SAFE_REV(stfb, next, &st->winsys_buffers, head) { - struct st_framebuffer_iface *stfbi = stfb->iface; - - assert(stfbi); + uint32_t ID = stfb->iface_ID; /** * If the corresponding framebuffer interface object no longer exists, @@ -627,7 +623,7 @@ st_framebuffers_purge(struct st_context *st) * and unreference the framebuffer object, so its resources can be * deleted. */ - if (!st_framebuffer_iface_lookup(smapi, stfbi)) { + if (!st_framebuffer_iface_lookup(smapi, ID)) { LIST_DEL(&stfb->head); st_framebuffer_reference(&stfb, NULL); } -- 2.7.4