Hi Stephen,
> From: Stephen Warren <swar...@nvidia.com>
>
> DFU currently allocates buffer memory at the start of each data
> transfer operation and frees it at the end. Especially since
> memalign() is used to allocate the buffer, and various other
> allocations happen during the transfer, this can expose the code to
> heap fragmentation, which prevents the allocation from succeeding on
> subsequent transfers.
>
> Fix the code to allocate the buffer once when DFU mode is initialized,
> and free the buffer once when DFU mode is exited, to reduce the
> exposure to heap fragmentation.
>
> The failure mode is:
>
> // Internally to memalign(), this allocates a lot more than s to
> guarantee // that alignment can occur, then returns chunks of memory
> at the start/ // end of the allocated buffer to the heap.
> p = memalign(a, s);
> // Various other malloc()s occur here, some of which allocate the RAM
> // immediately before/after "p".
> //
> // DFU transfer is complete, so buffer is released.
> free(p);
> // By chance, no other malloc()/free() here, in DFU at least.
> //
> // A new DFU transfer starts, so the buffer is allocated again.
> // In theory this should succeed since we just free()d a buffer of the
> // same size. However, this fails because memalign() internally
> attempts // to allocate much more than "s", yet free(p) above only
> free()d a // little more than "s".
> p = memalign(a, s);
>
> Signed-off-by: Stephen Warren <swar...@nvidia.com>
> ---
> drivers/dfu/dfu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 675162d927d8..d85d3f507a7b 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -164,7 +164,6 @@ static int dfu_write_buffer_drain(struct
> dfu_entity *dfu) void dfu_write_transaction_cleanup(struct dfu_entity
> *dfu) {
> /* clear everything */
> - dfu_free_buf();
> dfu->crc = 0;
> dfu->offset = 0;
> dfu->i_blk_seq_num = 0;
> @@ -385,7 +384,6 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu_hash_algo->name, dfu->crc);
> puts("\nUPLOAD ... done\nCtrl+C to exit ...\n");
>
> - dfu_free_buf();
> dfu->i_blk_seq_num = 0;
> dfu->crc = 0;
> dfu->offset = 0;
> @@ -433,6 +431,7 @@ static int dfu_fill_entity(struct dfu_entity
> *dfu, char *s, int alt, __func__, interface);
> return -1;
> }
> + dfu_get_buf(dfu);
>
> return 0;
> }
> @@ -441,6 +440,7 @@ void dfu_free_entities(void)
> {
> struct dfu_entity *dfu, *p, *t = NULL;
>
> + dfu_free_buf();
> list_for_each_entry_safe_reverse(dfu, p, &dfu_list, list) {
> list_del(&dfu->list);
> if (dfu->free_entity)
Acked-by: Lukasz Majewski <l.majew...@samsung.com>
Tested-by: Lukasz Majewski <l.majew...@samsung.com>
Test HW: Odroid XU3 - Exynos5433
[DFU tests]
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
