Hello,
On Monday, November 29, 2010 3:37 PM Laurent Pinchart wrote:
> On Monday 29 November 2010 15:27:00 Marek Szyprowski wrote:
> > Hello,
> >
> > On Monday, November 29, 2010 2:41 PM Laurent Pinchart wrote:
> > > Hi Marek,
> > >
> > > On Monday 29 November 2010 14:24:35 Marek Szyprowski wrote:
> > > > On Monday, November 29, 2010 10:39 AM Laurent Pinchart wrote:
> > > > > On Friday 19 November 2010 16:55:39 Marek Szyprowski wrote:
> > > > > > From: Pawel Osciak <p.osc...@samsung.com>
> > > > > >
> > > > > > Add generic memory handling routines for userspace pointer
> > > > > > handling, contiguous memory verification and mapping.
> > > > > >
> > > > > > Signed-off-by: Pawel Osciak <p.osc...@samsung.com>
> > > > > > Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> > > > > > Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> > > > > > CC: Pawel Osciak <pa...@osciak.com>
> > >
> > > [snip]
> > >
> > > > > > + if (ret) {
> > > > > > + printk(KERN_ERR "Invalid userspace address\n");
> > > > > > + goto done;
> > > > > > + }
> > > > > > +
> > > > > > + *paddr = (curr_pfn << PAGE_SHIFT) + offset;
> > > > > > +
> > > > > > + for (i = 1; i < num_pages; ++i) {
> > > > > > + prev_pfn = curr_pfn;
> > > > > > + vaddr += PAGE_SIZE;
> > > > > > +
> > > > > > + ret = follow_pfn(vma, vaddr, &curr_pfn);
> > > > > > + if (ret || curr_pfn != prev_pfn + 1) {
> > > > > > + printk(KERN_ERR "Invalid userspace address\n");
> > > > > > + ret = -EINVAL;
> > > > >
> > > > > I would use -EFAULT when curr_pfn != prev_pfn + 1.
> > > >
> > > > I don't think that this situation desires for a EFAULT error code.
> > > > EINVAL is imho enough to let application know that this memory cannot
> > > > be used for the userptr buffer.
> > >
> > > -EINVAL is used quite a lot already to report many different error
> > > conditions. -EFAULT would let application know that the issue is about
> > > the pointer address as opposed to the buffer index for instance.
> >
> > The application will not notice it probably in this case, because a default
> > EFAULT handler will kill it in most of the cases ;)
>
> Will it ? Where ? ioctls can return -EFAULT.
Ok. You are right. I mixed something. Please ignore my previous comment.
> > > > > > +/**
> > > > > > + * vb2_get_userptr() - acquire an area pointed to by userspace
> > > > > > addres vaddr + * @vaddr: virtual userspace address to the given
> > > > > > area + *
> > > > > > + * This function attempts to acquire an area mapped in the
> > > > > > userspace for + * the duration of a hardware operation.
> > > > > > + *
> > > > > > + * Returns a virtual memory region associated with the given vaddr
> > > > > > on success + * or NULL.
> > > > > > + */
> > > > > > +struct vm_area_struct *vb2_get_userptr(unsigned long vaddr)
> > > > > > +{
> > > > > > + struct mm_struct *mm = current->mm;
> > > > > > + struct vm_area_struct *vma;
> > > > > > + struct vm_area_struct *vma_copy;
> > > > > > +
> > > > > > + vma_copy = kmalloc(sizeof(struct vm_area_struct), GFP_KERNEL);
> > > > > > + if (vma_copy == NULL)
> > > > > > + return NULL;
> > > > > > +
> > > > > > + down_read(&mm->mmap_sem);
> > > > > > +
> > > > > > + vma = find_vma(mm, vaddr);
> > > > > > + if (!vma)
> > > > > > + goto done;
> > > > > > +
> > > > > > + if (vma->vm_ops && vma->vm_ops->open)
> > > > > > + vma->vm_ops->open(vma);
> > > > > > +
> > > > > > + if (vma->vm_file)
> > > > > > + get_file(vma->vm_file);
> > > > >
> > > > > Could you explain what this is for ?
> > > >
> > > > This is called when you access physically contiguous memory from
> > > > different device which has been earlier mmaped by the application.
> > > > This way applications uses userptr mode with dmacontig. When doing
> > > > this, you must ensure somehow that this memory won't be released/freed
> > > > before you finish. This is achieved by calling open() callback and
> > > > increasing the use count of the mmaped file. Same things happen when
> > > > your application calls fork() and a new process gets access to your
> > > > mmaped device.
> > >
> > > Good point.
> > >
> > > Don't you also need to pin the pages to memory with get_user_pages() ?
> >
> > Nope. get_user_pages() doesn't support pfn-type memory at all (it just
> > fails this that case). PFN-type memory cannot be swapped out anyway so it
> > not a problem here.
>
> But vb2_get_userptr() can be used on non-PFN memory, can't it ?
I thought that this function is used only by dma-coherent and cma allocators.
dma-sg and iommu allocators will definitely call get_user_pages().
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
