Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Matthieu CASTET]

Hi,

I am writing a soc camera driver, and I use sh_mobile_ceu_camera as an
example.

But I don't understand how buffer are handled when the application is
doing a streamoff :

streamoff will call videobuf_streamoff and then videobuf_queue_cancel.
videobuf_queue_cancel will call free_buffer.

But we didn't do stop_capture, so as far I understand the controller is
still writing data in memory. What prevent us to free the buffer we are
writing.


Why doesn't we do a stop_capture before videobuf_streamoff ?

I saw that pxa_camera use videobuf_waiton, before freeing the buffer.
That seem more safe, but that mean we need to wait that controller
finish to write all the pending buffer.


Matthieu

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Magnus Damm]

Hi Matthieu,

On Mon, Jan 19, 2009 at 11:02 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Hi,
>
> I am writing a soc camera driver, and I use sh_mobile_ceu_camera as an
> example.
>
> But I don't understand how buffer are handled when the application is
> doing a streamoff :
>
> streamoff will call videobuf_streamoff and then videobuf_queue_cancel.
> videobuf_queue_cancel will call free_buffer.
>
> But we didn't do stop_capture, so as far I understand the controller is
> still writing data in memory. What prevent us to free the buffer we are
> writing.

I have not looked into this in great detail, but isn't this handled by
the videobuf state? The videobuf has state VIDEOBUF_ACTIVE while it is
in use. I don't think such a buffer is freed.

> Why doesn't we do a stop_capture before videobuf_streamoff ?
>
> I saw that pxa_camera use videobuf_waiton, before freeing the buffer.
> That seem more safe, but that mean we need to wait that controller
> finish to write all the pending buffer.

Hm, but vivi.c does not use videbuf_waiton(). I guess this depends on
how the frames are queued in the driver.

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Matthieu CASTET]

Hi Magnus,

Magnus Damm a écrit :
> Hi Matthieu,
> 
> On Mon, Jan 19, 2009 at 11:02 PM, Matthieu CASTET
>> But we didn't do stop_capture, so as far I understand the controller is
>> still writing data in memory. What prevent us to free the buffer we are
>> writing.
> 
> I have not looked into this in great detail, but isn't this handled by
> the videobuf state? The videobuf has state VIDEOBUF_ACTIVE while it is
> in use. I don't think such a buffer is freed.
Well from my understanding form videobuf_queue_cancel [1], we call
buf_release on all buffer.

>> I saw that pxa_camera use videobuf_waiton, before freeing the buffer.
>> That seem more safe, but that mean we need to wait that controller
>> finish to write all the pending buffer.
> 
> Hm, but vivi.c does not use videbuf_waiton(). I guess this depends on
> how the frames are queued in the driver.
May be.

Matthieu


[1]
void videobuf_queue_cancel(struct videobuf_queue *q)
{
	unsigned long flags = 0;
	int i;

	q->streaming = 0;
	q->reading  = 0;
	wake_up_interruptible_sync(&q->wait);

	/* remove queued buffers from list */
	spin_lock_irqsave(q->irqlock, flags);
	for (i = 0; i < VIDEO_MAX_FRAME; i++) {
		if (NULL == q->bufs[i])
			continue;
		if (q->bufs[i]->state == VIDEOBUF_QUEUED) {
			list_del(&q->bufs[i]->queue);
			q->bufs[i]->state = VIDEOBUF_ERROR;
			wake_up_all(&q->bufs[i]->done);
		}
	}
	spin_unlock_irqrestore(q->irqlock, flags);

	/* free all buffers + clear queue */
	for (i = 0; i < VIDEO_MAX_FRAME; i++) {
		if (NULL == q->bufs[i])
			continue;
		q->ops->buf_release(q, q->bufs[i]);
	}
	INIT_LIST_HEAD(&q->stream);
}

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Magnus Damm]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

Hi Matthieu,

[CC Morimoto-san]
[Changed list to linux-media]

