diff mbox

soc-camera : sh_mobile_ceu_camera race on free_buffer ?

Message ID Pine.LNX.4.64.0902190924050.5156@axis700.grange (mailing list archive)
State RFC
Headers show

Commit Message

Guennadi Liakhovetski Feb. 19, 2009, 8:33 a.m. UTC 
On Thu, 19 Feb 2009, morimoto.kuninori@renesas.com wrote:

> 
> Dear Guennadi
> 
> > No, sorry, this is not the test I meant. "-c" doesn't really stress the 
> > path we need. You really have to execute capture_example multiple times 
> > completely. The race we're trying to catch happens on STREAMOFF, and for 
> > that you have to run the example completely through. So, I would suggest 
> > something like
> > 
> > for (( i=0; i<100; i++ )); do capture_example -d /dev/videoX -f -c 4; done
> 
> wow !!
> sorry miss understanding.
> 
> OK. I re-tried test with your script (300 times).
> 
> sh_mobile_ceu_camera didn't crashe with
> and without Magnus patch.
> 
> MigoR + ov772x + capture_example
> MigoR + tw9910 + capture_example (VUYU fix)
> AP325 + ov772x + capture_example

Hm, ok, maybe I can ask you about one more test, if you don't mind. The 
thing is, you only see the problem, if after the ->active buffer has been 
freed in free_buffer(), your DMA engine continues writing to the freed 
memory, but you only notice this, if some other driver manages to grab and 
use it in this time, then its data is going to be overwritten.

To actually see, if the race occurs, please do


Also, I think, even better than restarting capture-example completely 
would be to edit capture-example.c and put a loop around

	init_device();
	start_capturing();
	mainloop();
	stop_capturing();
	uninit_device();

or maybe even only

	start_capturing();
	mainloop();
	stop_capturing();

Best would be to first try all three loops without the patch from Magnus 
and see if any of them triggers. And use a larger frame (maximum that your 
sensor can deliver) to give the DMA engine more time per frame.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff -u a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -326,6 +326,7 @@ 
 	spin_lock_irqsave(&pcdev->lock, flags);
 
 	vb = pcdev->active;
+	WARN_ON(vb->state == VIDEOBUF_NEEDS_INIT);
 	list_del_init(&vb->queue);
 
 	if (!list_empty(&pcdev->capture))