RE: [Linaro-mm-sig] [PATCH 6/8] drivers: add Contiguous Memory Allocator

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On Monday, July 11, 2011 9:01 PM Janusz Krzysztofik wrote:

> Dnia poniedziałek, 11 lipca 2011 o 15:47:32 Marek Szyprowski napisał(a):
> > Hello,
> >
> > On Saturday, July 09, 2011 4:57 PM Janusz Krzysztofik	wrote:
> > > On Wed, 6 Jul 2011 at 16:59:45 Arnd Bergmann wrote:
> > > > On Wednesday 06 July 2011, Nicolas Pitre wrote:
> > > > > On Wed, 6 Jul 2011, Russell King - ARM Linux wrote:
> > > > > > Another issue is that when a platform has restricted DMA
> > > > > > regions, they typically don't fall into the highmem zone.
> > > > > > As the dmabounce code allocates from the DMA coherent
> > > > > > allocator to provide it with guaranteed DMA-able memory,
> > > > > > that would be rather inconvenient.
> > > > >
> > > > > Do we encounter this in practice i.e. do those platforms
> > > > > requiring large contiguous allocations motivating this work
> > > > > have such DMA restrictions?
> > > >
> > > > You can probably find one or two of those, but we don't have to
> > > > optimize for that case. I would at least expect the maximum size
> > > > of the allocation to be smaller than the DMA limit for these,
> > > > and consequently mandate that they define a sufficiently large
> > > > CONSISTENT_DMA_SIZE for the crazy devices, or possibly add a
> > > > hack to unmap some low memory and call
> > > > dma_declare_coherent_memory() for the device.
> > >
> > > Once found that Russell has dropped his "ARM: DMA: steal memory for
> > > DMA coherent mappings" for now, let me get back to this idea of a
> > > hack that would allow for safely calling
> > > dma_declare_coherent_memory() in order to assign a device with a
> > > block of contiguous memory for exclusive use.
> >
> > We tested such approach and finally with 3.0-rc1 it works fine. You
> > can find an example for dma_declare_coherent() together with
> > required memblock_remove() calls in the following patch series:
> > http://www.spinics.net/lists/linux-samsung-soc/msg05026.html
> > "[PATCH 0/3 v2] ARM: S5P: Add support for MFC device on S5PV210 and
> > EXYNOS4"
> >
> > > Assuming there should be no problem with successfully allocating a
> > > large continuous block of coherent memory at boot time with
> > > dma_alloc_coherent(), this block could be reserved for the device.
> > > The only problem is with the dma_declare_coherent_memory() calling
> > > ioremap(), which was designed with a device's dedicated physical
> > > memory in mind, but shouldn't be called on a memory already
> > > mapped.
> >
> > All these issues with ioremap has been finally resolved in 3.0-rc1.
> > Like Russell pointed me in
> > http://www.spinics.net/lists/arm-kernel/msg127644.html, ioremap can
> > be fixed to work on early reserved memory areas by selecting
> > ARCH_HAS_HOLES_MEMORYMODEL Kconfig option.
> 
> I'm not sure. Recently I tried to refresh my now 7 months old patch in
> which I used that 'memblock_remove() then dma_declare_coherent_memery()'
> method[1]. It was different from your S5P MFC example in that it didn't
> punch any holes in the system memory, only stole a block of SDRAM from
> its tail. But Russell reminded me again: "we should not be mapping SDRAM
> using device mappings."[2]. Would defining ARCH_HAS_HOLES_MEMORYMODEL
> (even if it was justified) make any diference in my case? I don't think
> so.

Defining ARCH_HAS_HOLES_MEMORYMODEL changes the behavior of valid_pfn()
macro/function, which is used in the ioremap(). When defined, valid_pfn()
checks if the selected pfn is inside system memory or not (using memblock
information). If the area is removed with memblock_remove(), then a check
with valid_pfn() fails and ioremap() doesn't complain about mapping
system memory.

> Wnat I think, after Russell, is that we still need that obligatory
> ioremap() removed from dma_declare_coherent_memory(), or made it
> optional, or a separate dma_declare_coherent_memory()-like function
> without (obligatory) ioremap() provided by the DMA API, in order to get
> the dma_declare_coherent_memery() method being accepted without any
> reservations when used inside arch/arm, I'm afraid.

Please check again with 3.0-rc1. ARCH_HAS_HOLES_MEMORYMODEL solution was
suggested by Russell. It looks like this is the correct solution for this
problem, because I don't believe that ioremap() will be removed from 
dma_declare_coherent() anytime soon. 

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center