From: Stephen Warren <swarren@nvidia.com>
Date: Sat, 5 Sep 2015 04:03:46 +0000 (-0600)
Subject: dfu: don't keep freeing/reallocating
X-Git-Url: http://git.dujemihanovic.xyz/?a=commitdiff_plain;h=806bd245b1ab;p=u-boot.git

dfu: don't keep freeing/reallocating

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 <swarren@nvidia.com>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>
---

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 6cf240de22..6842aef420 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)