diff mbox

[v2] Clean index map on exit

Message ID B9523649-97F2-4979-8590-F63788B4AC9D@gmx.net (mailing list archive)
State Rejected
Headers show

Commit Message

Hannes Weisbach April 11, 2014, 6:22 p.m. UTC
From: Hannes Weisbach <hannes_weisbach@gmx.net>

librdmacm already has a cleanup function which releases all resources it
holds. Missing from that cleanup are entries with struct index_map,
which are dynamically allocated, whereas the map itself is declared
static. Because librdmacm plays otherwise nicely with valgrind this
patch aims to free the memory held by index map entries, mainly to
reduce valgrind noise.
This patch adds the function idm_free() to free all entries of an index
map. A call to this function is added in the ucma_cleanup destructor.
The ucma_idm struct index_map is cleaned using idm_free().
There is another struct index_map in preload.c and rsocket.c, but there
are no destructors yet and I don't feel competent enough to add them.

Signed-off-by: Hannes Weisbach <hannes_weisbach@gmx.net>
---
 src/cma.c     | 1 +
 src/indexer.c | 8 ++++++++
 src/indexer.h | 1 +
 3 files changed, 10 insertions(+)

Comments

Hefty, Sean May 24, 2014, 1:22 a.m. UTC | #1
> librdmacm already has a cleanup function which releases all resources it
> holds. Missing from that cleanup are entries with struct index_map,
> which are dynamically allocated, whereas the map itself is declared
> static. Because librdmacm plays otherwise nicely with valgrind this
> patch aims to free the memory held by index map entries, mainly to
> reduce valgrind noise.
> This patch adds the function idm_free() to free all entries of an index
> map. A call to this function is added in the ucma_cleanup destructor.
> The ucma_idm struct index_map is cleaned using idm_free().
> There is another struct index_map in preload.c and rsocket.c, but there
> are no destructors yet and I don't feel competent enough to add them.

Instead of adding idm_free, I added count values to the index_map structure.  The allocated arrays are freed whenever the count drops to 0.  This should handle all three cases -- cma, preload, and rsocket.

I sent out a separate patch to address this.  Please let me know if you see any issues with it.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/cma.c b/src/cma.c
index 0cf4203..a533bf9 100644
--- a/src/cma.c
+++ b/src/cma.c
@@ -139,6 +139,7 @@  static void ucma_cleanup(void)
 			ibv_close_device(cma_dev_array[cma_dev_cnt].verbs);
 		}
 
+		idm_free(&ucma_idm);
 		fastlock_destroy(&idm_lock);
 		free(cma_dev_array);
 		cma_dev_cnt = 0;
diff --git a/src/indexer.c b/src/indexer.c
index c8e8bce..8dd8a82 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -164,3 +164,11 @@  void *idm_clear(struct index_map *idm, int index)
 	entry[idx_entry_index(index)] = NULL;
 	return item;
 }
+
+void idm_free(struct index_map *idm)
+{
+	size_t i;
+	for (i = 0; i < IDX_ARRAY_SIZE; i++) {
+		free(idm->array[i]);
+	}
+}
diff --git a/src/indexer.h b/src/indexer.h
index 0c5f388..829d46b 100644
--- a/src/indexer.h
+++ b/src/indexer.h
@@ -89,6 +89,7 @@  struct index_map
 
 int idm_set(struct index_map *idm, int index, void *item);
 void *idm_clear(struct index_map *idm, int index);
+void idm_free(struct index_map *idm);
 
 static inline void *idm_at(struct index_map *idm, int index)
 {