+ arm-move-arm_dma_limit-to-setup_dma_zone.patch added to -mm tree

+ arm-move-arm_dma_limit-to-setup_dma_zone.patch added to -mm tree

[Andrew Morton]

On Sun, 18 May 2014 22:49:16 -0700 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> Gents,
> 
> I noticed this patch turning up in -next:
> http://ozlabs.org/~akpm/mmots/broken-out/arm-move-arm_dma_limit-to-setup_dma_zone.patch
> 
> which I think is incorrect. More specifically, it replaces a reference
> to arm_dma_limit, which is defined as either
> 
> arch/arm/mm/init.c:177:         arm_dma_limit = PHYS_OFFSET +
> arm_dma_zone_size - 1;
> 
> or
> 
> arch/arm/mm/init.c:179:         arm_dma_limit = 0xffffffff;
> 
> with 'arm_dma_pfn_limit << PAGE_SHIFT', which means the bottom
> PAGE_SHIFT '1' bytes are lost.

That patch has been floating about in "stuck" state for months.

I think I'll drop it.  Vladimir, please resend something if you think
it still should be addressed.

+ arm-move-arm_dma_limit-to-setup_dma_zone.patch added to -mm tree

[Vladimir Murzin]

Hi Ard

On 5/19/14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Gents,
>
> I noticed this patch turning up in -next:
> http://ozlabs.org/~akpm/mmots/broken-out/arm-move-arm_dma_limit-to-setup_dma_zone.patch
>
> which I think is incorrect. More specifically, it replaces a reference
> to arm_dma_limit, which is defined as either
>
> arch/arm/mm/init.c:177:         arm_dma_limit = PHYS_OFFSET +
> arm_dma_zone_size - 1;
>
> or
>
> arch/arm/mm/init.c:179:         arm_dma_limit = 0xffffffff;
>
> with 'arm_dma_pfn_limit << PAGE_SHIFT', which means the bottom
> PAGE_SHIFT '1' bytes are lost.
>

Do not we lost even more with
dma_contiguous_reserve
     dma_contiguous_reserve_area
...
        /* Sanitise input arguments */
        alignment = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
        base = ALIGN(base, alignment);
        size = ALIGN(size, alignment);
        limit &= ~(alignment - 1);

Vladimir

> Relevant part of patch quoted below.
>
> Regards,
> Ard.
>
>
>
>
> diff -puN arch/arm/mm/init.c~arm-move-arm_dma_limit-to-setup_dma_zone
> arch/arm/mm/init.c
> --- a/arch/arm/mm/init.c~arm-move-arm_dma_limit-to-setup_dma_zone
> +++ a/arch/arm/mm/init.c
> @@ -328,7 +329,8 @@ void __init arm_memblock_init(struct mem
>   * reserve memory for DMA contigouos allocations,
>   * must come from DMA area inside low memory
>   */
> - dma_contiguous_reserve(min(arm_dma_limit, arm_lowmem_limit));
> +
(min((phys_addr_t)arm_dma_pfn_limit << PAGE_SHIFT,
> +   arm_lowmem_limit));
>
>   arm_memblock_steal_permitted = false;
>   memblock_dump_all();
>

+ arm-move-arm_dma_limit-to-setup_dma_zone.patch added to -mm tree

[Ard Biesheuvel]

On 21 May 2014 08:50, Vladimir Murzin <murzin.v@gmail.com> wrote:
> Hi Ard
>
> On 5/19/14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> Gents,
>>
>> I noticed this patch turning up in -next:
>> http://ozlabs.org/~akpm/mmots/broken-out/arm-move-arm_dma_limit-to-setup_dma_zone.patch
>>
>> which I think is incorrect. More specifically, it replaces a reference
>> to arm_dma_limit, which is defined as either
>>
>> arch/arm/mm/init.c:177:         arm_dma_limit = PHYS_OFFSET +
>> arm_dma_zone_size - 1;
>>
>> or
>>
>> arch/arm/mm/init.c:179:         arm_dma_limit = 0xffffffff;
>>
>> with 'arm_dma_pfn_limit << PAGE_SHIFT', which means the bottom
>> PAGE_SHIFT '1' bytes are lost.
>>
>
> Do not we lost even more with
> dma_contiguous_reserve
>      dma_contiguous_reserve_area
> ...
>         /* Sanitise input arguments */
>         alignment = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>         base = ALIGN(base, alignment);
>         size = ALIGN(size, alignment);
>         limit &= ~(alignment - 1);
>

That's interesting. It looks like there may be a mismatch here: we
pass an inclusive limit where the callee expects it to be exclusive,
resulting in the top 4 megabytes of the DMA zone to be lost (for
pageblock_order == MAX_ORDER-1). I.e., if arm_dma_zone == SZ_64M, we
will end up calling __memblock_alloc_base() with a limit of 60 megs.

@Russell: care to share your thought on this?

-- 
Ard.