diff mbox

media: vb2: dma-sg allocator: change scatterlist allocation method

Message ID 1312964617-3192-1-git-send-email-m.szyprowski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Szyprowski Aug. 10, 2011, 8:23 a.m. UTC
From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>

Scatter-gather lib provides a helper functions to allocate scatter list,
so there is no need to use vmalloc for it. sg_alloc_table() splits
allocation into page size chunks and links them together into a chain.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/video/videobuf2-dma-sg.c |   54 +++++++++++++++++++------------
 1 files changed, 33 insertions(+), 21 deletions(-)

Comments

Laurent Pinchart Aug. 12, 2011, 9:54 p.m. UTC | #1
Hi,

On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> 
> Scatter-gather lib provides a helper functions to allocate scatter list,
> so there is no need to use vmalloc for it. sg_alloc_table() splits
> allocation into page size chunks and links them together into a chain.

Last time I check ARM platforms didn't support SG list chaining. Has that been 
fixed ?

> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> CC: Pawel Osciak <pawel@osciak.com>
> ---
>  drivers/media/video/videobuf2-dma-sg.c |   54
> +++++++++++++++++++------------ 1 files changed, 33 insertions(+), 21
> deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-dma-sg.c
> b/drivers/media/video/videobuf2-dma-sg.c index 065f468..e1158f9 100644
> --- a/drivers/media/video/videobuf2-dma-sg.c
> +++ b/drivers/media/video/videobuf2-dma-sg.c
> @@ -36,6 +36,8 @@ static void vb2_dma_sg_put(void *buf_priv);
>  static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
>  {
>  	struct vb2_dma_sg_buf *buf;
> +	struct sg_table sgt;
> +	struct scatterlist *sl;
>  	int i;
> 
>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> @@ -48,23 +50,21 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size) buf->sg_desc.size = size;
>  	buf->sg_desc.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> 
> -	buf->sg_desc.sglist = vzalloc(buf->sg_desc.num_pages *
> -				      sizeof(*buf->sg_desc.sglist));
> -	if (!buf->sg_desc.sglist)
> +	if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
>  		goto fail_sglist_alloc;
> -	sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
> +	buf->sg_desc.sglist = sgt.sgl;
> 
>  	buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
>  			     GFP_KERNEL);
>  	if (!buf->pages)
>  		goto fail_pages_array_alloc;
> 
> -	for (i = 0; i < buf->sg_desc.num_pages; ++i) {
> -		buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN);
> +	for_each_sg(buf->sg_desc.sglist, sl, buf->sg_desc.num_pages, i) {
> +		buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO |
> +					   __GFP_NOWARN);
>  		if (NULL == buf->pages[i])
>  			goto fail_pages_alloc;
> -		sg_set_page(&buf->sg_desc.sglist[i],
> -			    buf->pages[i], PAGE_SIZE, 0);
> +		sg_set_page(sl, buf->pages[i], PAGE_SIZE, 0);
>  	}
> 
>  	buf->handler.refcount = &buf->refcount;
> @@ -89,7 +89,7 @@ fail_pages_alloc:
>  	kfree(buf->pages);
> 
>  fail_pages_array_alloc:
> -	vfree(buf->sg_desc.sglist);
> +	sg_free_table(&sgt);
> 
>  fail_sglist_alloc:
>  	kfree(buf);
> @@ -99,6 +99,7 @@ fail_sglist_alloc:
>  static void vb2_dma_sg_put(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table sgt;
>  	int i = buf->sg_desc.num_pages;
> 
>  	if (atomic_dec_and_test(&buf->refcount)) {
> @@ -106,7 +107,9 @@ static void vb2_dma_sg_put(void *buf_priv)
>  			buf->sg_desc.num_pages);
>  		if (buf->vaddr)
>  			vm_unmap_ram(buf->vaddr, buf->sg_desc.num_pages);
> -		vfree(buf->sg_desc.sglist);
> +		sgt.sgl = buf->sg_desc.sglist;
> +		sgt.orig_nents = sgt.nents = i;
> +		sg_free_table(&sgt);
>  		while (--i >= 0)
>  			__free_page(buf->pages[i]);
>  		kfree(buf->pages);
> @@ -118,6 +121,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, unsigned long size, int write)
>  {
>  	struct vb2_dma_sg_buf *buf;
> +	struct sg_table sgt;
> +	struct scatterlist *sl;
>  	unsigned long first, last;
>  	int num_pages_from_user, i;
> 
> @@ -134,12 +139,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, last  = ((vaddr + size - 1) & PAGE_MASK) >>
> PAGE_SHIFT;
>  	buf->sg_desc.num_pages = last - first + 1;
> 
> -	buf->sg_desc.sglist = vzalloc(
> -		buf->sg_desc.num_pages * sizeof(*buf->sg_desc.sglist));
> -	if (!buf->sg_desc.sglist)
> +	if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
>  		goto userptr_fail_sglist_alloc;
> -
> -	sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
> +	buf->sg_desc.sglist = sgt.sgl;
> 
>  	buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
>  			     GFP_KERNEL);
> @@ -158,12 +160,12 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (num_pages_from_user != buf->sg_desc.num_pages)
>  		goto userptr_fail_get_user_pages;
> 
> -	sg_set_page(&buf->sg_desc.sglist[0], buf->pages[0],
> -		    PAGE_SIZE - buf->offset, buf->offset);
> +	sl = buf->sg_desc.sglist;
> +	sg_set_page(sl, buf->pages[0], PAGE_SIZE - buf->offset, buf->offset);
>  	size -= PAGE_SIZE - buf->offset;
>  	for (i = 1; i < buf->sg_desc.num_pages; ++i) {
> -		sg_set_page(&buf->sg_desc.sglist[i], buf->pages[i],
> -			    min_t(size_t, PAGE_SIZE, size), 0);
> +		sl = sg_next(sl);
> +		sg_set_page(sl, buf->pages[i], min_t(size_t, PAGE_SIZE, size), 0);
>  		size -= min_t(size_t, PAGE_SIZE, size);
>  	}
>  	return buf;
> @@ -176,7 +178,7 @@ userptr_fail_get_user_pages:
>  	kfree(buf->pages);
> 
>  userptr_fail_pages_array_alloc:
> -	vfree(buf->sg_desc.sglist);
> +	sg_free_table(&sgt);
> 
>  userptr_fail_sglist_alloc:
>  	kfree(buf);
> @@ -190,6 +192,8 @@ userptr_fail_sglist_alloc:
>  static void vb2_dma_sg_put_userptr(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table sgt;
> +
>  	int i = buf->sg_desc.num_pages;
> 
>  	printk(KERN_DEBUG "%s: Releasing userspace buffer of %d pages\n",
> @@ -201,7 +205,9 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>  			set_page_dirty_lock(buf->pages[i]);
>  		put_page(buf->pages[i]);
>  	}
> -	vfree(buf->sg_desc.sglist);
> +	sgt.sgl = buf->sg_desc.sglist;
> +	sgt.orig_nents = sgt.nents = buf->sg_desc.num_pages;
> +	sg_free_table(&sgt);
>  	kfree(buf->pages);
>  	kfree(buf);
>  }
> @@ -218,6 +224,12 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
>  					-1,
>  					PAGE_KERNEL);
> 
> +	if (!buf->vaddr) {
> +		printk(KERN_ERR "Cannot map buffer memory "
> +				"into kernel address space\n");
> +		return NULL;
> +	}
> +
>  	/* add offset in case userptr is not page-aligned */
>  	return buf->vaddr + buf->offset;
>  }
Marek Szyprowski Aug. 16, 2011, 5:35 a.m. UTC | #2
Hello,

