* [PATCH 1/1] v4l: mem2mem_testdev: Fix race conditions in driver.
@ 2012-05-07 20:39 Tomasz Moń
2012-05-09 10:59 ` Marek Szyprowski
0 siblings, 1 reply; 2+ messages in thread
From: Tomasz Moń @ 2012-05-07 20:39 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Marek Szyprowski, Guennadi Liakhovetski,
Kyungmin Park, linux-media
Cc: linux-kernel, Tomasz Moń
The mem2mem_testdev allows multiple instances to be opened in parallel.
Source and destination queue data are being shared between all
instances, which can lead to kernel oops due to race conditions (most
likely to happen inside device_run()).
Attached patch fixes mentioned problem by storing queue data per device
context.
Signed-off-by: Tomasz Moń <desowin@gmail.com>
---
drivers/media/video/mem2mem_testdev.c | 50 +++++++++++++++++----------------
1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c
index 12897e8..ae7ca12 100644
--- a/drivers/media/video/mem2mem_testdev.c
+++ b/drivers/media/video/mem2mem_testdev.c
@@ -110,22 +110,6 @@ enum {
V4L2_M2M_DST = 1,
};
-/* Source and destination queue data */
-static struct m2mtest_q_data q_data[2];
-
-static struct m2mtest_q_data *get_q_data(enum v4l2_buf_type type)
-{
- switch (type) {
- case V4L2_BUF_TYPE_VIDEO_OUTPUT:
- return &q_data[V4L2_M2M_SRC];
- case V4L2_BUF_TYPE_VIDEO_CAPTURE:
- return &q_data[V4L2_M2M_DST];
- default:
- BUG();
- }
- return NULL;
-}
-
#define V4L2_CID_TRANS_TIME_MSEC V4L2_CID_PRIVATE_BASE
#define V4L2_CID_TRANS_NUM_BUFS (V4L2_CID_PRIVATE_BASE + 1)
@@ -198,8 +182,26 @@ struct m2mtest_ctx {
int aborting;
struct v4l2_m2m_ctx *m2m_ctx;
+
+ /* Source and destination queue data */
+ struct m2mtest_q_data q_data[2];
};
+static struct m2mtest_q_data *get_q_data(struct m2mtest_ctx *ctx,
+ enum v4l2_buf_type type)
+{
+ switch (type) {
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+ return &ctx->q_data[V4L2_M2M_SRC];
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+ return &ctx->q_data[V4L2_M2M_DST];
+ default:
+ BUG();
+ }
+ return NULL;
+}
+
+
static struct v4l2_queryctrl *get_ctrl(int id)
{
int i;
@@ -223,7 +225,7 @@ static int device_process(struct m2mtest_ctx *ctx,
int tile_w, bytes_left;
int width, height, bytesperline;
- q_data = get_q_data(V4L2_BUF_TYPE_VIDEO_OUTPUT);
+ q_data = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
width = q_data->width;
height = q_data->height;
@@ -436,7 +438,7 @@ static int vidioc_g_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
if (!vq)
return -EINVAL;
- q_data = get_q_data(f->type);
+ q_data = get_q_data(ctx, f->type);
f->fmt.pix.width = q_data->width;
f->fmt.pix.height = q_data->height;
@@ -535,7 +537,7 @@ static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
if (!vq)
return -EINVAL;
- q_data = get_q_data(f->type);
+ q_data = get_q_data(ctx, f->type);
if (!q_data)
return -EINVAL;
@@ -747,7 +749,7 @@ static int m2mtest_queue_setup(struct vb2_queue *vq,
struct m2mtest_q_data *q_data;
unsigned int size, count = *nbuffers;
- q_data = get_q_data(vq->type);
+ q_data = get_q_data(ctx, vq->type);
size = q_data->width * q_data->height * q_data->fmt->depth >> 3;
@@ -775,7 +777,7 @@ static int m2mtest_buf_prepare(struct vb2_buffer *vb)
dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
- q_data = get_q_data(vb->vb2_queue->type);
+ q_data = get_q_data(ctx, vb->vb2_queue->type);
if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
dprintk(ctx->dev, "%s data will not fit into plane (%lu < %lu)\n",
@@ -860,6 +862,9 @@ static int m2mtest_open(struct file *file)
ctx->transtime = MEM2MEM_DEF_TRANSTIME;
ctx->num_processed = 0;
+ ctx->q_data[V4L2_M2M_SRC].fmt = &formats[0];
+ ctx->q_data[V4L2_M2M_DST].fmt = &formats[0];
+
ctx->m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, &queue_init);
if (IS_ERR(ctx->m2m_ctx)) {
@@ -982,9 +987,6 @@ static int m2mtest_probe(struct platform_device *pdev)
goto err_m2m;
}
- q_data[V4L2_M2M_SRC].fmt = &formats[0];
- q_data[V4L2_M2M_DST].fmt = &formats[0];
-
return 0;
v4l2_m2m_release(dev->m2m_dev);
--
1.7.10
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH 1/1] v4l: mem2mem_testdev: Fix race conditions in driver.
2012-05-07 20:39 [PATCH 1/1] v4l: mem2mem_testdev: Fix race conditions in driver Tomasz Moń
@ 2012-05-09 10:59 ` Marek Szyprowski
0 siblings, 0 replies; 2+ messages in thread
From: Marek Szyprowski @ 2012-05-09 10:59 UTC (permalink / raw)
To: 'Tomasz Moń', 'Mauro Carvalho Chehab',
'Guennadi Liakhovetski', 'Kyungmin Park',
linux-media
Cc: linux-kernel, pawel
Hi Tomasz,
On Monday, May 07, 2012 10:39 PM Tomasz Moń wrote:
> The mem2mem_testdev allows multiple instances to be opened in parallel.
> Source and destination queue data are being shared between all
> instances, which can lead to kernel oops due to race conditions (most
> likely to happen inside device_run()).
>
> Attached patch fixes mentioned problem by storing queue data per device
> context.
>
> Signed-off-by: Tomasz Moń <desowin@gmail.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Thanks for fixing this bug!
> ---
> drivers/media/video/mem2mem_testdev.c | 50 +++++++++++++++++----------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c
> index 12897e8..ae7ca12 100644
> --- a/drivers/media/video/mem2mem_testdev.c
> +++ b/drivers/media/video/mem2mem_testdev.c
> @@ -110,22 +110,6 @@ enum {
> V4L2_M2M_DST = 1,
> };
>
> -/* Source and destination queue data */
> -static struct m2mtest_q_data q_data[2];
> -
> -static struct m2mtest_q_data *get_q_data(enum v4l2_buf_type type)
> -{
> - switch (type) {
> - case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> - return &q_data[V4L2_M2M_SRC];
> - case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> - return &q_data[V4L2_M2M_DST];
> - default:
> - BUG();
> - }
> - return NULL;
> -}
> -
> #define V4L2_CID_TRANS_TIME_MSEC V4L2_CID_PRIVATE_BASE
> #define V4L2_CID_TRANS_NUM_BUFS (V4L2_CID_PRIVATE_BASE + 1)
>
> @@ -198,8 +182,26 @@ struct m2mtest_ctx {
> int aborting;
>
> struct v4l2_m2m_ctx *m2m_ctx;
> +
> + /* Source and destination queue data */
> + struct m2mtest_q_data q_data[2];
> };
>
> +static struct m2mtest_q_data *get_q_data(struct m2mtest_ctx *ctx,
> + enum v4l2_buf_type type)
> +{
> + switch (type) {
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> + return &ctx->q_data[V4L2_M2M_SRC];
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> + return &ctx->q_data[V4L2_M2M_DST];
> + default:
> + BUG();
> + }
> + return NULL;
> +}
> +
> +
> static struct v4l2_queryctrl *get_ctrl(int id)
> {
> int i;
> @@ -223,7 +225,7 @@ static int device_process(struct m2mtest_ctx *ctx,
> int tile_w, bytes_left;
> int width, height, bytesperline;
>
> - q_data = get_q_data(V4L2_BUF_TYPE_VIDEO_OUTPUT);
> + q_data = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
>
> width = q_data->width;
> height = q_data->height;
> @@ -436,7 +438,7 @@ static int vidioc_g_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
> if (!vq)
> return -EINVAL;
>
> - q_data = get_q_data(f->type);
> + q_data = get_q_data(ctx, f->type);
>
> f->fmt.pix.width = q_data->width;
> f->fmt.pix.height = q_data->height;
> @@ -535,7 +537,7 @@ static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f)
> if (!vq)
> return -EINVAL;
>
> - q_data = get_q_data(f->type);
> + q_data = get_q_data(ctx, f->type);
> if (!q_data)
> return -EINVAL;
>
> @@ -747,7 +749,7 @@ static int m2mtest_queue_setup(struct vb2_queue *vq,
> struct m2mtest_q_data *q_data;
> unsigned int size, count = *nbuffers;
>
> - q_data = get_q_data(vq->type);
> + q_data = get_q_data(ctx, vq->type);
>
> size = q_data->width * q_data->height * q_data->fmt->depth >> 3;
>
> @@ -775,7 +777,7 @@ static int m2mtest_buf_prepare(struct vb2_buffer *vb)
>
> dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
>
> - q_data = get_q_data(vb->vb2_queue->type);
> + q_data = get_q_data(ctx, vb->vb2_queue->type);
>
> if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
> dprintk(ctx->dev, "%s data will not fit into plane (%lu < %lu)\n",
> @@ -860,6 +862,9 @@ static int m2mtest_open(struct file *file)
> ctx->transtime = MEM2MEM_DEF_TRANSTIME;
> ctx->num_processed = 0;
>
> + ctx->q_data[V4L2_M2M_SRC].fmt = &formats[0];
> + ctx->q_data[V4L2_M2M_DST].fmt = &formats[0];
> +
> ctx->m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, &queue_init);
>
> if (IS_ERR(ctx->m2m_ctx)) {
> @@ -982,9 +987,6 @@ static int m2mtest_probe(struct platform_device *pdev)
> goto err_m2m;
> }
>
> - q_data[V4L2_M2M_SRC].fmt = &formats[0];
> - q_data[V4L2_M2M_DST].fmt = &formats[0];
> -
> return 0;
>
> v4l2_m2m_release(dev->m2m_dev);
> --
> 1.7.10
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-05-09 10:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-07 20:39 [PATCH 1/1] v4l: mem2mem_testdev: Fix race conditions in driver Tomasz Moń
2012-05-09 10:59 ` Marek Szyprowski
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).