| Message ID | 20220325010328.32963-1-chuansheng.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: fix one mem leak in mmap_offset_attach() | expand |
On 25/03/2022 01:03, Chuansheng Liu wrote: > The below memory leak information is caught: > > unreferenced object 0xffff997dd4e3b240 (size 64): > comm "gem_tiled_fence", pid 10332, jiffies 4294959326 (age > 220778.420s) > hex dump (first 32 bytes): > 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 00 be f2 d4 7d 99 ff ff ............}... > backtrace: > [<ffffffffa0f04365>] kmem_cache_alloc_trace+0x2e5/0x450 > [<ffffffffc062f3ac>] drm_vma_node_allow+0x2c/0xe0 [drm] > [<ffffffffc13149ea>] __assign_mmap_offset_handle+0x1da/0x4a0 [i915] > [<ffffffffc1315235>] i915_gem_mmap_offset_ioctl+0x55/0xb0 [i915] > [<ffffffffc06207e4>] drm_ioctl_kernel+0xb4/0x140 [drm] > [<ffffffffc0620ac7>] drm_ioctl+0x257/0x410 [drm] > [<ffffffffa0f553ae>] __x64_sys_ioctl+0x8e/0xc0 > [<ffffffffa1821128>] do_syscall_64+0x38/0xc0 > [<ffffffffa1a0007c>] entry_SYSCALL_64_after_hwframe+0x44/0xae > > The issue is always reproduced with the test: > gem_tiled_fence_blits --run-subtest basic > > It tries to mmap_gtt the same object several times, it is like: > create BO > mmap_gtt BO > unmap BO > mmap_gtt BO <== second time mmap_gtt > unmap > close BO > > The leak happens at the second time mmap_gtt in function > mmap_offset_attach(),it will simply increase the reference > count to 2 by calling drm_vma_node_allow() directly since > the mmo has been created at the first time. > > However the driver just revokes the vma_node only one time > when closing the object, it leads to memory leak easily. > > This patch is to fix the memory leak by calling drm_vma_node_allow() one > time also. > > V2: add "Fixes and Cc stable". (Tvrtko Ursulin) > > Fixes: 786555987207 ("drm/i915/gem: Store mmap_offsets in an rbtree > rather than a plain list") > Cc: <stable@vger.kernel.org> # v5.7+ > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index c3ea243d414d..fda346d687fd 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -680,7 +680,7 @@ mmap_offset_attach(struct drm_i915_gem_object *obj, > mmo = insert_mmo(obj, mmo); > GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo); > out: > - if (file) > + if (file && !drm_vma_node_is_allowed(&mmo->vma_node, file)) > drm_vma_node_allow(&mmo->vma_node, file); > return mmo; > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index c3ea243d414d..fda346d687fd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -680,7 +680,7 @@ mmap_offset_attach(struct drm_i915_gem_object *obj, mmo = insert_mmo(obj, mmo); GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo); out: - if (file) + if (file && !drm_vma_node_is_allowed(&mmo->vma_node, file)) drm_vma_node_allow(&mmo->vma_node, file); return mmo;
The below memory leak information is caught: unreferenced object 0xffff997dd4e3b240 (size 64): comm "gem_tiled_fence", pid 10332, jiffies 4294959326 (age 220778.420s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 be f2 d4 7d 99 ff ff ............}... backtrace: [<ffffffffa0f04365>] kmem_cache_alloc_trace+0x2e5/0x450 [<ffffffffc062f3ac>] drm_vma_node_allow+0x2c/0xe0 [drm] [<ffffffffc13149ea>] __assign_mmap_offset_handle+0x1da/0x4a0 [i915] [<ffffffffc1315235>] i915_gem_mmap_offset_ioctl+0x55/0xb0 [i915] [<ffffffffc06207e4>] drm_ioctl_kernel+0xb4/0x140 [drm] [<ffffffffc0620ac7>] drm_ioctl+0x257/0x410 [drm] [<ffffffffa0f553ae>] __x64_sys_ioctl+0x8e/0xc0 [<ffffffffa1821128>] do_syscall_64+0x38/0xc0 [<ffffffffa1a0007c>] entry_SYSCALL_64_after_hwframe+0x44/0xae The issue is always reproduced with the test: gem_tiled_fence_blits --run-subtest basic It tries to mmap_gtt the same object several times, it is like: create BO mmap_gtt BO unmap BO mmap_gtt BO <== second time mmap_gtt unmap close BO The leak happens at the second time mmap_gtt in function mmap_offset_attach(),it will simply increase the reference count to 2 by calling drm_vma_node_allow() directly since the mmo has been created at the first time. However the driver just revokes the vma_node only one time when closing the object, it leads to memory leak easily. This patch is to fix the memory leak by calling drm_vma_node_allow() one time also. V2: add "Fixes and Cc stable". (Tvrtko Ursulin) Fixes: 786555987207 ("drm/i915/gem: Store mmap_offsets in an rbtree rather than a plain list") Cc: <stable@vger.kernel.org> # v5.7+ Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)