Re: [PATCH] videobuf2-dma-sg: Support io userptr operations on io memory

Ricardo Ribalda Delgado
2013-11-25T16:23:05Z

Hello Marek

Could you review the patch? Is there something that needs to be fixed?

Thanks!

On Mon, Nov 25, 2013 at 4:41 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
>
> On 2013-11-11 12:36, Matthias Wächter wrote:
>>
>> > @@ -180,7 +186,26 @@ static void *vb2_dma_sg_get_userptr(void
>> > *alloc_ctx, unsigned long vaddr,
>> > if (!buf->pages)
>> > return NULL;
>> >
>> > - num_pages_from_user = get_user_pages(current, current->mm,
>> > + buf->vma = find_vma(current->mm, vaddr);
>> > + if (!buf->vma) {
>> > + dprintk(1, "no vma for address %lu\n", vaddr);
>> > + return NULL;
>> > + }
>> > +
>> > + if (vma_is_io(buf->vma)) {
>> > + for (num_pages_from_user = 0;
>> > + num_pages_from_user < buf->num_pages;
>> > + ++num_pages_from_user, vaddr += PAGE_SIZE) {
>> > + unsigned long pfn;
>> > +
>> > + if (follow_pfn(buf->vma, vaddr, &pfn)) {
>> > + dprintk(1, "no page for address %lu\n",
>> > vaddr);
>> > + break;
>> > + }
>> > + buf->pages[num_pages_from_user] =
>> > pfn_to_page(pfn);
>> > + }
>> > + } else
>> > + num_pages_from_user = get_user_pages(current, current->mm,
>> > vaddr & PAGE_MASK,
>> > buf->num_pages,
>> > write,
>>
>> Can you safely assume that your userptr will cover only one vma? At least,
>> get_user_pages (calling __get_user_pages) does not assume that and calls
>> find_vma() whenever vma->vm_end is reached.
>
>
> We care only about io mappings which cover only one vma. Such mappings
> are created by other device drivers and can be used mainly for
> zero-copy buffer sharing between multimedia devices. Although it is
> technically possible to provide code for multiple vma, there will be
> no real use case for it.
>
> Best regards
> --
> Marek Szyprowski
> Samsung R&D Institute Poland
>

--
Ricardo Ribalda