On Sun, May 2, 2010 at 4:17 PM, Felipe Contreras <felipe.contre...@gmail.com> wrote: > On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen <o...@wizery.com> wrote: >> This patchset introduces an approach to eliminate the direct calls >> to follow_page and to the low level cache APIs. >> >> The patchset works by caching the page information while memory >> is mapped, and then using that information later when needed >> instead of calling follow_page. The low level cache API is then replaced >> by the standard DMA API. > > Finally! Very interesting patch indeed.
Thanks! > >> A few key points in the current approach that I'd be happy to hear >> your feedback about: >> 1. The new mapping + page information is currently cached in the >> proc object, but it might fit better inside dmm map objects >> (by enhancing the DMM layer to support the required data caching, >> storing and retrieving). > > Sounds like a good idea. Great, I'll look into this. > >> 2. The information is stored in a linked list. That's pretty fine >> as long as the number of memory mappings per application is not >> extremely high. If that assumption is wrong, a different underlying >> data structure might be better (hash table, priority tree, etc..). > > I think a linked list is fine for now. AFAIK only a limited number of > mmaps happen at the same time, usually 4. Thanks for confirming. > >> 3. Moving to standard DMA API completely changes the user's point >> of view; users should no longer think in terms of which cache >> manipulation is required, but instead, they should just tell dspbridge >> before a DMA transfer begins, and after it ends. Between the begin >> and end calls, the buffer "belongs" to the DSP and should not >> be accessed by the user. > > This is really nice. That API should have been that way since the beginning. > >> The patchset renames the flush ioctl to begin_dma_to_dsp and >> the invalidate ioctl to begin_dma_from_dsp. Both functions >> eventually call dma_map_sg, with the former requesting a >> DMA_BIDIRECTIONAL direction, and the latter requesting a >> DMA_FROM_DEVICE direction. >> In addition, the patchset adds two new APIs which calls dma_unmap_sg: >> end_dma_to_dsp and end_dma_from_dsp. >> >> Ideally, there would be only a single begin_dma command and a single >> end_dma one, which would accept an additional parameter that will >> determine the direction of the transfer. Such an approach would be more >> versatile and cleaner, but it would also break all user space apps that >> use dspbridge today. > > If I understand correctly all user-space apps would be broken anyway > because they are not issuing the end_dma calls. At the very least they > need to be updated to use them. Basically you're right, but the patches currently silently allow today's user space to somehow work (because if you don't use dma bounce buffers and you feel lucky about speculative prefetching then you might get away with not calling dma unmap). I did that on purpose, to ease testing & migration, but didn't explicitely said it because frankly it's just wrong. > > Also, in Nokia we patched the bridgedriver to be able to have the 3 > operations available from user-space (clean, inv, and flush), so we > would be very interested in having the direction of the transfer > available. Thanks, that's important to know. What do you say about the following plan then: - I'll add a single pair of begin_dma and end_dma commands that will take the additional direction parameter as described above. I'll then covert the existing flush & invalidate commands to use this begin_dma command supplying it the appropriate direction argument. - In a separate patch, I'll deprecate flush & invalidate entirely (usage of this deprecated API will fail, resulting in a guiding error message). We get a sane and versatile (and platform-independent) implementation that always work, but breaks user space. If anyone would need to work with current user space, the deprecating patch can always be reverted locally to get back a flush & invalidate implementations that (somehow) work. Anyway, I'm sure that breaking user space can be somewhat traumatic so let's make sure we get an across-the-board support for this. Thanks a lot for the comments! Ohad. > > Cheers. > > -- > Felipe Contreras > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