On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:

> On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> >
> > Scatter-gather lib provides a helper functions to allocate scatter list,
> > so there is no need to use vmalloc for it. sg_alloc_table() splits
> > allocation into page size chunks and links them together into a chain.
> 
> Last time I check ARM platforms didn't support SG list chaining. Has that been
> fixed ?

DMA-mapping code for ARM platform use for_each_sg() macro which has no problems
with chained SG lists.

(snipped)

Best regards
Laurent Pinchart Aug. 16, 2011, 8:41 a.m. UTC | #3
Hi Marek,

On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > 
> > > Scatter-gather lib provides a helper functions to allocate scatter
> > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > splits allocation into page size chunks and links them together into a
> > > chain.
> > 
> > Last time I check ARM platforms didn't support SG list chaining. Has that
> > been fixed ?
> 
> DMA-mapping code for ARM platform use for_each_sg() macro which has no
> problems with chained SG lists.

for_each_sg() is fine, but sg_alloc_table() doesn't seem to be. 
__sg_alloc_table(), called from sg_alloc_table(), starts with

#ifndef ARCH_HAS_SG_CHAIN
        BUG_ON(nents > max_ents);
#endif

It also calls sg_chain() internally, which starts with

#ifndef ARCH_HAS_SG_CHAIN
        BUG();
#endif

ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set. That's 
a boolean Kconfig option that is currently never set.
Marek Szyprowski Aug. 16, 2011, 10:34 a.m. UTC | #4
Hello,

On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:

> On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > >
> > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > splits allocation into page size chunks and links them together into a
> > > > chain.
> > >
> > > Last time I check ARM platforms didn't support SG list chaining. Has that
> > > been fixed ?
> >
> > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > problems with chained SG lists.
> 
> for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> __sg_alloc_table(), called from sg_alloc_table(), starts with
> 
> #ifndef ARCH_HAS_SG_CHAIN
>         BUG_ON(nents > max_ents);
> #endif
> 
> It also calls sg_chain() internally, which starts with
> 
> #ifndef ARCH_HAS_SG_CHAIN
>         BUG();
> #endif
> 
> ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set. That's
> a boolean Kconfig option that is currently never set.

Right, I wasn't aware of that, but it still doesn't look like an issue. The only

client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If one
needs dma-sg allocator on ARM, he should follow the suggestion from the 
74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.

Best regards
Laurent Pinchart Aug. 16, 2011, 11:01 a.m. UTC | #5
Hi Marek,

On Tuesday 16 August 2011 12:34:56 Marek Szyprowski wrote:
> On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:
> > On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > > > 
> > > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > > splits allocation into page size chunks and links them together
> > > > > into a chain.
> > > > 
> > > > Last time I check ARM platforms didn't support SG list chaining. Has
> > > > that been fixed ?
> > > 
> > > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > > problems with chained SG lists.
> > 
> > for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> > __sg_alloc_table(), called from sg_alloc_table(), starts with
> > 
> > #ifndef ARCH_HAS_SG_CHAIN
> > 
> >         BUG_ON(nents > max_ents);
> > 
> > #endif
> > 
> > It also calls sg_chain() internally, which starts with
> > 
> > #ifndef ARCH_HAS_SG_CHAIN
> > 
> >         BUG();
> > 
> > #endif
> > 
> > ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set.
> > That's a boolean Kconfig option that is currently never set.
> 
> Right, I wasn't aware of that, but it still doesn't look like an issue. The
> only client of dma-sg allocator is marvell-ccic, which is used on x86
> systems. If one needs dma-sg allocator on ARM, he should follow the
> suggestion from the 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.

Won't the dma-sg allocator be the right one for systems with an IOMMU ? If so 
we'll soon run into this issue. I'd like to port the OMAP3 ISP driver to 
videobuf2.
Marek Szyprowski Aug. 16, 2011, 11:48 a.m. UTC | #6
Hello,

On Tuesday, August 16, 2011 1:02 PM Laurent Pinchart wrote:

> On Tuesday 16 August 2011 12:34:56 Marek Szyprowski wrote:
> > On Tuesday, August 16, 2011 10:42 AM Laurent Pinchart wrote:
> > > On Tuesday 16 August 2011 07:35:05 Marek Szyprowski wrote:
> > > > On Friday, August 12, 2011 11:55 PM Laurent Pinchart wrote:
> > > > > On Wednesday 10 August 2011 10:23:37 Marek Szyprowski wrote:
> > > > > > From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> > > > > >
> > > > > > Scatter-gather lib provides a helper functions to allocate scatter
> > > > > > list, so there is no need to use vmalloc for it. sg_alloc_table()
> > > > > > splits allocation into page size chunks and links them together
> > > > > > into a chain.
> > > > >
> > > > > Last time I check ARM platforms didn't support SG list chaining. Has
> > > > > that been fixed ?
> > > >
> > > > DMA-mapping code for ARM platform use for_each_sg() macro which has no
> > > > problems with chained SG lists.
> > >
> > > for_each_sg() is fine, but sg_alloc_table() doesn't seem to be.
> > > __sg_alloc_table(), called from sg_alloc_table(), starts with
> > >
> > > #ifndef ARCH_HAS_SG_CHAIN
> > >
> > >         BUG_ON(nents > max_ents);
> > >
> > > #endif
> > >
> > > It also calls sg_chain() internally, which starts with
> > >
> > > #ifndef ARCH_HAS_SG_CHAIN
> > >
> > >         BUG();
> > >
> > > #endif
> > >
> > > ARCH_HAS_SG_CHAIN is defined on ARM if CONFIG_ARM_HAS_SG_CHAIN is set.
> > > That's a boolean Kconfig option that is currently never set.
> >
> > Right, I wasn't aware of that, but it still doesn't look like an issue. The
> > only client of dma-sg allocator is marvell-ccic, which is used on x86
> > systems. If one needs dma-sg allocator on ARM, he should follow the
> > suggestion from the 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
> 
> Won't the dma-sg allocator be the right one for systems with an IOMMU ? If so
> we'll soon run into this issue. I'd like to port the OMAP3 ISP driver to
> videobuf2.

Nope. The correct place for IOMMU is dma-contig allocator. In theory DMA-mapping
functions SHOULD hide the presence of the IOMMU from the driver. The driver
would
only need to get the 'dma_address' of the buffer and write it to the respective
multimedia device register.

The proof-of-concept of such solution for kernel allocated buffer has been 
presented in my first ARM dma-mapping patches: 
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/dma-
mapping 

User pointer buffers will need a bit more work to get them working correctly for
both iommu and non-iommu cases. For IOMMU dma_map_sg() will just fill the
scatter
list with just one dma_addr entry and it will contain the address of the buffer
in driver's io address space. Non-IOMMU needs some more tweaking (also in the
DMA-mapping)

IMHO the best way to porting OMAP3 ISP to videobuf2 is to first create a custom
memory allocator that working with OMAP3 IOMMU directly and then
adapt/merge/port
it to DMA-mapping based approach.

You can also refer to our first approaches with videobuf2-dma-iommu allocator:
http://lwn.net/Articles/439175/ 
Especially the discussion in that thread is really important for understanding
how the IOMMU should be integrated in the Linux kernel.

Best regards
Jonathan Corbet Aug. 17, 2011, 12:03 a.m. UTC | #7
On Tue, 16 Aug 2011 12:34:56 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> Right, I wasn't aware of that, but it still doesn't look like an issue. The only
> 
> client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If one
> needs dma-sg allocator on ARM, he should follow the suggestion from the 
> 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.

Um...that driver runs on ARM, actually - the controller is part of the
ARMADA 610 SoC...

For the OLPC 1.75 there will never be a scatterlist longer than one
page, I don't think.  That controller can do HD, though, so longer
lists are possible in the future.

jon
--
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
Marek Szyprowski Aug. 17, 2011, 8:51 a.m. UTC | #8
Hello,

On Wednesday, August 17, 2011 2:03 AM Jonathan Corbet wrote:

> On Tue, 16 Aug 2011 12:34:56 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> > Right, I wasn't aware of that, but it still doesn't look like an issue. The
only
> >
> > client of dma-sg allocator is marvell-ccic, which is used on x86 systems. If
one
> > needs dma-sg allocator on ARM, he should follow the suggestion from the
> > 74facffeca3795ffb5cf8898f5859fbb822e4c5d commit message.
> 
> Um...that driver runs on ARM, actually - the controller is part of the
> ARMADA 610 SoC...

Ups, I'm really sorry, it looks that I mixed something. I thought that OLPCs are

only x86 based.

> For the OLPC 1.75 there will never be a scatterlist longer than one
> page, I don't think.  That controller can do HD, though, so longer
> lists are possible in the future.

Ok, I see. Do you think it would be possible to ask PXA/MMP platform developers
to 
review all the drivers that can be used on that platform and enable scatter-list

chaining for this arm sub-arch? If this is a problem then we will have to delay
this
sg chaining patch.

Best regards
diff mbox

Patch

diff --git a/drivers/media/video/videobuf2-dma-sg.c b/drivers/media/video/videobuf2-dma-sg.c
index 065f468..e1158f9 100644
--- a/drivers/media/video/videobuf2-dma-sg.c
+++ b/drivers/media/video/videobuf2-dma-sg.c
@@ -36,6 +36,8 @@  static void vb2_dma_sg_put(void *buf_priv);
 static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
 {
 	struct vb2_dma_sg_buf *buf;
+	struct sg_table sgt;
+	struct scatterlist *sl;
 	int i;
 
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
@@ -48,23 +50,21 @@  static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size)
 	buf->sg_desc.size = size;
 	buf->sg_desc.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
 
-	buf->sg_desc.sglist = vzalloc(buf->sg_desc.num_pages *
-				      sizeof(*buf->sg_desc.sglist));
-	if (!buf->sg_desc.sglist)
+	if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
 		goto fail_sglist_alloc;
-	sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
+	buf->sg_desc.sglist = sgt.sgl;
 
 	buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
 			     GFP_KERNEL);
 	if (!buf->pages)
 		goto fail_pages_array_alloc;
 
-	for (i = 0; i < buf->sg_desc.num_pages; ++i) {
-		buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN);
+	for_each_sg(buf->sg_desc.sglist, sl, buf->sg_desc.num_pages, i) {
+		buf->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO |
+					   __GFP_NOWARN);
 		if (NULL == buf->pages[i])
 			goto fail_pages_alloc;
-		sg_set_page(&buf->sg_desc.sglist[i],
-			    buf->pages[i], PAGE_SIZE, 0);
+		sg_set_page(sl, buf->pages[i], PAGE_SIZE, 0);
 	}
 
 	buf->handler.refcount = &buf->refcount;
@@ -89,7 +89,7 @@  fail_pages_alloc:
 	kfree(buf->pages);
 
 fail_pages_array_alloc:
-	vfree(buf->sg_desc.sglist);
+	sg_free_table(&sgt);
 
 fail_sglist_alloc:
 	kfree(buf);
@@ -99,6 +99,7 @@  fail_sglist_alloc:
 static void vb2_dma_sg_put(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table sgt;
 	int i = buf->sg_desc.num_pages;
 
 	if (atomic_dec_and_test(&buf->refcount)) {
@@ -106,7 +107,9 @@  static void vb2_dma_sg_put(void *buf_priv)
 			buf->sg_desc.num_pages);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->sg_desc.num_pages);
-		vfree(buf->sg_desc.sglist);
+		sgt.sgl = buf->sg_desc.sglist;
+		sgt.orig_nents = sgt.nents = i;
+		sg_free_table(&sgt);
 		while (--i >= 0)
 			__free_page(buf->pages[i]);
 		kfree(buf->pages);
@@ -118,6 +121,8 @@  static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 				    unsigned long size, int write)
 {
 	struct vb2_dma_sg_buf *buf;
+	struct sg_table sgt;
+	struct scatterlist *sl;
 	unsigned long first, last;
 	int num_pages_from_user, i;
 
@@ -134,12 +139,9 @@  static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
 	buf->sg_desc.num_pages = last - first + 1;
 
-	buf->sg_desc.sglist = vzalloc(
-		buf->sg_desc.num_pages * sizeof(*buf->sg_desc.sglist));
-	if (!buf->sg_desc.sglist)
+	if (sg_alloc_table(&sgt, buf->sg_desc.num_pages, GFP_KERNEL))
 		goto userptr_fail_sglist_alloc;
-
-	sg_init_table(buf->sg_desc.sglist, buf->sg_desc.num_pages);
+	buf->sg_desc.sglist = sgt.sgl;
 
 	buf->pages = kzalloc(buf->sg_desc.num_pages * sizeof(struct page *),
 			     GFP_KERNEL);
@@ -158,12 +160,12 @@  static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (num_pages_from_user != buf->sg_desc.num_pages)
 		goto userptr_fail_get_user_pages;
 
-	sg_set_page(&buf->sg_desc.sglist[0], buf->pages[0],
-		    PAGE_SIZE - buf->offset, buf->offset);
+	sl = buf->sg_desc.sglist;
+	sg_set_page(sl, buf->pages[0], PAGE_SIZE - buf->offset, buf->offset);
 	size -= PAGE_SIZE - buf->offset;
 	for (i = 1; i < buf->sg_desc.num_pages; ++i) {
-		sg_set_page(&buf->sg_desc.sglist[i], buf->pages[i],
-			    min_t(size_t, PAGE_SIZE, size), 0);
+		sl = sg_next(sl);
+		sg_set_page(sl, buf->pages[i], min_t(size_t, PAGE_SIZE, size), 0);
 		size -= min_t(size_t, PAGE_SIZE, size);
 	}
 	return buf;
@@ -176,7 +178,7 @@  userptr_fail_get_user_pages:
 	kfree(buf->pages);
 
 userptr_fail_pages_array_alloc:
-	vfree(buf->sg_desc.sglist);
+	sg_free_table(&sgt);
 
 userptr_fail_sglist_alloc:
 	kfree(buf);
@@ -190,6 +192,8 @@  userptr_fail_sglist_alloc:
 static void vb2_dma_sg_put_userptr(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table sgt;
+
 	int i = buf->sg_desc.num_pages;
 
 	printk(KERN_DEBUG "%s: Releasing userspace buffer of %d pages\n",
@@ -201,7 +205,9 @@  static void vb2_dma_sg_put_userptr(void *buf_priv)
 			set_page_dirty_lock(buf->pages[i]);
 		put_page(buf->pages[i]);
 	}
-	vfree(buf->sg_desc.sglist);
+	sgt.sgl = buf->sg_desc.sglist;
+	sgt.orig_nents = sgt.nents = buf->sg_desc.num_pages;
+	sg_free_table(&sgt);
 	kfree(buf->pages);
 	kfree(buf);
 }
@@ -218,6 +224,12 @@  static void *vb2_dma_sg_vaddr(void *buf_priv)
 					-1,
 					PAGE_KERNEL);
 
+	if (!buf->vaddr) {
+		printk(KERN_ERR "Cannot map buffer memory "
+				"into kernel address space\n");
+		return NULL;
+	}
+
 	/* add offset in case userptr is not page-aligned */
 	return buf->vaddr + buf->offset;
 }