Hi Marek,
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.osciak@xxxxxxxxxxx>
> > > > >
> > > > > Add generic memory handling routines for userspace pointer
> > > > > handling, contiguous memory verification and mapping.
> > > > >
> > > > > Signed-off-by: Pawel Osciak <p.osciak@xxxxxxxxxxx>
> > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > > > > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > > > > CC: Pawel Osciak <pawel@xxxxxxxxxx>
> >
> > [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.
> > > > > +/**
> > > > > + * 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 ?
--
Regards,
Laurent Pinchart
--
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
