* [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe
@ 2016-04-28 10:25 Marek Szyprowski
2016-04-28 10:25 ` [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe() Marek Szyprowski
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Marek Szyprowski @ 2016-04-28 10:25 UTC (permalink / raw)
To: linux-media, linux-samsung-soc
Cc: Marek Szyprowski, Sylwester Nawrocki, Jacek Anaszewski,
Sakari Ailus, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz
Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
keep a graph walk large enough around") changed
media_device_register_entity() function to take mdev->graph_mutex. This
causes deadlock in driver probe, which calls (indirectly) this function
with ->graph_mutex taken. This patch removes taking ->graph_mutex in
driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
for entity registration, so this change should be safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/platform/exynos4-is/media-dev.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 04348b502232..891625e77ef5 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1448,22 +1448,13 @@ static int fimc_md_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, fmd);
- /* Protect the media graph while we're registering entities */
- mutex_lock(&fmd->media_dev.graph_mutex);
-
ret = fimc_md_register_platform_entities(fmd, dev->of_node);
- if (ret) {
- mutex_unlock(&fmd->media_dev.graph_mutex);
+ if (ret)
goto err_clk;
- }
ret = fimc_md_register_sensor_entities(fmd);
- if (ret) {
- mutex_unlock(&fmd->media_dev.graph_mutex);
+ if (ret)
goto err_m_ent;
- }
-
- mutex_unlock(&fmd->media_dev.graph_mutex);
ret = device_create_file(&pdev->dev, &dev_attr_subdev_conf_mode);
if (ret)
--
1.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe()
2016-04-28 10:25 [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Marek Szyprowski
@ 2016-04-28 10:25 ` Marek Szyprowski
2016-05-02 13:36 ` Hans Verkuil
2016-05-01 21:59 ` [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Sakari Ailus
2016-05-01 22:09 ` Hans Verkuil
2 siblings, 1 reply; 7+ messages in thread
From: Marek Szyprowski @ 2016-04-28 10:25 UTC (permalink / raw)
To: linux-media, linux-samsung-soc
Cc: Marek Szyprowski, Sylwester Nawrocki, Jacek Anaszewski,
Sakari Ailus, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz
Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
keep a graph walk large enough around") changed
media_device_register_entity() function to take mdev->graph_mutex. This
causes deadlock in driver probe, which calls (indirectly) this function
with ->graph_mutex taken. This patch removes taking ->graph_mutex in
driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
for entity registration, so this change should be safe.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/platform/s3c-camif/camif-core.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
index 0b44b9accf50..af237af204e2 100644
--- a/drivers/media/platform/s3c-camif/camif-core.c
+++ b/drivers/media/platform/s3c-camif/camif-core.c
@@ -493,21 +493,17 @@ static int s3c_camif_probe(struct platform_device *pdev)
if (ret < 0)
goto err_sens;
- mutex_lock(&camif->media_dev.graph_mutex);
-
ret = v4l2_device_register_subdev_nodes(&camif->v4l2_dev);
if (ret < 0)
- goto err_unlock;
+ goto err_sens;
ret = camif_register_video_nodes(camif);
if (ret < 0)
- goto err_unlock;
+ goto err_sens;
ret = camif_create_media_links(camif);
if (ret < 0)
- goto err_unlock;
-
- mutex_unlock(&camif->media_dev.graph_mutex);
+ goto err_sens;
ret = media_device_register(&camif->media_dev);
if (ret < 0)
@@ -516,8 +512,6 @@ static int s3c_camif_probe(struct platform_device *pdev)
pm_runtime_put(dev);
return 0;
-err_unlock:
- mutex_unlock(&camif->media_dev.graph_mutex);
err_sens:
v4l2_device_unregister(&camif->v4l2_dev);
media_device_unregister(&camif->media_dev);
--
1.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe
2016-04-28 10:25 [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Marek Szyprowski
2016-04-28 10:25 ` [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe() Marek Szyprowski
@ 2016-05-01 21:59 ` Sakari Ailus
2016-05-02 10:52 ` Mauro Carvalho Chehab
2016-05-01 22:09 ` Hans Verkuil
2 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2016-05-01 21:59 UTC (permalink / raw)
To: Marek Szyprowski, linux-media, linux-samsung-soc
Cc: Sylwester Nawrocki, Jacek Anaszewski, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Mauro Carvalho Chehab
Hi Marek,
Marek Szyprowski wrote:
> Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
> keep a graph walk large enough around") changed
> media_device_register_entity() function to take mdev->graph_mutex. This
> causes deadlock in driver probe, which calls (indirectly) this function
> with ->graph_mutex taken. This patch removes taking ->graph_mutex in
> driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
> for entity registration, so this change should be safe.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
You could also add:
Fixes: 0c426c472b55 ("[media] media: Always keep a graph walk large
enough around")
I guess these should go to fixes, the patches in question are already
heading for v4.6. Cc Mauro.
--
Kind regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe
2016-04-28 10:25 [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Marek Szyprowski
2016-04-28 10:25 ` [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe() Marek Szyprowski
2016-05-01 21:59 ` [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Sakari Ailus
@ 2016-05-01 22:09 ` Hans Verkuil
2 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2016-05-01 22:09 UTC (permalink / raw)
To: Marek Szyprowski, linux-media, linux-samsung-soc
Cc: Sylwester Nawrocki, Jacek Anaszewski, Sakari Ailus,
Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz
On 04/28/2016 12:25 PM, Marek Szyprowski wrote:
> Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
> keep a graph walk large enough around") changed
> media_device_register_entity() function to take mdev->graph_mutex. This
> causes deadlock in driver probe, which calls (indirectly) this function
> with ->graph_mutex taken. This patch removes taking ->graph_mutex in
> driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
> for entity registration, so this change should be safe.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks!
Hans
> ---
> drivers/media/platform/exynos4-is/media-dev.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 04348b502232..891625e77ef5 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -1448,22 +1448,13 @@ static int fimc_md_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, fmd);
>
> - /* Protect the media graph while we're registering entities */
> - mutex_lock(&fmd->media_dev.graph_mutex);
> -
> ret = fimc_md_register_platform_entities(fmd, dev->of_node);
> - if (ret) {
> - mutex_unlock(&fmd->media_dev.graph_mutex);
> + if (ret)
> goto err_clk;
> - }
>
> ret = fimc_md_register_sensor_entities(fmd);
> - if (ret) {
> - mutex_unlock(&fmd->media_dev.graph_mutex);
> + if (ret)
> goto err_m_ent;
> - }
> -
> - mutex_unlock(&fmd->media_dev.graph_mutex);
>
> ret = device_create_file(&pdev->dev, &dev_attr_subdev_conf_mode);
> if (ret)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe
2016-05-01 21:59 ` [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Sakari Ailus
@ 2016-05-02 10:52 ` Mauro Carvalho Chehab
2016-05-02 11:00 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2016-05-02 10:52 UTC (permalink / raw)
To: Sakari Ailus
Cc: Marek Szyprowski, linux-media, linux-samsung-soc,
Sylwester Nawrocki, Jacek Anaszewski, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Hans Verkuil
Em Mon, 2 May 2016 00:59:56 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> Hi Marek,
>
> Marek Szyprowski wrote:
> > Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
> > keep a graph walk large enough around") changed
> > media_device_register_entity() function to take mdev->graph_mutex. This
> > causes deadlock in driver probe, which calls (indirectly) this function
> > with ->graph_mutex taken. This patch removes taking ->graph_mutex in
> > driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
> > for entity registration, so this change should be safe.
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> You could also add:
>
> Fixes: 0c426c472b55 ("[media] media: Always keep a graph walk large
> enough around")
>
> I guess these should go to fixes, the patches in question are already
> heading for v4.6. Cc Mauro.
The patches from Sakari for v4.6 were merged already at -rc6. Just merged
them back at the master branch.
Regards,
Mauro
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe
2016-05-02 10:52 ` Mauro Carvalho Chehab
@ 2016-05-02 11:00 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2016-05-02 11:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus
Cc: Marek Szyprowski, linux-media, linux-samsung-soc,
Sylwester Nawrocki, Jacek Anaszewski, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz
On 05/02/16 12:52, Mauro Carvalho Chehab wrote:
> Em Mon, 2 May 2016 00:59:56 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
>
>> Hi Marek,
>>
>> Marek Szyprowski wrote:
>>> Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
>>> keep a graph walk large enough around") changed
>>> media_device_register_entity() function to take mdev->graph_mutex. This
>>> causes deadlock in driver probe, which calls (indirectly) this function
>>> with ->graph_mutex taken. This patch removes taking ->graph_mutex in
>>> driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
>>> for entity registration, so this change should be safe.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>
>> You could also add:
>>
>> Fixes: 0c426c472b55 ("[media] media: Always keep a graph walk large
>> enough around")
>>
>> I guess these should go to fixes, the patches in question are already
>> heading for v4.6. Cc Mauro.
>
> The patches from Sakari for v4.6 were merged already at -rc6. Just merged
> them back at the master branch.
I'm confused. The two patches from Marek fixing driver probe deadlock are not
in the master tree (just pulled from it, it's now rc6), so the deadlock still
happens in kernel 4.6.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe()
2016-04-28 10:25 ` [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe() Marek Szyprowski
@ 2016-05-02 13:36 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2016-05-02 13:36 UTC (permalink / raw)
To: Marek Szyprowski, linux-media, linux-samsung-soc
Cc: Sylwester Nawrocki, Jacek Anaszewski, Sakari Ailus,
Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz
On 04/28/16 12:25, Marek Szyprowski wrote:
> Commit 0c426c472b5585ed6e59160359c979506d45ae49 ("[media] media: Always
> keep a graph walk large enough around") changed
> media_device_register_entity() function to take mdev->graph_mutex. This
> causes deadlock in driver probe, which calls (indirectly) this function
> with ->graph_mutex taken. This patch removes taking ->graph_mutex in
> driver probe to avoid deadlock. Other drivers don't take ->graph_mutex
> for entity registration, so this change should be safe.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks!
Hans
> ---
> drivers/media/platform/s3c-camif/camif-core.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
> index 0b44b9accf50..af237af204e2 100644
> --- a/drivers/media/platform/s3c-camif/camif-core.c
> +++ b/drivers/media/platform/s3c-camif/camif-core.c
> @@ -493,21 +493,17 @@ static int s3c_camif_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_sens;
>
> - mutex_lock(&camif->media_dev.graph_mutex);
> -
> ret = v4l2_device_register_subdev_nodes(&camif->v4l2_dev);
> if (ret < 0)
> - goto err_unlock;
> + goto err_sens;
>
> ret = camif_register_video_nodes(camif);
> if (ret < 0)
> - goto err_unlock;
> + goto err_sens;
>
> ret = camif_create_media_links(camif);
> if (ret < 0)
> - goto err_unlock;
> -
> - mutex_unlock(&camif->media_dev.graph_mutex);
> + goto err_sens;
>
> ret = media_device_register(&camif->media_dev);
> if (ret < 0)
> @@ -516,8 +512,6 @@ static int s3c_camif_probe(struct platform_device *pdev)
> pm_runtime_put(dev);
> return 0;
>
> -err_unlock:
> - mutex_unlock(&camif->media_dev.graph_mutex);
> err_sens:
> v4l2_device_unregister(&camif->v4l2_dev);
> media_device_unregister(&camif->media_dev);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-02 13:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28 10:25 [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Marek Szyprowski
2016-04-28 10:25 ` [PATCH 2/2] media: s3c-camif: fix deadlock on driver probe() Marek Szyprowski
2016-05-02 13:36 ` Hans Verkuil
2016-05-01 21:59 ` [PATCH 1/2] media: exynos4-is: fix deadlock on driver probe Sakari Ailus
2016-05-02 10:52 ` Mauro Carvalho Chehab
2016-05-02 11:00 ` Hans Verkuil
2016-05-01 22:09 ` Hans Verkuil
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).