Re: bug in changeset 13239:54535665f94b ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mauro Carvalho Chehab schrieb:
Hi Hartmut,

Em Sun, 01 Nov 2009 16:59:26 +0100
e9hack <e9hack@xxxxxxxxxxxxxx> escreveu:

Hi,

something is wrong in changeset 13239:54535665f94b. After applying it, I get page faults
in various applications:
...

If I remove the call to release_all_pagetables() in buffer_release(), I don't see this
page faults.


Em Mon, 2 Nov 2009 21:27:44 +0100
e9hack@xxxxxxxxxxxxxx escreveu:

the BUG is in vidioc_streamoff() for the saa7146. This function
releases all buffers first, and stops the capturing and dma tranfer of
the saa7146 as second. If the page table, which is currently used by
the saa7146, is modified by another thread, the saa7146 writes
anywhere to the physical RAM. IMHO vidioc_streamoff() must stop the
saa7146 first and may then release the buffers.

I agree. We need first to stop DMA activity, and then release the page tables.

Could you please test if the enclosed patch fixes the issue?

Cheers,
Mauro

saa7146: stop DMA before de-allocating DMA scatter/gather page buffers

Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>

diff --git a/linux/drivers/media/common/saa7146_video.c b/linux/drivers/media/common/saa7146_video.c
--- a/linux/drivers/media/common/saa7146_video.c
+++ b/linux/drivers/media/common/saa7146_video.c
@@ -1334,9 +1334,9 @@ static void buffer_release(struct videob
DEB_CAP(("vbuf:%p\n",vb)); + saa7146_dma_free(dev,q,buf);
+
 	release_all_pagetables(dev, buf);
-
-	saa7146_dma_free(dev,q,buf);
 }
static struct videobuf_queue_ops video_qops = {

Hi Mauro,
I cannot easily catch any memory overwriting done by saa7146-capture-dma, but Hartmut is right that there is quite a chance. I would prefer calling video_end() before videobuf_streamoff() in vidioc_streamoff(), because this physically shuts down the capture immediately at the hardware source. By the way, this is done in the same sequence in video_close, where videobuf_stop (which calls videobuf_streamoff) also gets called after video_end. I have no negative impact with your patch and it might shutdown the dma as well, but as said before, I don't see any immediate errors even with the released version.
Cheers
Johann
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux