| Message ID | 20170222102108.23936-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 22, 2017 at 10:52:24AM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/selftests: don't leak the gem object > URL : https://patchwork.freedesktop.org/series/20050/ > State : success > > == Summary == > > Series 20050v1 drm/i915/selftests: don't leak the gem object > https://patchwork.freedesktop.org/api/1.0/series/20050/revisions/1/mbox/ Hmm, checked on pw, probably better to use i915_gem_object_put(). Calling free_object directly is a bad habit :) -Chris
On ke, 2017-02-22 at 10:21 +0000, Matthew Auld wrote: > For our fake dma objects we can leak the underlying gem object if we > fail to pin our "backing storage". > > [ 39.952618] ============================================================================= > [ 39.952625] BUG mock_object (Tainted: G U ): Objects remaining in mock_object on __kmem_cache_shutdown() > [ 39.952629] ----------------------------------------------------------------------------- > > [ 39.952633] Disabling lock debugging due to kernel taint > [ 39.952635] INFO: Slab 0xffffea00086c6a00 objects=21 used=1 fp=0xffff88021b1abc00 flags=0x5fff8000008100 > [ 39.952640] CPU: 1 PID: 1258 Comm: drv_selftest Tainted: G BU 4.10.0+ #46 > [ 39.952641] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS MBP111.88Z.0138.B17.1602221718 02/22/2016 > [ 39.952642] Call Trace: > [ 39.952648] dump_stack+0x4d/0x6f > [ 39.952651] slab_err+0x9d/0xb0 > [ 39.952654] ? ksm_migrate_page+0xe0/0xe0 > [ 39.952657] ? on_each_cpu_cond+0x9a/0xc0 > [ 39.952658] ? __kmalloc+0x1af/0x1c0 > [ 39.952660] ? __kmem_cache_shutdown+0x173/0x3e0 > [ 39.952661] __kmem_cache_shutdown+0x196/0x3e0 > [ 39.952664] kmem_cache_destroy+0xa0/0x150 > [ 39.952708] mock_device_release+0x113/0x140 [i915] > [ 39.952726] drm_dev_release+0x20/0x40 [drm] > [ 39.952735] drm_dev_unref+0x23/0x30 [drm] > [ 39.952768] i915_gem_gtt_mock_selftests+0x55/0x70 [i915] > [ 39.952803] __run_selftests+0x169/0x1c0 [i915] > [ 39.952805] ? 0xffffffffa0151000 > [ 39.952840] i915_mock_selftests+0x30/0x60 [i915] > [ 39.952869] i915_init+0xc/0x78 [i915] > [ 39.952870] ? 0xffffffffa0151000 > [ 39.952872] do_one_initcall+0x43/0x170 > [ 39.952874] ? __vunmap+0x81/0xd0 > [ 39.952875] ? kmem_cache_alloc_trace+0x37/0x170 > [ 39.952877] ? do_init_module+0x27/0x1f8 > [ 39.952879] do_init_module+0x5f/0x1f8 > [ 39.952881] load_module+0x2423/0x29b0 > [ 39.952882] ? __symbol_put+0x40/0x40 > [ 39.952885] ? kernel_read_file+0x1a3/0x1c0 > [ 39.952887] SYSC_finit_module+0xbc/0xf0 > [ 39.952889] SyS_finit_module+0xe/0x10 > [ 39.952892] entry_SYSCALL_64_fastpath+0x13/0x94 > > Fixes: 8d28ba4568f4 ("drm/i915: Exercise filling the top/bottom portions of the ppgtt") > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> <SNIP> > @@ -111,8 +111,10 @@ fake_dma_object(struct drm_i915_private *i915, u64 size) if (!obj) goto err; <SNIP> > obj->cache_level = I915_CACHE_NONE; > > /* Preallocate the "backing storage" */ > - if (i915_gem_object_pin_pages(obj)) goto err_obj; > + if (i915_gem_object_pin_pages(obj)) { > + i915_gem_object_free(obj); > return ERR_PTR(-ENOMEM); > + } > > i915_gem_object_unpin_pages(obj); > return obj; err_obj: i915_gem_object_free(obj); err: return ERR_PTR(-ENOMEM); With the proper onion teardown; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index e23753181720..5fc929ed6fcb 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -111,8 +111,10 @@ fake_dma_object(struct drm_i915_private *i915, u64 size) obj->cache_level = I915_CACHE_NONE; /* Preallocate the "backing storage" */ - if (i915_gem_object_pin_pages(obj)) + if (i915_gem_object_pin_pages(obj)) { + i915_gem_object_free(obj); return ERR_PTR(-ENOMEM); + } i915_gem_object_unpin_pages(obj); return obj;
For our fake dma objects we can leak the underlying gem object if we fail to pin our "backing storage". [ 39.952618] ============================================================================= [ 39.952625] BUG mock_object (Tainted: G U ): Objects remaining in mock_object on __kmem_cache_shutdown() [ 39.952629] ----------------------------------------------------------------------------- [ 39.952633] Disabling lock debugging due to kernel taint [ 39.952635] INFO: Slab 0xffffea00086c6a00 objects=21 used=1 fp=0xffff88021b1abc00 flags=0x5fff8000008100 [ 39.952640] CPU: 1 PID: 1258 Comm: drv_selftest Tainted: G BU 4.10.0+ #46 [ 39.952641] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS MBP111.88Z.0138.B17.1602221718 02/22/2016 [ 39.952642] Call Trace: [ 39.952648] dump_stack+0x4d/0x6f [ 39.952651] slab_err+0x9d/0xb0 [ 39.952654] ? ksm_migrate_page+0xe0/0xe0 [ 39.952657] ? on_each_cpu_cond+0x9a/0xc0 [ 39.952658] ? __kmalloc+0x1af/0x1c0 [ 39.952660] ? __kmem_cache_shutdown+0x173/0x3e0 [ 39.952661] __kmem_cache_shutdown+0x196/0x3e0 [ 39.952664] kmem_cache_destroy+0xa0/0x150 [ 39.952708] mock_device_release+0x113/0x140 [i915] [ 39.952726] drm_dev_release+0x20/0x40 [drm] [ 39.952735] drm_dev_unref+0x23/0x30 [drm] [ 39.952768] i915_gem_gtt_mock_selftests+0x55/0x70 [i915] [ 39.952803] __run_selftests+0x169/0x1c0 [i915] [ 39.952805] ? 0xffffffffa0151000 [ 39.952840] i915_mock_selftests+0x30/0x60 [i915] [ 39.952869] i915_init+0xc/0x78 [i915] [ 39.952870] ? 0xffffffffa0151000 [ 39.952872] do_one_initcall+0x43/0x170 [ 39.952874] ? __vunmap+0x81/0xd0 [ 39.952875] ? kmem_cache_alloc_trace+0x37/0x170 [ 39.952877] ? do_init_module+0x27/0x1f8 [ 39.952879] do_init_module+0x5f/0x1f8 [ 39.952881] load_module+0x2423/0x29b0 [ 39.952882] ? __symbol_put+0x40/0x40 [ 39.952885] ? kernel_read_file+0x1a3/0x1c0 [ 39.952887] SYSC_finit_module+0xbc/0xf0 [ 39.952889] SyS_finit_module+0xe/0x10 [ 39.952892] entry_SYSCALL_64_fastpath+0x13/0x94 Fixes: 8d28ba4568f4 ("drm/i915: Exercise filling the top/bottom portions of the ppgtt") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)