[U-Boot] malloc: work around some memalign fragmentation issues

[Tom Rini]

On Mon, Jan 25, 2016 at 02:03:42PM -0700, Stephen Warren wrote:

> From: Stephen Warren <swarren at nvidia.com>
> 
> Use of memalign can trigger fragmentation issues such as:
> 
> // Internally, this needs to find a free block quite bit larger than s.
> // Once the free region is found, any unaligned "padding" immediately
> // before and after the block is marked free, so that the allocation
> // takes only s bytes (plus malloc header overhead).
> p = memalign(a, s);
> // If there's little fragmentation so far, this allocation is likely
> // located immediately after p.
> p2 = malloc(x);
> free(p);
> // In theory, this should return the same value for p. However, the hole
> // left by the free() call is only s in size (plus malloc header overhead)
> // whereas memalign searches for a larger block in order to guarantee it
> // can adjust the returned pointer to the alignment requirements. Hence,
> // the pointer returned, if any, won't be p. If there's little or no space
> // left after p2, this allocation will fail.
> p = memalign(a, s);
> 
> In practice, this issue occurs when running the "dfu" command repeatedly
> on NVIDIA Tegra boards, since DFU allocates a large 32M data buffer, and
> then initializes the USB controller. If this is the first time USB has
> been used in the U-Boot session, this causes a probe of the USB driver,
> which causes various allocations, including a strdup() of a GPIO name
> when requesting the VBUS GPIO. When DFU is torn down, the USB driver
> is left probed, and hence its memory is left allocated. If "dfu" is
> executed again, allocation of the 32M data buffer fails as described
> above.
> 
> In practice, there is a memory hole exactly large enough to hold the 32M
> data buffer than DFU needs. However, memalign() can't know that in a
> general way. Given that, it's particularly annoying that the allocation
> fails!
> 
> The issue is that memalign() tries to allocate something larger to
> guarantee the ability to align the returned pointer. This patch modifies
> memalign() so that if the "general case" over-sized allocation fails,
> another allocation is attempted, of the exact size the user desired. If
> that allocation just happens to be aligned in the way the user wants,
> (and in the case described above, it will be, since the free memory
> region is located where a previous identical allocation was located),
> the pointer can be returned.
> 
> This patch is somewhat related to 806bd245b1ab "dfu: don't keep
> freeing/reallocating". That patch worked around the issue by removing
> repeated free/memalign within a single execution of "dfu". However,
> the same technique can't be applied across multiple invocations, since
> there's no reason to keep the DFU buffer allocated while DFU isn't
> running. This patch addresses the root-cause a bit more directly.
> 
> This problem highlights some of the disadvantages of dynamic allocation
> and deferred probing of devices.
> 
> This patch isn't checkpatch-clean, since it conforms to the existing
> coding style in dlmalloc.c, which is different to the rest of U-Boot.
> 
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> Reviewed-by: Tom Rini <trini at konsulko.com>
> Acked-by: Lukasz Majewski <l.majewski at samsung.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160201/d2a82e00/attachment.sig>