On Tue, Jan 20, 2009 at 6:27 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Magnus Damm a écrit :
>> On Mon, Jan 19, 2009 at 11:02 PM, Matthieu CASTET
>>> But we didn't do stop_capture, so as far I understand the controller is
>>> still writing data in memory. What prevent us to free the buffer we are
>>> writing.
>>
>> I have not looked into this in great detail, but isn't this handled by
>> the videobuf state? The videobuf has state VIDEOBUF_ACTIVE while it is
>> in use. I don't think such a buffer is freed.
> Well from my understanding form videobuf_queue_cancel [1], we call
> buf_release on all buffer.

Yeah, you are correct. I guess waiting for the buffer before freeing
is the correct way to do this. I guess vivi doesn't have to do this
since it's not using DMA.

Morimoto-san, can you check the attached patch? I've tested it on my
Migo-R board together with mplayer and it seems to work well here. I
don't think using mplayer triggers this error case though, so maybe we
should try some other application.

Cheers,

/ magnus

[-- Attachment #2: linux-2.6.29-media-video-sh_mobile_ceu-videobuf_waiton-20090213.patch --]
[-- Type: text/x-patch, Size: 397 bytes --]

--- 0001/drivers/media/video/sh_mobile_ceu_camera.c
+++ work/drivers/media/video/sh_mobile_ceu_camera.c	2009-02-13 18:55:55.000000000 +0900
@@ -150,6 +150,7 @@ static void free_buffer(struct videobuf_
 	if (in_interrupt())
 		BUG();
 
+	videobuf_waiton(&buf->vb, 0, 0);
 	videobuf_dma_contig_free(vq, &buf->vb);
 	dev_dbg(&icd->dev, "%s freed\n", __func__);
 	buf->vb.state = VIDEOBUF_NEEDS_INIT;

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[morimoto.kuninori]

Hi Magnus

> Morimoto-san, can you check the attached patch? I've tested it on my
> Migo-R board together with mplayer and it seems to work well here. I
> don't think using mplayer triggers this error case though, so maybe we
> should try some other application.

I checked this patch with following case.

Migo-R + ov772x + mplayer
Migo-R + tw9910 + mplayer
AP325  + ov772x + mplayer

It works well on all cases.
And I can agree with your opinion
"so maybe we should try some other application."

Best regards
--
Kuninori Morimoto

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Guennadi Liakhovetski]

Hi Morimoto-san

On Mon, 16 Feb 2009, morimoto.kuninori@renesas.com wrote:

> 
> Hi Magnus
> 
> > Morimoto-san, can you check the attached patch? I've tested it on my
> > Migo-R board together with mplayer and it seems to work well here. I
> > don't think using mplayer triggers this error case though, so maybe we
> > should try some other application.
> 
> I checked this patch with following case.
> 
> Migo-R + ov772x + mplayer
> Migo-R + tw9910 + mplayer
> AP325  + ov772x + mplayer
> 
> It works well on all cases.

So I can add your "Tested-by:"?

> And I can agree with your opinion
> "so maybe we should try some other application."

You can try to trigger the race with the capture.c example. Reduce the 
"count" variable in mainloop() and run capture.c in a loop for a while... 
Try without this patch and then with this patch. But I think this is a 
correct fix, so, unless you say, it crashes without it and with it, I'll 
apply it.

Thanks
Guennadi

> 
> Best regards
> --
> Kuninori Morimoto
> --
> 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
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[morimoto.kuninori]

Dear Guennadi

> > It works well on all cases.
> So I can add your "Tested-by:"?

Yes please.

> You can try to trigger the race with the capture.c example. Reduce the 
> "count" variable in mainloop() and run capture.c in a loop for a while... 
> Try without this patch and then with this patch. But I think this is a 
> correct fix, so, unless you say, it crashes without it and with it, I'll 
> apply it.

I tried with following command option

# capture_example -d /dev/videoX -f -c 1000

I used -f option, I think you already know the reason =).
I think -c 1000 is enough.

