Re: [PATCH 10 of 20] ipath - support for userspace apps using core driver

[Nick Piggin <nickpiggin@yahoo.com.au>]

Roland Dreier wrote:
>     Andrew> Someone left PG_private set on this page (!)
> 
> How does PG_private get set?  Could doing a > 1 page kmalloc set it
> (because we end up with a compound page)?
> 

I think you should avoid slab allocations for user mappings because
you don't know that it will do compound pages properly, depending on
slab implementation details.

Also, slab lifetimes are not controlled by page refcounting, so when
your driver decides to kfree the memory, a process could still have
refs on the pages via get_user_pages, which might be reused for
unrelated slab allocations.

>     Andrew> You need to decide who "owns" these pages.  Once that's
>     Andrew> decided, it tells you who should release them.
> 
>     [... really good guide to mapping pages into userspace snipped ...]
> 
> How about the case where one wants to map pages from
> dma_alloc_coherent() into userspace?  It seems one should do
> get_page() in .nopage, and then the driver can do dma_free_coherent()
> when the vma is released.
> 

I think so, provided you set VM_IO on the vma. You need VM_IO to
ensure that get_user_pages callers can't hijack your page's lifetime
rules (Mr. Christmas tree has been carefully engineered for optimal
confusion and best bug count).

> Or maybe it's just simpler to use vm_insert_page() in the .mmap method
> and not try to be fancy with .nopage?
> 

That's not being particularly fancy. Either way you need to set VM_IO
on the vma, and make sure the correct memory freeing function (in this
case dma_free_coherent) is called sometime after the last userspace
ref goes away.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com