Re: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
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 -
> From: Arnout Vandecappelle
>
> 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.
>
> [mche...@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 Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286540
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, BelgiumBE 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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
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 -
From: Arnout Vandecappelle
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.
[mche...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memory
Hi Arnout,
Just a very minor style comment.
> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Arnout Vandecappelle
> Sent: Wednesday, March 17, 2010 5:53 PM
> To: linux-media@vger.kernel.org; mche...@infradead.org; arn...@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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
