* [PATCH 1/3] [media] exynos-gsc: don't release a non-dynamically allocated video_device
2016-10-07 20:39 [PATCH 0/3] [media] exynos-gsc: Another round of cleanup and fixes Javier Martinez Canillas
@ 2016-10-07 20:39 ` Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 2/3] [media] exynos-gsc: unregister video device node on driver removal Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 3/3] [media] exynos-gsc: cleanup m2m src and dst vb2 queues on STREAMOFF Javier Martinez Canillas
2 siblings, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2016-10-07 20:39 UTC (permalink / raw)
To: linux-kernel
Cc: Mauro Carvalho Chehab, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Hans Verkuil, Inki Dae, linux-samsung-soc,
Sylwester Nawrocki, Shuah Khan, Nicolas Dufresne, linux-media,
Javier Martinez Canillas
The struct v4l2_device instance for the G-Scaler is not dyanmically
allocated but a member of the struct gsc_dev. In fact, the assigned
.release callback is video_device_release_empty().
But gsc_register_m2m_device() attempts to release the v4l2_device by
calling video_device_release() in its error path. This is wrong since
the v4l2_device wasn't allocated directly and will be freed once its
parent struct gsc_dev is freed.
While being there, rename the remaining goto label in the error path
to something that better explains the error path cleanup.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/platform/exynos-gsc/gsc-m2m.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index e2a16b52f87d..a1cac52ea230 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -760,24 +760,21 @@ int gsc_register_m2m_device(struct gsc_dev *gsc)
gsc->m2m.m2m_dev = v4l2_m2m_init(&gsc_m2m_ops);
if (IS_ERR(gsc->m2m.m2m_dev)) {
dev_err(&pdev->dev, "failed to initialize v4l2-m2m device\n");
- ret = PTR_ERR(gsc->m2m.m2m_dev);
- goto err_m2m_r1;
+ return PTR_ERR(gsc->m2m.m2m_dev);
}
ret = video_register_device(&gsc->vdev, VFL_TYPE_GRABBER, -1);
if (ret) {
dev_err(&pdev->dev,
"%s(): failed to register video device\n", __func__);
- goto err_m2m_r2;
+ goto err_m2m_release;
}
pr_debug("gsc m2m driver registered as /dev/video%d", gsc->vdev.num);
return 0;
-err_m2m_r2:
+err_m2m_release:
v4l2_m2m_release(gsc->m2m.m2m_dev);
-err_m2m_r1:
- video_device_release(gsc->m2m.vfd);
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] [media] exynos-gsc: unregister video device node on driver removal
2016-10-07 20:39 [PATCH 0/3] [media] exynos-gsc: Another round of cleanup and fixes Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 1/3] [media] exynos-gsc: don't release a non-dynamically allocated video_device Javier Martinez Canillas
@ 2016-10-07 20:39 ` Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 3/3] [media] exynos-gsc: cleanup m2m src and dst vb2 queues on STREAMOFF Javier Martinez Canillas
2 siblings, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2016-10-07 20:39 UTC (permalink / raw)
To: linux-kernel
Cc: Mauro Carvalho Chehab, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Hans Verkuil, Inki Dae, linux-samsung-soc,
Sylwester Nawrocki, Shuah Khan, Nicolas Dufresne, linux-media,
Javier Martinez Canillas
The driver doesn't unregister the video device node when the driver is
removed, this keeps video device nodes that makes the machine to crash
with a NULL pointer dereference when nodes are attempted to be opened:
[ 36.530006] Unable to handle kernel paging request at virtual address bf1f8200
[ 36.535985] pgd = edbbc000
[ 36.538486] [bf1f8200] *pgd=6d99a811, *pte=00000000, *ppte=00000000
[ 36.544727] Internal error: Oops: 7 [#1] PREEMPT SMP ARM
[ 36.550016] Modules linked in: s5p_jpeg s5p_mfc v4l2_mem2mem videobuf2_dma_contig
[ 36.566303] CPU: 6 PID: 533 Comm: v4l2-ctl Not tainted 4.8.0
[ 36.574466] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 36.580526] task: ee3cc600 task.stack: ed626000
[ 36.585046] PC is at try_module_get+0x1c/0xac
[ 36.589364] LR is at try_module_get+0x1c/0xac
[ 36.593698] pc : [<c0187a60>] lr : [<c0187a60>] psr: 80070013
[ 36.593698] sp : ed627de0 ip : a0070013 fp : 00000000
[ 36.605156] r10: 00000002 r9 : ed627ed0 r8 : 00000000
[ 36.610331] r7 : c01e5f14 r6 : ed57be00 r5 : bf1f8200 r4 : bf1f8200
[ 36.616834] r3 : 00000002 r2 : 00000002 r1 : 01930192 r0 : 00000001
..
[ 36.785004] [<c0187a60>] (try_module_get) from [<c01e5c10>] (cdev_get+0x1c/0x4c)
[ 36.792362] [<c01e5c10>] (cdev_get) from [<c01e5f40>] (chrdev_open+0x2c/0x178)
[ 36.799555] [<c01e5f40>] (chrdev_open) from [<c01df5d4>] (do_dentry_open+0x1e0/0x300)
[ 36.807360] [<c01df5d4>] (do_dentry_open) from [<c01eecdc>] (path_openat+0x35c/0xf58)
[ 36.815154] [<c01eecdc>] (path_openat) from [<c01f0668>] (do_filp_open+0x5c/0xc0)
[ 36.822606] [<c01f0668>] (do_filp_open) from [<c01e09ac>] (do_sys_open+0x10c/0x1bc)
[ 36.830235] [<c01e09ac>] (do_sys_open) from [<c01078c0>] (ret_fast_syscall+0x0/0x3c)
[ 36.837942] Code: 0a00001c e1a04000 e3a00001 ebfec92d (e5943000)
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index a1cac52ea230..c8c0bcec35ed 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -781,6 +781,8 @@ err_m2m_release:
void gsc_unregister_m2m_device(struct gsc_dev *gsc)
{
- if (gsc)
+ if (gsc) {
v4l2_m2m_release(gsc->m2m.m2m_dev);
+ video_unregister_device(&gsc->vdev);
+ }
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] [media] exynos-gsc: cleanup m2m src and dst vb2 queues on STREAMOFF
2016-10-07 20:39 [PATCH 0/3] [media] exynos-gsc: Another round of cleanup and fixes Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 1/3] [media] exynos-gsc: don't release a non-dynamically allocated video_device Javier Martinez Canillas
2016-10-07 20:39 ` [PATCH 2/3] [media] exynos-gsc: unregister video device node on driver removal Javier Martinez Canillas
@ 2016-10-07 20:39 ` Javier Martinez Canillas
2 siblings, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2016-10-07 20:39 UTC (permalink / raw)
To: linux-kernel
Cc: Mauro Carvalho Chehab, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Hans Verkuil, Inki Dae, linux-samsung-soc,
Sylwester Nawrocki, Shuah Khan, Nicolas Dufresne, linux-media,
Javier Martinez Canillas
Media drivers that use the videobuf2 framework have to give back to vb2
all the buffers that received from vb2 using its .buf_queue callback.
But the exynos-gsc driver isn't doing a proper cleanup so vb2 complains
that the number of buffers enqueued and received are not balanced:
WARNING: CPU: 2 PID: 660 at drivers/media/v4l2-core/videobuf2-core.c:1654 __vb2_queue_cancel+0xec/0x150 [videobuf2_core]
Modules linked in: mwifiex_sdio mwifiex uvcvideo exynos_gsc videobuf2_vmalloc s5p_mfc s5p_jpeg
CPU: 2 PID: 660 Comm: lt-gst-validate Not tainted 4.8.0
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c010e24c>] (unwind_backtrace) from [<c010af30>] (show_stack+0x10/0x14)
[<c010af30>] (show_stack) from [<c03291a4>] (dump_stack+0x88/0x9c)
[<c03291a4>] (dump_stack) from [<c011a858>] (__warn+0xe8/0x100)
[<c011a858>] (__warn) from [<c011a920>] (warn_slowpath_null+0x20/0x28)
[<c011a920>] (warn_slowpath_null) from [<bf0b6ed0>] (__vb2_queue_cancel+0xec/0x150 [videobuf2_core])
[<bf0b6ed0>] (__vb2_queue_cancel [videobuf2_core]) from [<bf0b7464>] (vb2_core_streamoff+0x34/0x9c [videobuf2_core])
[<bf0b7464>] (vb2_core_streamoff [videobuf2_core]) from [<bf11b9e8>] (v4l2_m2m_streamoff+0x2c/0xe4 [v4l2_mem2mem])
[<bf11b9e8>] (v4l2_m2m_streamoff [v4l2_mem2mem]) from [<bf01b84c>] (__video_do_ioctl+0x298/0x30c [videodev])
[<bf01b84c>] (__video_do_ioctl [videodev]) from [<bf01b234>] (video_usercopy+0x174/0x4e8 [videodev])
[<bf01b234>] (video_usercopy [videodev]) from [<bf0165c8>] (v4l2_ioctl+0xc4/0xd8 [videodev])
[<bf0165c8>] (v4l2_ioctl [videodev]) from [<c01f291c>] (do_vfs_ioctl+0x9c/0x8f4)
[<c01f291c>] (do_vfs_ioctl) from [<c01f31a8>] (SyS_ioctl+0x34/0x5c)
[<c01f31a8>] (SyS_ioctl) from [<c01078c0>] (ret_fast_syscall+0x0/0x3c)
Fix this by passing back to vb2 all the received buffers that were not
processed.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/platform/exynos-gsc/gsc-m2m.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index c8c0bcec35ed..f49f24b4462a 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -66,12 +66,29 @@ static int gsc_m2m_start_streaming(struct vb2_queue *q, unsigned int count)
return ret > 0 ? 0 : ret;
}
+static void __gsc_m2m_cleanup_queue(struct gsc_ctx *ctx)
+{
+ struct vb2_v4l2_buffer *src_vb, *dst_vb;
+
+ while (v4l2_m2m_num_src_bufs_ready(ctx->m2m_ctx) > 0) {
+ src_vb = v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
+ v4l2_m2m_buf_done(src_vb, VB2_BUF_STATE_ERROR);
+ }
+
+ while (v4l2_m2m_num_dst_bufs_ready(ctx->m2m_ctx) > 0) {
+ dst_vb = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
+ v4l2_m2m_buf_done(dst_vb, VB2_BUF_STATE_ERROR);
+ }
+}
+
static void gsc_m2m_stop_streaming(struct vb2_queue *q)
{
struct gsc_ctx *ctx = q->drv_priv;
__gsc_m2m_job_abort(ctx);
+ __gsc_m2m_cleanup_queue(ctx);
+
pm_runtime_put(&ctx->gsc_dev->pdev->dev);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread