public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-03 14:12 Support for zerocopy to DSP on OMAP3 Arnout Vandecappelle
@ 2010-03-04 16:00 ` Arnout Vandecappelle
  2010-03-17 20:57   ` Sakari Ailus
  0 siblings, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-04 16:00 UTC (permalink / raw)
  To: linux-media, Sakari Ailus, mchehab; +Cc: Arnout Vandecappelle

videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
a scatterlist element for every page.  However, this is not true for
bus addresses, so other functions shouldn't rely on the length of the
scatter list being equal to nr_pages.
---
 drivers/media/video/videobuf-dma-sg.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
index da1790e..3b6f1b8 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q, struct videobuf_dmabuf *dma)
 	}
 	if (!dma->bus_addr) {
 		dma->sglen = dma_map_sg(q->dev, dma->sglist,
-					dma->nr_pages, dma->direction);
+					dma->sglen, dma->direction);
 		if (0 == dma->sglen) {
 			printk(KERN_WARNING
 			       "%s: videobuf_map_sg failed\n",__func__);
@@ -262,7 +262,7 @@ int videobuf_dma_sync(struct videobuf_queue *q, struct videobuf_dmabuf *dma)
 	MAGIC_CHECK(dma->magic, MAGIC_DMABUF);
 	BUG_ON(!dma->sglen);
 
-	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->nr_pages, dma->direction);
+	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->sglen, dma->direction);
 	return 0;
 }
 
@@ -272,7 +272,7 @@ int videobuf_dma_unmap(struct videobuf_queue* q,struct videobuf_dmabuf *dma)
 	if (!dma->sglen)
 		return 0;
 
-	dma_unmap_sg(q->dev, dma->sglist, dma->nr_pages, dma->direction);
+	dma_unmap_sg(q->dev, dma->sglist, dma->sglen, dma->direction);
 
 	kfree(dma->sglist);
 	dma->sglist = NULL;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-04 16:00 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
@ 2010-03-17 20:57   ` Sakari Ailus
  2010-03-17 22:50     ` Arnout Vandecappelle
  0 siblings, 1 reply; 11+ messages in thread
From: Sakari Ailus @ 2010-03-17 20:57 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: linux-media, mchehab

Hi Arnout,

Arnout Vandecappelle wrote:
> videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
> a scatterlist element for every page.  However, this is not true for
> bus addresses, so other functions shouldn't rely on the length of the
> scatter list being equal to nr_pages.
> ---
>  drivers/media/video/videobuf-dma-sg.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
> index da1790e..3b6f1b8 100644
> --- a/drivers/media/video/videobuf-dma-sg.c
> +++ b/drivers/media/video/videobuf-dma-sg.c
> @@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q, struct videobuf_dmabuf *dma)
>  	}
>  	if (!dma->bus_addr) {
>  		dma->sglen = dma_map_sg(q->dev, dma->sglist,
> -					dma->nr_pages, dma->direction);
> +					dma->sglen, dma->direction);
>  		if (0 == dma->sglen) {
>  			printk(KERN_WARNING
>  			       "%s: videobuf_map_sg failed\n",__func__);

Where is dma->sglen actually set?

videobuf_dma_map() is used in __videobuf_iolock
(drivers/media/video/videobuf-dma-sg.c) but neither
videobuf_dma_init_kernel() nor videobuf_dma_init_user() seem to set it.
This apparently leaves the value uninitialised.

I definitely think it should be assigned somewhere. :-)

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-17 20:57   ` Sakari Ailus
@ 2010-03-17 22:50     ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-17 22:50 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, mchehab


On Wednesday 17 March 2010 21:57:18, Sakari Ailus wrote:
> Arnout Vandecappelle wrote:
> > diff --git a/drivers/media/video/videobuf-dma-sg.c
> > b/drivers/media/video/videobuf-dma-sg.c index da1790e..3b6f1b8 100644
> > --- a/drivers/media/video/videobuf-dma-sg.c
> > +++ b/drivers/media/video/videobuf-dma-sg.c
> > @@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q,
> > struct videobuf_dmabuf *dma)
> > 
> >  	}
> >  	if (!dma->bus_addr) {
> >  	
> >  		dma->sglen = dma_map_sg(q->dev, dma->sglist,
> > 
> > -					dma->nr_pages, dma->direction);
> > +					dma->sglen, dma->direction);
> > 
> >  		if (0 == dma->sglen) {
> >  		
> >  			printk(KERN_WARNING
> >  			
> >  			       "%s: videobuf_map_sg failed\n",__func__);
> 
> Where is dma->sglen actually set?
> 
> videobuf_dma_map() is used in __videobuf_iolock
> (drivers/media/video/videobuf-dma-sg.c) but neither
> videobuf_dma_init_kernel() nor videobuf_dma_init_user() seem to set it.
> This apparently leaves the value uninitialised.
> 
> I definitely think it should be assigned somewhere. :-)

 It's assigned there exactly - nr_pages shouldn't have been replaced there.

 Updated patches follow.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  31BB CF53 8660 6F88 345D  54CC A836 5879 20D7 CF43

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] Support for zerocopy to DMA buffers
@ 2010-03-17 22:53 Arnout Vandecappelle
  2010-03-17 22:53 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
  2010-03-17 22:53 ` [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory Arnout Vandecappelle
  0 siblings, 2 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-17 22:53 UTC (permalink / raw)
  To: linux-media, mchehab, arnout

 Hoi,

 [Please CC me, I'm not subscribed.]

 I'm implementing zerocopy transfer from a v4l2 camera to the DSP on an 
OMAP3 (based on earlier work by Stefan Kost [1][2]).  Therefore I'm using 
V4L2_MEMORY_USERPTR to pass in the memory area allocated by TI's DMAI 
driver.  However, this has flags VM_IO | VM_PFNMAP.  This means that it is 
not possible to do get_user_pages() on it - it's an area that is not 
pageable and possibly even doesn't pass the MMU.

 In order to support this kind of zerocopy construct, I propose to add 
checks for VM_IO | VM_PFNMAP and only get pages from areas that don't have 
these flags set.

 I also fixed another issue: it was assumed that dma->sglen == dma->nr_pages,
which is not true for the USERPTR memory.  Thanks to Sakari Ailus for
pointing out a bug I introduced there.

 Regards,
 Arnout

[1] https://bugzilla.gnome.org/show_bug.cgi?id=583890
[2] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/6209


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-17 22:53 [PATCH v3] Support for zerocopy to DMA buffers Arnout Vandecappelle
@ 2010-03-17 22:53 ` Arnout Vandecappelle
  2010-03-24  5:43   ` Sakari Ailus
  2010-03-17 22:53 ` [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory Arnout Vandecappelle
  1 sibling, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-17 22:53 UTC (permalink / raw)
  To: linux-media, mchehab, arnout

videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
a scatterlist element for every page.  However, this is not true for
bus addresses, so other functions shouldn't rely on the length of the
scatter list being equal to nr_pages.
---
 drivers/media/video/videobuf-dma-sg.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
index da1790e..18aaf54 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -262,7 +262,7 @@ int videobuf_dma_sync(struct videobuf_queue *q, struct videobuf_dmabuf *dma)
 	MAGIC_CHECK(dma->magic, MAGIC_DMABUF);
 	BUG_ON(!dma->sglen);
 
-	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->nr_pages, dma->direction);
+	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->sglen, dma->direction);
 	return 0;
 }
 
@@ -272,7 +272,7 @@ int videobuf_dma_unmap(struct videobuf_queue* q,struct videobuf_dmabuf *dma)
 	if (!dma->sglen)
 		return 0;
 
-	dma_unmap_sg(q->dev, dma->sglist, dma->nr_pages, dma->direction);
+	dma_unmap_sg(q->dev, dma->sglist, dma->sglen, dma->direction);
 
 	kfree(dma->sglist);
 	dma->sglist = NULL;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
  2010-03-17 22:53 [PATCH v3] Support for zerocopy to DMA buffers Arnout Vandecappelle
  2010-03-17 22:53 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
@ 2010-03-17 22:53 ` Arnout Vandecappelle
  2010-03-18 11:58   ` Aguirre, Sergio
  2010-05-06  4:06   ` Mauro Carvalho Chehab
  1 sibling, 2 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-17 22:53 UTC (permalink / raw)
  To: linux-media, mchehab, arnout

videobuf_dma_init_user_locked() uses get_user_pages() to get the
virtual-to-physical address mapping for user-allocated memory.
However, the user-allocated memory may be non-pageable because it
is an I/O range or similar.  get_user_pages() fails with -EFAULT
in that case.

If the user-allocated memory is physically contiguous, the approach
of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
returned.
---
 drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
index 18aaf54..bd2d95d 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -136,6 +136,7 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
 {
 	unsigned long first,last;
 	int err, rw = 0;
+	struct vm_area_struct *vma;
 
 	dma->direction = direction;
 	switch (dma->direction) {
@@ -153,6 +154,23 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
 	last  = ((data+size-1) & PAGE_MASK) >> PAGE_SHIFT;
 	dma->offset   = data & ~PAGE_MASK;
 	dma->nr_pages = last-first+1;
+
+	/* In case the buffer is user-allocated and is actually an IO buffer for
+	   some other hardware, we cannot map pages for it.  It in fact behaves
+	   the same as an overlay. */
+	vma = find_vma (current->mm, data);
+	if (vma && (vma->vm_flags & VM_IO)) {
+		/* Only a single contiguous buffer is supported. */
+		if (vma->vm_end < data + size) {
+			dprintk(1, "init user: non-contiguous IO buffer.\n");
+			return -EFAULT; /* same error that get_user_pages() would give */
+		}
+		dma->bus_addr = (vma->vm_pgoff << PAGE_SHIFT) +	(data - vma->vm_start);
+		dprintk(1,"init user IO [0x%lx+0x%lx => %d pages at 0x%x]\n",
+			data, size, dma->nr_pages, dma->bus_addr);
+		return 0;
+	}
+
 	dma->pages = kmalloc(dma->nr_pages * sizeof(struct page*),
 			     GFP_KERNEL);
 	if (NULL == dma->pages)
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
  2010-03-17 22:53 ` [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory Arnout Vandecappelle
@ 2010-03-18 11:58   ` Aguirre, Sergio
  2010-05-06  4:06   ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 11+ messages in thread
From: Aguirre, Sergio @ 2010-03-18 11:58 UTC (permalink / raw)
  To: Arnout Vandecappelle, linux-media@vger.kernel.org,
	mchehab@infradead.org

Hi Arnout,

Just a very minor style comment.

> -----Original Message-----
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Arnout Vandecappelle
> Sent: Wednesday, March 17, 2010 5:53 PM
> To: linux-media@vger.kernel.org; mchehab@infradead.org; arnout@mind.be
> Subject: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-
> allocated memory
> 
> videobuf_dma_init_user_locked() uses get_user_pages() to get the
> virtual-to-physical address mapping for user-allocated memory.
> However, the user-allocated memory may be non-pageable because it
> is an I/O range or similar.  get_user_pages() fails with -EFAULT
> in that case.
> 
> If the user-allocated memory is physically contiguous, the approach
> of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
> returned.
> ---
>  drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf-dma-sg.c
> b/drivers/media/video/videobuf-dma-sg.c
> index 18aaf54..bd2d95d 100644
> --- a/drivers/media/video/videobuf-dma-sg.c
> +++ b/drivers/media/video/videobuf-dma-sg.c
> @@ -136,6 +136,7 @@ static int videobuf_dma_init_user_locked(struct
> videobuf_dmabuf *dma,
>  {
>  	unsigned long first,last;
>  	int err, rw = 0;
> +	struct vm_area_struct *vma;
> 
>  	dma->direction = direction;
>  	switch (dma->direction) {
> @@ -153,6 +154,23 @@ static int videobuf_dma_init_user_locked(struct
> videobuf_dmabuf *dma,
>  	last  = ((data+size-1) & PAGE_MASK) >> PAGE_SHIFT;
>  	dma->offset   = data & ~PAGE_MASK;
>  	dma->nr_pages = last-first+1;
> +
> +	/* In case the buffer is user-allocated and is actually an IO buffer
> for
> +	   some other hardware, we cannot map pages for it.  It in fact
> behaves
> +	   the same as an overlay. */

> +	vma = find_vma (current->mm, data);

Remove space before parenthesis above, so it becomes:

	vma = find_vma(current->mm, data);

Regards,
Sergio

> +	if (vma && (vma->vm_flags & VM_IO)) {
> +		/* Only a single contiguous buffer is supported. */
> +		if (vma->vm_end < data + size) {
> +			dprintk(1, "init user: non-contiguous IO buffer.\n");
> +			return -EFAULT; /* same error that get_user_pages()
> would give */
> +		}
> +		dma->bus_addr = (vma->vm_pgoff << PAGE_SHIFT) +	(data - vma-
> >vm_start);
> +		dprintk(1,"init user IO [0x%lx+0x%lx => %d pages at 0x%x]\n",
> +			data, size, dma->nr_pages, dma->bus_addr);
> +		return 0;
> +	}
> +
>  	dma->pages = kmalloc(dma->nr_pages * sizeof(struct page*),
>  			     GFP_KERNEL);
>  	if (NULL == dma->pages)
> --
> 1.6.3.3
> 
> --
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-17 22:53 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
@ 2010-03-24  5:43   ` Sakari Ailus
  2010-03-30 10:27     ` Arnout Vandecappelle
  0 siblings, 1 reply; 11+ messages in thread
From: Sakari Ailus @ 2010-03-24  5:43 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: linux-media, mchehab

Hi Arnout,

Thanks for the patch.

Arnout Vandecappelle wrote:
> videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
> a scatterlist element for every page.  However, this is not true for
> bus addresses, so other functions shouldn't rely on the length of the
> scatter list being equal to nr_pages.
> ---
>  drivers/media/video/videobuf-dma-sg.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
> index da1790e..18aaf54 100644
> --- a/drivers/media/video/videobuf-dma-sg.c
> +++ b/drivers/media/video/videobuf-dma-sg.c
> @@ -262,7 +262,7 @@ int videobuf_dma_sync(struct videobuf_queue *q, struct videobuf_dmabuf *dma)
>  	MAGIC_CHECK(dma->magic, MAGIC_DMABUF);
>  	BUG_ON(!dma->sglen);
>  
> -	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->nr_pages, dma->direction);
> +	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->sglen, dma->direction);
>  	return 0;
>  }

I think the same problem still exists --- dma->sglen is not initialised
anywhere, is it?

> @@ -272,7 +272,7 @@ int videobuf_dma_unmap(struct videobuf_queue* q,struct videobuf_dmabuf *dma)
>  	if (!dma->sglen)
>  		return 0;
>  
> -	dma_unmap_sg(q->dev, dma->sglist, dma->nr_pages, dma->direction);
> +	dma_unmap_sg(q->dev, dma->sglist, dma->sglen, dma->direction);
>  
>  	kfree(dma->sglist);
>  	dma->sglist = NULL;


-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
  2010-03-24  5:43   ` Sakari Ailus
@ 2010-03-30 10:27     ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-03-30 10:27 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, mchehab


On Wednesday 24 March 2010 06:43:22, Sakari Ailus wrote:
> Hi Arnout,
> 
> Thanks for the patch.
> 
> Arnout Vandecappelle wrote:
[snip]
> > -	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->nr_pages, dma->direction); 
> > +	dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->sglen, dma->direction);
> 
> I think the same problem still exists --- dma->sglen is not initialised
> anywhere, is it?

 Yes it is.
 In videobuf_dma_map (where dma->sglist is set), there are two conditions:

        if (dma->bus_addr) {
                dma->sglist = kmalloc(sizeof(struct scatterlist), GFP_KERNEL);
                if (NULL != dma->sglist) {
                        dma->sglen  = 1;
...
                }
        }
...
        if (!dma->bus_addr) {
                dma->sglen = dma_map_sg(q->dev, dma->sglist,
                                        dma->nr_pages, dma->direction);
...
        }


 Regards,
 Arnout
-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  31BB CF53 8660 6F88 345D  54CC A836 5879 20D7 CF43

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
  2010-03-17 22:53 ` [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory Arnout Vandecappelle
  2010-03-18 11:58   ` Aguirre, Sergio
@ 2010-05-06  4:06   ` Mauro Carvalho Chehab
  2010-05-20 19:22     ` Arnout Vandecappelle
  1 sibling, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2010-05-06  4:06 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: linux-media

Hi Arnout,

Arnout Vandecappelle wrote:
> videobuf_dma_init_user_locked() uses get_user_pages() to get the
> virtual-to-physical address mapping for user-allocated memory.
> However, the user-allocated memory may be non-pageable because it
> is an I/O range or similar.  get_user_pages() fails with -EFAULT
> in that case.
> 
> If the user-allocated memory is physically contiguous, the approach
> of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
> returned.
> ---
>  drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 

Your patch looked sane to my eyes. I just noticed one warning at the dprintk,
when compiled with 32 bits address space, and a few coding style issues.
I needed to rebase it also, due to some changes at videobuf.

However, you missed your SOB. Could you please send it? I'm enclosing
the version with my changes for you to sign.

---

V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
Date: Wed, 17 Mar 2010 22:53:05 -0000
From: Arnout Vandecappelle <arnout@mind.be>

videobuf_dma_init_user_locked() uses get_user_pages() to get the
virtual-to-physical address mapping for user-allocated memory.
However, the user-allocated memory may be non-pageable because it
is an I/O range or similar.  get_user_pages() fails with -EFAULT
in that case.

If the user-allocated memory is physically contiguous, the approach
of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
returned.

[mchehab@redhat.com: Fixed CodingStyle and warning at dprintk on i386]

---
drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
 drivers/media/video/videobuf-dma-sg.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--- work.orig/drivers/media/video/videobuf-dma-sg.c
+++ work/drivers/media/video/videobuf-dma-sg.c
@@ -145,6 +145,7 @@ static int videobuf_dma_init_user_locked
 {
 	unsigned long first, last;
 	int err, rw = 0;
+	struct vm_area_struct *vma;
 
 	dma->direction = direction;
 	switch (dma->direction) {
@@ -162,6 +163,25 @@ static int videobuf_dma_init_user_locked
 	last  = ((data+size-1) & PAGE_MASK) >> PAGE_SHIFT;
 	dma->offset   = data & ~PAGE_MASK;
 	dma->nr_pages = last-first+1;
+
+	/* In case the buffer is user-allocated and is actually an IO buffer for
+	   some other hardware, we cannot map pages for it.  It in fact behaves
+	   the same as an overlay. */
+	vma = find_vma(current->mm, data);
+	if (vma && (vma->vm_flags & VM_IO)) {
+		/* Only a single contiguous buffer is supported. */
+		if (vma->vm_end < data + size) {
+			dprintk(1, "init user: non-contiguous IO buffer.\n");
+			/* same error that get_user_pages() would give */
+			return -EFAULT;
+		}
+		dma->bus_addr = (vma->vm_pgoff << PAGE_SHIFT) +
+				(data - vma->vm_start);
+		dprintk(1, "init user IO [0x%lx+0x%lx => %d pages at 0x%llx]\n",
+			data, size, dma->nr_pages, (long long)dma->bus_addr);
+		return 0;
+	}
+
 	dma->pages = kmalloc(dma->nr_pages * sizeof(struct page *), GFP_KERNEL);
 	if (NULL == dma->pages)
 		return -ENOMEM;


-- 

Cheers,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
  2010-05-06  4:06   ` Mauro Carvalho Chehab
@ 2010-05-20 19:22     ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2010-05-20 19:22 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media


On Thursday 06 May 2010 06:06:21, Mauro Carvalho Chehab wrote:
> Hi Arnout,
> 
> Arnout Vandecappelle wrote:
> > videobuf_dma_init_user_locked() uses get_user_pages() to get the
> > virtual-to-physical address mapping for user-allocated memory.
> > However, the user-allocated memory may be non-pageable because it
> > is an I/O range or similar.  get_user_pages() fails with -EFAULT
> > in that case.
> > 
> > If the user-allocated memory is physically contiguous, the approach
> > of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
> > returned.
> > ---
> > 
> >  drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> Your patch looked sane to my eyes. I just noticed one warning at the
> dprintk, when compiled with 32 bits address space, and a few coding
> style issues. I needed to rebase it also, due to some changes at
> videobuf.
> 
> However, you missed your SOB. Could you please send it? I'm enclosing
> the version with my changes for you to sign.
> 
> ---
> 
> V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
> Date: Wed, 17 Mar 2010 22:53:05 -0000
> From: Arnout Vandecappelle <arnout@mind.be>
> 
> videobuf_dma_init_user_locked() uses get_user_pages() to get the
> virtual-to-physical address mapping for user-allocated memory.
> However, the user-allocated memory may be non-pageable because it
> is an I/O range or similar.  get_user_pages() fails with -EFAULT
> in that case.
> 
> If the user-allocated memory is physically contiguous, the approach
> of V4L2_MEMORY_OVERLAY can be used.  If it is not, -EFAULT is still
> returned.
> 
> [mchehab@redhat.com: Fixed CodingStyle and warning at dprintk on i386]
> 
> ---
> drivers/media/video/videobuf-dma-sg.c |   18 ++++++++++++++++++
>  drivers/media/video/videobuf-dma-sg.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> --- work.orig/drivers/media/video/videobuf-dma-sg.c
> +++ work/drivers/media/video/videobuf-dma-sg.c
> @@ -145,6 +145,7 @@ static int videobuf_dma_init_user_locked
>  {
>  	unsigned long first, last;
>  	int err, rw = 0;
> +	struct vm_area_struct *vma;
> 
>  	dma->direction = direction;
>  	switch (dma->direction) {
> @@ -162,6 +163,25 @@ static int videobuf_dma_init_user_locked
>  	last  = ((data+size-1) & PAGE_MASK) >> PAGE_SHIFT;
>  	dma->offset   = data & ~PAGE_MASK;
>  	dma->nr_pages = last-first+1;
> +
> +	/* In case the buffer is user-allocated and is actually an IO buffer
> for +	   some other hardware, we cannot map pages for it.  It in fact
> behaves +	   the same as an overlay. */
> +	vma = find_vma(current->mm, data);
> +	if (vma && (vma->vm_flags & VM_IO)) {
> +		/* Only a single contiguous buffer is supported. */
> +		if (vma->vm_end < data + size) {
> +			dprintk(1, "init user: non-contiguous IO buffer.\n");
> +			/* same error that get_user_pages() would give */
> +			return -EFAULT;
> +		}
> +		dma->bus_addr = (vma->vm_pgoff << PAGE_SHIFT) +
> +				(data - vma->vm_start);
> +		dprintk(1, "init user IO [0x%lx+0x%lx => %d pages at 0x%llx]\n",
> +			data, size, dma->nr_pages, (long long)dma->bus_addr);
> +		return 0;
> +	}
> +
>  	dma->pages = kmalloc(dma->nr_pages * sizeof(struct page *),
> GFP_KERNEL); if (NULL == dma->pages)
>  		return -ENOMEM;

Signed-off-by: Arnout Vandecappelle <arnout@mind.be>

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  31BB CF53 8660 6F88 345D  54CC A836 5879 20D7 CF43

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-05-20 19:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-17 22:53 [PATCH v3] Support for zerocopy to DMA buffers Arnout Vandecappelle
2010-03-17 22:53 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
2010-03-24  5:43   ` Sakari Ailus
2010-03-30 10:27     ` Arnout Vandecappelle
2010-03-17 22:53 ` [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory Arnout Vandecappelle
2010-03-18 11:58   ` Aguirre, Sergio
2010-05-06  4:06   ` Mauro Carvalho Chehab
2010-05-20 19:22     ` Arnout Vandecappelle
  -- strict thread matches above, loose matches on Subject: below --
2010-03-03 14:12 Support for zerocopy to DSP on OMAP3 Arnout Vandecappelle
2010-03-04 16:00 ` [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen Arnout Vandecappelle
2010-03-17 20:57   ` Sakari Ailus
2010-03-17 22:50     ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox