Marek Szyprowski wrote:
2010/11/20 Marek Szyprowski <m.szyprow...@samsung.com>:
> From: Pawel Osciak <p.osc...@samsung.com>
>
> Add support for the CMA contiguous memory allocator to videobuf2.
>
> Signed-off-by: Pawel Osciak <p.osc...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> CC: Pawel Osciak <pa...@osciak.com>
> ---

Hi Marek,

<snip>

> +static void *vb2_cma_get_userptr(unsigned long vaddr, unsigned long size)

static void *vb2_cma_get_userptr(unsigned long vaddr, unsigned long size, 
                                                const struct vb2_alloc_ctx
*alloc_ctx)
alloc_ctx can be useful for many cases.

> +{
> +       struct vb2_cma_buf *buf;
> +       unsigned long paddr = 0;
> +       int ret;
> +
> +       buf = kzalloc(sizeof *buf, GFP_KERNEL);
> +       if (!buf)
> +               return ERR_PTR(-ENOMEM);
> +
> +       buf->vma = vb2_get_userptr(vaddr);

vb2_get_vma looks good instead of vb2_get_userptr.
How do you think about this ?

> +       if (!buf->vma) {
> +               printk(KERN_ERR "Failed acquiring VMA for vaddr
0x%08lx\n",
> +                               vaddr);
> +               ret = -EINVAL;
> +               goto done;
> +       }
> +
> +       ret = vb2_contig_verify_userptr(buf->vma, vaddr, size, &paddr);
> +       if (ret) {
> +               vb2_put_userptr(buf->vma);
> +               goto done;
> +       }
> +

In vb2_contig_verify_userptr, vma is re-found via find_vma although it has
been checked after vb2_get_userptr.
Why double checking is needed ?

> +       buf->size = size;
> +       buf->paddr = paddr;
> +
> +       return buf;
> +
> +done:
> +       kfree(buf);
> +       return ERR_PTR(ret);
> +}
> +

<snip>

BRs,

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

Reply via email to