# Because tw9910 can not use YUYV,
# I fixed capture_example to use VUYU for tw9910.

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

Best regards
--
Kuninori Morimoto
 

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Guennadi Liakhovetski]

On Thu, 19 Feb 2009, morimoto.kuninori@renesas.com wrote:

> Dear Guennadi
> 
> > > It works well on all cases.
> > So I can add your "Tested-by:"?
> 
> Yes please.
> 
> > You can try to trigger the race with the capture.c example. Reduce the 
> > "count" variable in mainloop() and run capture.c in a loop for a while... 
> > Try without this patch and then with this patch. But I think this is a 
> > correct fix, so, unless you say, it crashes without it and with it, I'll 
> > apply it.
> 
> I tried with following command option
> 
> # capture_example -d /dev/videoX -f -c 1000
> 
> I used -f option, I think you already know the reason =).

Yes, and I'm working on that.

> I think -c 1000 is enough.

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

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[morimoto.kuninori]

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

Best regards
--
Kuninori Morimoto
 

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Guennadi Liakhovetski]

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

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))

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

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[morimoto.kuninori]

Dear Guennadi

> 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.
(snip)
> 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.

Hmm. maybe I could understand...

OK. 
I tested with MigoR + ov772x + following way.

1) apply patch to kernel

> 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))

2) apply patch to capture-example

I think your wanting to say is this...

diff -u capture_example.c capture_example.c.test
--- capture_example.c	2009-01-24 09:35:12.000000000 +0900
+++ capture_example.c.test	2009-02-19 18:09:32.000000000 +0900
@@ -590,6 +590,8 @@
 
 int main(int argc, char **argv)
 {
+	int i;
+
 	dev_name = "/dev/video0";
 
 	for (;;) {
@@ -648,11 +650,14 @@
 	}
 
 	open_device();
-	init_device();
-	start_capturing();
-	mainloop();
-	stop_capturing();
-	uninit_device();
+        for (i=0 ; i<100 ; i++) {
+            printf("%d ", i);
+            init_device();
+            start_capturing();
+            mainloop();
+            stop_capturing();
+            uninit_device();
+        }
 	close_device();
 	fprintf(stderr, "\n");
 	return 0;

3) test this capture_example.

Then, It say

======================================================================
camera 0-0: SuperH Mobile CEU driver attached to camera 0
 camera_host0: Format 0 not found
.0 ... camera_host0: Format 0 not found
------------[ cut here ]------------
Badness at 8c141b9e [verbose debug info unavailable]

Pid : 869, Comm:                c2
CPU : 0                 Not tainted  (2.6.29-rc4-00197-g41480ae-dirty #801)

PC is at sh_mobile_ceu_irq+0x2a/0xc8
PR is at handle_IRQ_event+0x2e/0x68
PC  : 8c141b9e SP  : 8fe15e40 SR  : 400001f1 TEA : 2961ba40
R0  : 8ff05afc R1  : 00000000 R2  : 8fe14000 R3  : 00000000
R4  : 8ff05a00 R5  : 8ff05a00 R6  : 00000010 R7  : 00000000
R8  : 8ff416e0 R9  : 000000f0 R10 : 00000000 R11 : 00000034
R12 : 8ff414d0 R13 : 8ff3a1e0 R14 : 8ff89e74
MACH: 00000000 MACL: 00000c30 GBR : 29686450 PR  : 8c030e0a

Call trace:
[<8c030e0a>] handle_IRQ_event+0x2e/0x68
[<8c0326a8>] handle_level_irq+0x6c/0x100
[<8c00356c>] do_IRQ+0x34/0x5c
[<8c007118>] ret_from_irq+0x0/0x10
[<8c003538>] do_IRQ+0x0/0x5c
[<8c0f2c44>] memset+0x30/0x4c
[<8c00b25a>] dma_alloc_coherent+0x5e/0x148
[<8c141742>] __videobuf_mmap_mapper+0xbe/0x1a4
[<8c13fb88>] videobuf_mmap_mapper+0x3c/0x70
[<8c050ad8>] mmap_region+0x184/0x34c
[<8c005234>] old_mmap+0x60/0xa8
[<8c007266>] syscall_call+0xc/0x10
[<8c0051d4>] old_mmap+0x0/0xa8

camera 0-0: SuperH Mobile CEU driver detached from camera 0
Trace/breakpoint trap
======================================================================

4) apply Magnus patch
5) and test it again

