david at gibson.dropbear.id.au
Thu Sep 12 13:26:40 EST 2002
On Wed, Sep 11, 2002 at 02:45:31PM -0700, Todd Poynor wrote:
> The APIs for consistent_free have diverged between PPC and ARM. It has,
> at least in the past, been a goal to keep the APIs in sync.
That's the least of our worries - the current consistent_alloc() is
badly broken. It breaks the DMA-mapping.txt rules, because it can't
safely be called from interrupt context. I think we need generic
changes (gfp masks to various functions) to fix this sanely.
It would be nice to have the same interface as ARM, though - on the
other hand the consistent_*() interface as such may be obsoleted by
unified device model interfaces: I believe per-bus consistent memory
interfaces are supposed to be in there.
> Revisiting this is prompted by a couple of new platforms recently added
> (Xilinx Virtex II Pro and IBM 405LP/Beech) that use consistent_alloc for
> framebuffers mmap'ed to X servers. It would be nice (but is hardly a
> critical need) if consistent_alloc/free would set/clear the reserved bit
> on the struct page's allocated, such that the returned memory can be
> mmap'ed to userspace without the driver explicitly setting these bits, a
> la ARM.
I've thought about this before, but on the whole I don't think it's a
good idea. I think setting VM_IO on the vma when it is mapped into
userspace is a better idea.
In PCI space, the assumption appears to be that PageReserved is *not*
set by pci_alloc_consistent() - a couple of the sound drivers assume
this, because they explicitly set and clear the Reserved bit when they
remap various DMA buffers.
Incidentally I think there are also cases where consistent_alloc()
(both ours and ARM's, IIRC) could leak memory if failures occur at
just the right point.
> On PPC this is complicated by the fact that consistent_free does not
> take a size parameter, describing the size of the allocated area, nor a
> dma_handle parameter, which ARM uses to derive struct page's.
Yes, on the other hand having a single "handle" on the memory in order
to free it is nice.
> Furthermore, consistent_alloc does not fill out vm_struct fields for the
> VM area that map it to the struct pages, so consistent_free can't use
> those to derive the physical pages. And neither can vfree(), which is
> what consistent_free() calls to do its work. And so consistent_free()
> does not free the physical pages allocated by consistent_alloc(), which
> is a potentially more serious matter (although I suppose consistent_free
> isn't normally in heavy use, but hi-res framebuffers can get large).
Erm... vfree() walks the page tables to find the pages to free, which
should work fine.
> Setting/clearing reserved bits and init'ing the vm_struct fields such
> that physical pages are reclaimed by vfree() can be accomplished without
> API changes, at the expense of intruding into private VM data structures
> and inefficiency of allocating an array of struct page pointers to
> describe a contiguous chunk of memory. So it seems preferable to add
> the size and dma_handle params to consistent_free and do the physical
> page freeing there (and update the drivers that call it). Comments?
David Gibson | For every complex problem there is a
david at gibson.dropbear.id.au | solution which is simple, neat and
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
More information about the Linuxppc-dev