Re: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Shuah Khan]

On 06/28/2016 09:55 AM, Shuah Khan wrote:
> On 06/28/2016 07:39 AM, Shuah Khan wrote:
>> On 06/28/2016 03:57 AM, Kamil Debski wrote:
>>> HI Shuah,
>>>
>>> Which branch do you base your patches on?
>>>
>>> I have trouble applying this path
>>> (https://patchwork.linuxtv.org/patch/34577/) and 
>>> "s5p-mfc fix null pointer deference in clk_core_enable()"
>>> (https://patchwork.linuxtv.org/patch/34751/) 
>>> onto current linuxtv/master.
>>>
>>> The top commit of linuxtv/master is :
>>> "commit 0db5c79989de2c68d5abb7ba891bfdb3cd3b7e05
>>> Author: Mauro Carvalho Chehab <mche...@s-opensource.com>
>>> Date:   Thu Jun 16 08:04:40 2016 -0300
>>>
>>> [media] media-devnode.h: Fix documentation"
>>>
>>> Could you please rebase the two patches mentioned above to the
>>> linuxtv/master?
>>>
>>> Best wishes,
>>>
>>
>> I based them on linux_next. I will rebase both patches on linuxtv/master
>> and resend.
>>
> 
> Hi Kamil,
> 
> Do you need me to rebase this one:
> 
> https://patchwork.kernel.org/patch/9166119/
> 
> I will do that anyway and send three patches. I don't see the above
> in linuxtv/master
> 

Hi Kamil,

https://patchwork.kernel.org/patch/9166119/
media: s5p-mfc fix video device release double release in probe error path

and 

(https://patchwork.linuxtv.org/patch/34577/)
media: s5p-mfc fix memory leak in s5p_mfc_remove()

applied as is on top of linuxtv/master latest with top
commit

The following failed and applied with fuzz.

"s5p-mfc fix null pointer deference in clk_core_enable()"
(https://patchwork.linuxtv.org/patch/34751/) 
onto current linuxtv/master.

I generated a rebasewd patch series with the above 3 patches
and sent it.

https://www.mail-archive.com/linux-media@vger.kernel.org/msg98784.html

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Shuah Khan]

On 06/28/2016 07:39 AM, Shuah Khan wrote:
> On 06/28/2016 03:57 AM, Kamil Debski wrote:
>> HI Shuah,
>>
>> Which branch do you base your patches on?
>>
>> I have trouble applying this path
>> (https://patchwork.linuxtv.org/patch/34577/) and 
>> "s5p-mfc fix null pointer deference in clk_core_enable()"
>> (https://patchwork.linuxtv.org/patch/34751/) 
>> onto current linuxtv/master.
>>
>> The top commit of linuxtv/master is :
>> "commit 0db5c79989de2c68d5abb7ba891bfdb3cd3b7e05
>> Author: Mauro Carvalho Chehab 
>> Date:   Thu Jun 16 08:04:40 2016 -0300
>>
>> [media] media-devnode.h: Fix documentation"
>>
>> Could you please rebase the two patches mentioned above to the
>> linuxtv/master?
>>
>> Best wishes,
>>
> 
> I based them on linux_next. I will rebase both patches on linuxtv/master
> and resend.
> 

Hi Kamil,

Do you need me to rebase this one:

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

I will do that anyway and send three patches. I don't see the above
in linuxtv/master

thanks,
-- Shuah

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Shuah Khan]

On 06/28/2016 03:57 AM, Kamil Debski wrote:
> HI Shuah,
> 
> Which branch do you base your patches on?
> 
> I have trouble applying this path
> (https://patchwork.linuxtv.org/patch/34577/) and 
> "s5p-mfc fix null pointer deference in clk_core_enable()"
> (https://patchwork.linuxtv.org/patch/34751/) 
> onto current linuxtv/master.
> 
> The top commit of linuxtv/master is :
> "commit 0db5c79989de2c68d5abb7ba891bfdb3cd3b7e05
> Author: Mauro Carvalho Chehab 
> Date:   Thu Jun 16 08:04:40 2016 -0300
> 
> [media] media-devnode.h: Fix documentation"
> 
> Could you please rebase the two patches mentioned above to the
> linuxtv/master?
> 
> Best wishes,
> 

I based them on linux_next. I will rebase both patches on linuxtv/master
and resend.

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Kamil Debski]

HI Shuah,

Which branch do you base your patches on?

I have trouble applying this path
(https://patchwork.linuxtv.org/patch/34577/) and 
"s5p-mfc fix null pointer deference in clk_core_enable()"
(https://patchwork.linuxtv.org/patch/34751/) 
onto current linuxtv/master.

The top commit of linuxtv/master is :
"commit 0db5c79989de2c68d5abb7ba891bfdb3cd3b7e05
Author: Mauro Carvalho Chehab <mche...@s-opensource.com>
Date:   Thu Jun 16 08:04:40 2016 -0300

[media] media-devnode.h: Fix documentation"

Could you please rebase the two patches mentioned above to the
linuxtv/master?

Best wishes,
-- 
Kamil Debski
Samsung R Institute Poland


> -Original Message-
> From: Shuah Khan [mailto:shua...@osg.samsung.com]
> Sent: Monday, June 13, 2016 9:45 PM
> To: kyungmin.p...@samsung.com; k.deb...@samsung.com;
> jtp.p...@samsung.com; mche...@osg.samsung.com
> Cc: Shuah Khan; linux-arm-ker...@lists.infradead.org; linux-
> me...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()
> 
> s5p_mfc_remove() fails to release encoder and decoder video devices.
> 
> Signed-off-by: Shuah Khan <shua...@osg.samsung.com>
> Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> ---
> 
> Changes since v1:
> - Addressed comments from Javier Martinez Canillas and added
>   his reviewed by:
> 
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 274b4f1..f537b74 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1318,6 +1318,8 @@ static int s5p_mfc_remove(struct platform_device
> *pdev)
> 
>   video_unregister_device(dev->vfd_enc);
>   video_unregister_device(dev->vfd_dec);
> + video_device_release(dev->vfd_enc);
> + video_device_release(dev->vfd_dec);
>   v4l2_device_unregister(>v4l2_dev);
>   s5p_mfc_release_firmware(dev);
>   vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
> --
> 2.7.4


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Shuah Khan]

s5p_mfc_remove() fails to release encoder and decoder video devices.

Signed-off-by: Shuah Khan 
Reviewed-by: Javier Martinez Canillas 
---

Changes since v1:
- Addressed comments from Javier Martinez Canillas and added
  his reviewed by:

 drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 274b4f1..f537b74 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1318,6 +1318,8 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 
video_unregister_device(dev->vfd_enc);
video_unregister_device(dev->vfd_dec);
+   video_device_release(dev->vfd_enc);
+   video_device_release(dev->vfd_dec);
v4l2_device_unregister(>v4l2_dev);
s5p_mfc_release_firmware(dev);
vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Javier Martinez Canillas]

Hello Shuah,

On Wed, Jun 8, 2016 at 9:35 PM, Shuah Khan  wrote:
> s5p_mfc_remove() fails to release encoder and decoder video devices.
>
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 274b4f1..af61f54 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1317,7 +1317,9 @@ static int s5p_mfc_remove(struct platform_device *pdev)
> destroy_workqueue(dev->watchdog_workqueue);
>
> video_unregister_device(dev->vfd_enc);
> +   video_device_release(dev->vfd_enc);
> video_unregister_device(dev->vfd_dec);
> +   video_device_release(dev->vfd_dec);
> v4l2_device_unregister(>v4l2_dev);
> s5p_mfc_release_firmware(dev);
> vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
> --

Can you please do the remove operations in the inverse order of their
counterparts? IOW to do the release for both encoder and decoder after
their unregistration.

After that change:

Reviewed-by: Javier Martinez Canillas 

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: s5p-mfc fix memory leak in s5p_mfc_remove()

[Shuah Khan]

s5p_mfc_remove() fails to release encoder and decoder video devices.

Signed-off-by: Shuah Khan 
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 274b4f1..af61f54 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1317,7 +1317,9 @@ static int s5p_mfc_remove(struct platform_device *pdev)
destroy_workqueue(dev->watchdog_workqueue);
 
video_unregister_device(dev->vfd_enc);
+   video_device_release(dev->vfd_enc);
video_unregister_device(dev->vfd_dec);
+   video_device_release(dev->vfd_dec);
v4l2_device_unregister(>v4l2_dev);
s5p_mfc_release_firmware(dev);
vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html