Hello,
On Wednesday, December 01, 2010 9:36 AM Jonghun Han wrote:
> Marek Szyprowski wrote:
> 2010/11/20 Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>:
> > From: Pawel Osciak <p.osciak@xxxxxxxxxxx>
> >
> > Add support for the CMA contiguous memory allocator to videobuf2.
> >
> > Signed-off-by: Pawel Osciak <p.osciak@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > CC: Pawel Osciak <pawel@xxxxxxxxxx>
> > ---
>
> 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.
Yes, right, I will add it in the next version.
> > +{
> > + 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 ?
Right, this will make the code easier to understand. Thanks for the hint!
> > + 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 ?
I'm not sure. I must check this core again, maybe it can be simplified a bit.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
--
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