then It say...

========================================================
-bash-3.00# ./c2 -d /dev/video0 -f -c 4
camera 0-0: SuperH Mobile CEU driver attached to camera 0
 camera_host0: Format 0 not found
.0 ... camera_host0: Format 0 not found
.1 ... camera_host0: Format 0 not found
.2 ... camera_host0: Format 0 not found
.3 ... camera_host0: Format 0 not found
(snip)
.96 ... camera_host0: Format 0 not found
.97 ... camera_host0: Format 0 not found
.98 ... camera_host0: Format 0 not found
.99 ...camera 0-0: SuperH Mobile CEU driver detached from camera 0
========================================================

Best regards
--
Kuninori Morimoto
 

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Guennadi Liakhovetski]

On Thu, 19 Feb 2009, morimoto.kuninori@renesas.com wrote:

> 
> Dear Guennadi
> 
> > 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.
> (snip)
> > 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.
> 
> Hmm. maybe I could understand...
> 
> OK. 
> I tested with MigoR + ov772x + following way.
> 
> 1) apply patch to kernel
> 
> > 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))
> 
> 2) apply patch to capture-example
> 
> I think your wanting to say is this...
> 
> diff -u capture_example.c capture_example.c.test
> --- capture_example.c	2009-01-24 09:35:12.000000000 +0900
> +++ capture_example.c.test	2009-02-19 18:09:32.000000000 +0900
> @@ -590,6 +590,8 @@
>  
>  int main(int argc, char **argv)
>  {
> +	int i;
> +
>  	dev_name = "/dev/video0";
>  
>  	for (;;) {
> @@ -648,11 +650,14 @@
>  	}
>  
>  	open_device();
> -	init_device();
> -	start_capturing();
> -	mainloop();
> -	stop_capturing();
> -	uninit_device();
> +        for (i=0 ; i<100 ; i++) {
> +            printf("%d ", i);
> +            init_device();
> +            start_capturing();
> +            mainloop();
> +            stop_capturing();
> +            uninit_device();
> +        }
>  	close_device();
>  	fprintf(stderr, "\n");
>  	return 0;
> 
> 3) test this capture_example.
> 
> Then, It say
> 
> ======================================================================
> camera 0-0: SuperH Mobile CEU driver attached to camera 0
>  camera_host0: Format 0 not found
> .0 ... camera_host0: Format 0 not found
> ------------[ cut here ]------------
> Badness at 8c141b9e [verbose debug info unavailable]
> 
> Pid : 869, Comm:                c2
> CPU : 0                 Not tainted  (2.6.29-rc4-00197-g41480ae-dirty #801)
> 
> PC is at sh_mobile_ceu_irq+0x2a/0xc8
> PR is at handle_IRQ_event+0x2e/0x68
> PC  : 8c141b9e SP  : 8fe15e40 SR  : 400001f1 TEA : 2961ba40
> R0  : 8ff05afc R1  : 00000000 R2  : 8fe14000 R3  : 00000000
> R4  : 8ff05a00 R5  : 8ff05a00 R6  : 00000010 R7  : 00000000
> R8  : 8ff416e0 R9  : 000000f0 R10 : 00000000 R11 : 00000034
> R12 : 8ff414d0 R13 : 8ff3a1e0 R14 : 8ff89e74
> MACH: 00000000 MACL: 00000c30 GBR : 29686450 PR  : 8c030e0a
> 
> Call trace:
> [<8c030e0a>] handle_IRQ_event+0x2e/0x68
> [<8c0326a8>] handle_level_irq+0x6c/0x100
> [<8c00356c>] do_IRQ+0x34/0x5c
> [<8c007118>] ret_from_irq+0x0/0x10
> [<8c003538>] do_IRQ+0x0/0x5c
> [<8c0f2c44>] memset+0x30/0x4c
> [<8c00b25a>] dma_alloc_coherent+0x5e/0x148
> [<8c141742>] __videobuf_mmap_mapper+0xbe/0x1a4
> [<8c13fb88>] videobuf_mmap_mapper+0x3c/0x70
> [<8c050ad8>] mmap_region+0x184/0x34c
> [<8c005234>] old_mmap+0x60/0xa8
> [<8c007266>] syscall_call+0xc/0x10
> [<8c0051d4>] old_mmap+0x0/0xa8

Yesss! There you go - this is the race we are hunting.

> camera 0-0: SuperH Mobile CEU driver detached from camera 0
> Trace/breakpoint trap
> ======================================================================
> 
> 4) apply Magnus patch
> 5) and test it again
> 
> then It say...
> 
> ========================================================
> -bash-3.00# ./c2 -d /dev/video0 -f -c 4
> camera 0-0: SuperH Mobile CEU driver attached to camera 0
>  camera_host0: Format 0 not found
> .0 ... camera_host0: Format 0 not found
> .1 ... camera_host0: Format 0 not found
> .2 ... camera_host0: Format 0 not found
> .3 ... camera_host0: Format 0 not found
> (snip)
> .96 ... camera_host0: Format 0 not found
> .97 ... camera_host0: Format 0 not found
> .98 ... camera_host0: Format 0 not found
> .99 ...camera 0-0: SuperH Mobile CEU driver detached from camera 0
> ========================================================

