linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures
@ 2020-03-04 22:00 ` Lukasz Luba
  2020-03-05  7:07   ` Marek Szyprowski
  2020-03-09  0:45   ` Inki Dae
  0 siblings, 2 replies; 5+ messages in thread
From: Lukasz Luba @ 2020-03-04 22:00 UTC (permalink / raw)
  To: linux-kernel, dri-devel, linux-arm-kernel, linux-samsung-soc
  Cc: inki.dae, jy0922.shim, sw0312.kim, kyungmin.park, airlied,
	daniel, kgene, krzk, b.zolnierkie, a.hajda, Dietmar.Eggemann,
	lukasz.luba

There is a memory leak which left some objects not freed. The reference
counter of mapping: 'mapping->kref' was 2 when calling
arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
Since the old mapping structure is not going to be used any more (because
it is detached and new one attached), call arm_iommu_release_mapping()
to trigger cleanup.

Found using kmemleak detector, the output:

unreferenced object 0xc2137640 (size 64):
  comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
  hex dump (first 32 bytes):
    50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00  P........... ...
    00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
    [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
    [<ba07704b>] really_probe+0xb0/0x47c
    [<4f510e4f>] driver_probe_device+0x78/0x1c4
    [<7481a0cf>] device_driver_attach+0x58/0x60
    [<0ff8f5c1>] __driver_attach+0xb8/0x158
    [<86006144>] bus_for_each_dev+0x74/0xb4
    [<10159dca>] bus_add_driver+0x1c0/0x200
    [<8a265265>] driver_register+0x74/0x108
    [<e0f3451a>] exynos_drm_init+0xb0/0x134
    [<db3fc7ba>] do_one_initcall+0x90/0x458
    [<6da35917>] kernel_init_freeable+0x188/0x200
    [<db3f74d4>] kernel_init+0x8/0x110
    [<1f3cddf9>] ret_from_fork+0x14/0x20
    [<8cd12507>] 0x0
unreferenced object 0xc214a280 (size 128):
  comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
  hex dump (first 32 bytes):
    00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
    [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
    [<ba07704b>] really_probe+0xb0/0x47c
    [<4f510e4f>] driver_probe_device+0x78/0x1c4
    [<7481a0cf>] device_driver_attach+0x58/0x60
    [<0ff8f5c1>] __driver_attach+0xb8/0x158
    [<86006144>] bus_for_each_dev+0x74/0xb4
    [<10159dca>] bus_add_driver+0x1c0/0x200
    [<8a265265>] driver_register+0x74/0x108
    [<e0f3451a>] exynos_drm_init+0xb0/0x134
    [<db3fc7ba>] do_one_initcall+0x90/0x458
    [<6da35917>] kernel_init_freeable+0x188/0x200
    [<db3f74d4>] kernel_init+0x8/0x110
    [<1f3cddf9>] ret_from_fork+0x14/0x20
    [<8cd12507>] 0x0
unreferenced object 0xedeca000 (size 4096):
  comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
    [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
    [<ba07704b>] really_probe+0xb0/0x47c
    [<4f510e4f>] driver_probe_device+0x78/0x1c4
    [<7481a0cf>] device_driver_attach+0x58/0x60
    [<0ff8f5c1>] __driver_attach+0xb8/0x158
    [<86006144>] bus_for_each_dev+0x74/0xb4
    [<10159dca>] bus_add_driver+0x1c0/0x200
    [<8a265265>] driver_register+0x74/0x108
    [<e0f3451a>] exynos_drm_init+0xb0/0x134
    [<db3fc7ba>] do_one_initcall+0x90/0x458
    [<6da35917>] kernel_init_freeable+0x188/0x200
    [<db3f74d4>] kernel_init+0x8/0x110
    [<1f3cddf9>] ret_from_fork+0x14/0x20
    [<8cd12507>] 0x0
unreferenced object 0xc214a300 (size 128):
  comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
  hex dump (first 32 bytes):
    00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2  .........@......
    02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff  .....N..........
  backtrace:
    [<08cbd8bc>] iommu_domain_alloc+0x24/0x50
    [<b835abee>] arm_iommu_create_mapping+0xe4/0x134
    [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
    [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
    [<ba07704b>] really_probe+0xb0/0x47c
    [<4f510e4f>] driver_probe_device+0x78/0x1c4
    [<7481a0cf>] device_driver_attach+0x58/0x60
    [<0ff8f5c1>] __driver_attach+0xb8/0x158
    [<86006144>] bus_for_each_dev+0x74/0xb4
    [<10159dca>] bus_add_driver+0x1c0/0x200
    [<8a265265>] driver_register+0x74/0x108
    [<e0f3451a>] exynos_drm_init+0xb0/0x134
    [<db3fc7ba>] do_one_initcall+0x90/0x458
    [<6da35917>] kernel_init_freeable+0x188/0x200
    [<db3f74d4>] kernel_init+0x8/0x110
    [<1f3cddf9>] ret_from_fork+0x14/0x20

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---

Hi all,

I have discovered this issue on OdroidXU4 while running some stress tests
for upcoming Energy Model. To reproduce it, kernel must be compiled with
DEBUG_KMEMLEAK. When the boot has finished, type:
# echo scan > /sys/kernel/debug/kmemleak
# cat /sys/kernel/debug/kmemleak
You should expect similar output to the one from the commit message.

I don't know if it should go via stable tree as well. I can resend with CC
stable, if there is a need.

Regards,
Lukasz Luba

 drivers/gpu/drm/exynos/exynos_drm_dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
index 9ebc02768847..45f209ec107f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
@@ -74,8 +74,13 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
 		return ret;
 
 	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
-		if (to_dma_iommu_mapping(subdrv_dev))
+		struct dma_iommu_mapping *mapping =
+					to_dma_iommu_mapping(subdrv_dev);
+
+		if (mapping) {
 			arm_iommu_detach_device(subdrv_dev);
+			arm_iommu_release_mapping(mapping);
+		}
 
 		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
 	} else if (IS_ENABLED(CONFIG_IOMMU_DMA)) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures
  2020-03-04 22:00 ` [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures Lukasz Luba
@ 2020-03-05  7:07   ` Marek Szyprowski
  2020-03-05 10:05     ` Lukasz Luba
  2020-03-09  0:45   ` Inki Dae
  1 sibling, 1 reply; 5+ messages in thread
From: Marek Szyprowski @ 2020-03-05  7:07 UTC (permalink / raw)
  To: Lukasz Luba, linux-kernel, dri-devel, linux-arm-kernel,
	linux-samsung-soc
  Cc: inki.dae, jy0922.shim, sw0312.kim, kyungmin.park, airlied,
	daniel, kgene, krzk, b.zolnierkie, a.hajda, Dietmar.Eggemann

Hi Lukasz,

On 04.03.2020 23:00, Lukasz Luba wrote:
> There is a memory leak which left some objects not freed. The reference
> counter of mapping: 'mapping->kref' was 2 when calling
> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
> Since the old mapping structure is not going to be used any more (because
> it is detached and new one attached), call arm_iommu_release_mapping()
> to trigger cleanup.

This will break IOMMU support in Exynos DRM if deferred probe happens. 
Here is a proper fix:

https://patchwork.kernel.org/patch/11415715/

The mapping initially created by DMA-mapping framework should be 
attached back when Exynos DRM releases the subdev device.

> Found using kmemleak detector, the output:
>
> unreferenced object 0xc2137640 (size 64):
>    comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>    hex dump (first 32 bytes):
>      50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00  P........... ...
>      00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>      [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>      [<ba07704b>] really_probe+0xb0/0x47c
>      [<4f510e4f>] driver_probe_device+0x78/0x1c4
>      [<7481a0cf>] device_driver_attach+0x58/0x60
>      [<0ff8f5c1>] __driver_attach+0xb8/0x158
>      [<86006144>] bus_for_each_dev+0x74/0xb4
>      [<10159dca>] bus_add_driver+0x1c0/0x200
>      [<8a265265>] driver_register+0x74/0x108
>      [<e0f3451a>] exynos_drm_init+0xb0/0x134
>      [<db3fc7ba>] do_one_initcall+0x90/0x458
>      [<6da35917>] kernel_init_freeable+0x188/0x200
>      [<db3f74d4>] kernel_init+0x8/0x110
>      [<1f3cddf9>] ret_from_fork+0x14/0x20
>      [<8cd12507>] 0x0
> unreferenced object 0xc214a280 (size 128):
>    comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>    hex dump (first 32 bytes):
>      00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>      [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>      [<ba07704b>] really_probe+0xb0/0x47c
>      [<4f510e4f>] driver_probe_device+0x78/0x1c4
>      [<7481a0cf>] device_driver_attach+0x58/0x60
>      [<0ff8f5c1>] __driver_attach+0xb8/0x158
>      [<86006144>] bus_for_each_dev+0x74/0xb4
>      [<10159dca>] bus_add_driver+0x1c0/0x200
>      [<8a265265>] driver_register+0x74/0x108
>      [<e0f3451a>] exynos_drm_init+0xb0/0x134
>      [<db3fc7ba>] do_one_initcall+0x90/0x458
>      [<6da35917>] kernel_init_freeable+0x188/0x200
>      [<db3f74d4>] kernel_init+0x8/0x110
>      [<1f3cddf9>] ret_from_fork+0x14/0x20
>      [<8cd12507>] 0x0
> unreferenced object 0xedeca000 (size 4096):
>    comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>      [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>      [<ba07704b>] really_probe+0xb0/0x47c
>      [<4f510e4f>] driver_probe_device+0x78/0x1c4
>      [<7481a0cf>] device_driver_attach+0x58/0x60
>      [<0ff8f5c1>] __driver_attach+0xb8/0x158
>      [<86006144>] bus_for_each_dev+0x74/0xb4
>      [<10159dca>] bus_add_driver+0x1c0/0x200
>      [<8a265265>] driver_register+0x74/0x108
>      [<e0f3451a>] exynos_drm_init+0xb0/0x134
>      [<db3fc7ba>] do_one_initcall+0x90/0x458
>      [<6da35917>] kernel_init_freeable+0x188/0x200
>      [<db3f74d4>] kernel_init+0x8/0x110
>      [<1f3cddf9>] ret_from_fork+0x14/0x20
>      [<8cd12507>] 0x0
> unreferenced object 0xc214a300 (size 128):
>    comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>    hex dump (first 32 bytes):
>      00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2  .........@......
>      02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff  .....N..........
>    backtrace:
>      [<08cbd8bc>] iommu_domain_alloc+0x24/0x50
>      [<b835abee>] arm_iommu_create_mapping+0xe4/0x134
>      [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>      [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>      [<ba07704b>] really_probe+0xb0/0x47c
>      [<4f510e4f>] driver_probe_device+0x78/0x1c4
>      [<7481a0cf>] device_driver_attach+0x58/0x60
>      [<0ff8f5c1>] __driver_attach+0xb8/0x158
>      [<86006144>] bus_for_each_dev+0x74/0xb4
>      [<10159dca>] bus_add_driver+0x1c0/0x200
>      [<8a265265>] driver_register+0x74/0x108
>      [<e0f3451a>] exynos_drm_init+0xb0/0x134
>      [<db3fc7ba>] do_one_initcall+0x90/0x458
>      [<6da35917>] kernel_init_freeable+0x188/0x200
>      [<db3f74d4>] kernel_init+0x8/0x110
>      [<1f3cddf9>] ret_from_fork+0x14/0x20
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>
> Hi all,
>
> I have discovered this issue on OdroidXU4 while running some stress tests
> for upcoming Energy Model. To reproduce it, kernel must be compiled with
> DEBUG_KMEMLEAK. When the boot has finished, type:
> # echo scan > /sys/kernel/debug/kmemleak
> # cat /sys/kernel/debug/kmemleak
> You should expect similar output to the one from the commit message.
>
> I don't know if it should go via stable tree as well. I can resend with CC
> stable, if there is a need.
>
> Regards,
> Lukasz Luba
>
>   drivers/gpu/drm/exynos/exynos_drm_dma.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..45f209ec107f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -74,8 +74,13 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>   		return ret;
>   
>   	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> -		if (to_dma_iommu_mapping(subdrv_dev))
> +		struct dma_iommu_mapping *mapping =
> +					to_dma_iommu_mapping(subdrv_dev);
> +
> +		if (mapping) {
>   			arm_iommu_detach_device(subdrv_dev);
> +			arm_iommu_release_mapping(mapping);
> +		}
>   
>   		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>   	} else if (IS_ENABLED(CONFIG_IOMMU_DMA)) {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures
  2020-03-05  7:07   ` Marek Szyprowski
@ 2020-03-05 10:05     ` Lukasz Luba
  0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Luba @ 2020-03-05 10:05 UTC (permalink / raw)
  To: Marek Szyprowski, linux-kernel, dri-devel, linux-arm-kernel,
	linux-samsung-soc
  Cc: inki.dae, jy0922.shim, sw0312.kim, kyungmin.park, airlied,
	daniel, kgene, krzk, b.zolnierkie, a.hajda, Dietmar.Eggemann

Hi Marek,

On 3/5/20 7:07 AM, Marek Szyprowski wrote:
> Hi Lukasz,
> 
> On 04.03.2020 23:00, Lukasz Luba wrote:
>> There is a memory leak which left some objects not freed. The reference
>> counter of mapping: 'mapping->kref' was 2 when calling
>> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
>> Since the old mapping structure is not going to be used any more (because
>> it is detached and new one attached), call arm_iommu_release_mapping()
>> to trigger cleanup.
> 
> This will break IOMMU support in Exynos DRM if deferred probe happens.
> Here is a proper fix:

I forgot about the deferred probe.

> 
> https://patchwork.kernel.org/patch/11415715/
> 
> The mapping initially created by DMA-mapping framework should be
> attached back when Exynos DRM releases the subdev device.
> 

Indeed, as you responded in that thread with the example, there is
more dependencies and attaching back the old mapping will work.

I am going add my reviewed-by to your patch.

Regards,
Lukasz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures
  2020-03-04 22:00 ` [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures Lukasz Luba
  2020-03-05  7:07   ` Marek Szyprowski
@ 2020-03-09  0:45   ` Inki Dae
  2020-03-09  8:41     ` Lukasz Luba
  1 sibling, 1 reply; 5+ messages in thread
From: Inki Dae @ 2020-03-09  0:45 UTC (permalink / raw)
  To: Lukasz Luba, linux-kernel, dri-devel, linux-arm-kernel,
	linux-samsung-soc
  Cc: jy0922.shim, sw0312.kim, kyungmin.park, airlied, daniel, kgene,
	krzk, b.zolnierkie, a.hajda, Dietmar.Eggemann

Hi Lukasz,

20. 3. 5. 오전 7:00에 Lukasz Luba 이(가) 쓴 글:
> There is a memory leak which left some objects not freed. The reference
> counter of mapping: 'mapping->kref' was 2 when calling
> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
> Since the old mapping structure is not going to be used any more (because
> it is detached and new one attached), call arm_iommu_release_mapping()
> to trigger cleanup.
> 
> Found using kmemleak detector, the output:
> 
> unreferenced object 0xc2137640 (size 64):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00  P........... ...
>     00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xc214a280 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xedeca000 (size 4096):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xc214a300 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2  .........@......
>     02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff  .....N..........
>   backtrace:
>     [<08cbd8bc>] iommu_domain_alloc+0x24/0x50
>     [<b835abee>] arm_iommu_create_mapping+0xe4/0x134
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
> 
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
> 
> Hi all,
> 
> I have discovered this issue on OdroidXU4 while running some stress tests
> for upcoming Energy Model. To reproduce it, kernel must be compiled with
> DEBUG_KMEMLEAK. When the boot has finished, type:
> # echo scan > /sys/kernel/debug/kmemleak
> # cat /sys/kernel/debug/kmemleak
> You should expect similar output to the one from the commit message.
> 
> I don't know if it should go via stable tree as well. I can resend with CC
> stable, if there is a need.

Thanks for fixup. BTW, as you commented on Marek's patch thread, with Marek's patch the memory leak will be solved.
Do you want Marek to rework his patch on top of your patch or are you ok me to pick up only Marek's one?

Marek's patch is conflicted with your one.

Thanks,
Inki Dae

> 
> Regards,
> Lukasz Luba
> 
>  drivers/gpu/drm/exynos/exynos_drm_dma.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..45f209ec107f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -74,8 +74,13 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>  		return ret;
>  
>  	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> -		if (to_dma_iommu_mapping(subdrv_dev))
> +		struct dma_iommu_mapping *mapping =
> +					to_dma_iommu_mapping(subdrv_dev);
> +
> +		if (mapping) {
>  			arm_iommu_detach_device(subdrv_dev);
> +			arm_iommu_release_mapping(mapping);
> +		}
>  
>  		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>  	} else if (IS_ENABLED(CONFIG_IOMMU_DMA)) {
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures
  2020-03-09  0:45   ` Inki Dae
@ 2020-03-09  8:41     ` Lukasz Luba
  0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Luba @ 2020-03-09  8:41 UTC (permalink / raw)
  To: Inki Dae, linux-kernel, dri-devel, linux-arm-kernel, linux-samsung-soc
  Cc: jy0922.shim, sw0312.kim, kyungmin.park, airlied, daniel, kgene,
	krzk, b.zolnierkie, a.hajda, Dietmar.Eggemann

Hi Inki,

On 3/9/20 12:45 AM, Inki Dae wrote:
> Hi Lukasz,
> 
> 20. 3. 5. 오전 7:00에 Lukasz Luba 이(가) 쓴 글:
>> There is a memory leak which left some objects not freed. The reference
>> counter of mapping: 'mapping->kref' was 2 when calling
>> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
>> Since the old mapping structure is not going to be used any more (because
>> it is detached and new one attached), call arm_iommu_release_mapping()
>> to trigger cleanup.
>>
>> Found using kmemleak detector, the output:
>>

[snip]

>>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>
>> Hi all,
>>
>> I have discovered this issue on OdroidXU4 while running some stress tests
>> for upcoming Energy Model. To reproduce it, kernel must be compiled with
>> DEBUG_KMEMLEAK. When the boot has finished, type:
>> # echo scan > /sys/kernel/debug/kmemleak
>> # cat /sys/kernel/debug/kmemleak
>> You should expect similar output to the one from the commit message.
>>
>> I don't know if it should go via stable tree as well. I can resend with CC
>> stable, if there is a need.
> 
> Thanks for fixup. BTW, as you commented on Marek's patch thread, with Marek's patch the memory leak will be solved.
> Do you want Marek to rework his patch on top of your patch or are you ok me to pick up only Marek's one?

Please drop this one and apply only Marek's patch, it fixes the issue.
I didn't know that he was working on similar stuff.

> 
> Marek's patch is conflicted with your one.
> 
> Thanks,
> Inki Dae

Regards,
Lukasz

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-03-09  8:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200304220106eucas1p232aae5af79945664c4586930a9412eda@eucas1p2.samsung.com>
2020-03-04 22:00 ` [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping structures Lukasz Luba
2020-03-05  7:07   ` Marek Szyprowski
2020-03-05 10:05     ` Lukasz Luba
2020-03-09  0:45   ` Inki Dae
2020-03-09  8:41     ` Lukasz Luba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).