| Message ID | 1461839104-29135-1-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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.
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) > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)
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(-)