Nice and clean. Good, now I just need a formal patch submission with a 
"Tested-by" string, please.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[morimoto.kuninori]

Dear Guennadi

> > ------------[ cut here ]------------
> > Badness at 8c141b9e [verbose debug info unavailable]
> > 
> > Pid : 869, Comm:                c2
> > CPU : 0                 Not tainted  (2.6.29-rc4-00197-g41480ae-dirty #801)
(snip)
> Yesss! There you go - this is the race we are hunting.
(snip)
> Nice and clean. Good, now I just need a formal patch submission with a 
> "Tested-by" string, please.

Fuuu. good !! Thank you.

Tested-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

Best regards
--
Kuninori Morimoto
 

Re: soc-camera : sh_mobile_ceu_camera race on free_buffer ?

[Guennadi Liakhovetski]

On Fri, 13 Feb 2009, Magnus Damm wrote:

> Hi Matthieu,
> 
> [CC Morimoto-san]
> [Changed list to linux-media]
> 
> On Tue, Jan 20, 2009 at 6:27 PM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
> > Magnus Damm a écrit :
> >> On Mon, Jan 19, 2009 at 11:02 PM, Matthieu CASTET
> >>> But we didn't do stop_capture, so as far I understand the controller is
> >>> still writing data in memory. What prevent us to free the buffer we are
> >>> writing.
> >>
> >> I have not looked into this in great detail, but isn't this handled by
> >> the videobuf state? The videobuf has state VIDEOBUF_ACTIVE while it is
> >> in use. I don't think such a buffer is freed.
> > Well from my understanding form videobuf_queue_cancel [1], we call
> > buf_release on all buffer.
> 
> Yeah, you are correct. I guess waiting for the buffer before freeing
> is the correct way to do this. I guess vivi doesn't have to do this
> since it's not using DMA.
> 
> Morimoto-san, can you check the attached patch? I've tested it on my
> Migo-R board together with mplayer and it seems to work well here. I
> don't think using mplayer triggers this error case though, so maybe we
> should try some other application.

Magnus, can you, please, submit it with an Sob, after Morimoto-san has 
tested it with capture.c?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer