[PATCH 0/3] various minor fixes

[PATCH 0/3] various minor fixes

[Eric Wong]

Eric Wong (3):
  totals_add_rcu: improve readability
  dump_to_file: check for backtrace_symbols errors
  malloc_trim: clean up idle arenas immediately

 mwrap_core.h | 18 +++++++++++-------
 mymalloc.h   | 29 ++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 18 deletions(-)

[PATCH 1/3] totals_add_rcu: improve readability

[Eric Wong]

We no longer need to unlock in this function if it fails,
and asserting rcu_read_ongoing() is a good idea to ensure
cds_lfht_lookup() is used properly.
---
 mwrap_core.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mwrap_core.h b/mwrap_core.h
index 09b7930..291a078 100644
--- a/mwrap_core.h
+++ b/mwrap_core.h
@@ -226,7 +226,8 @@ static struct src_loc *totals_add_rcu(struct src_loc *k)
 
 again:
 	t = CMM_LOAD_SHARED(totals);
-	if (!t) goto out_unlock;
+	if (!t) return l;
+	assert(rcu_read_ongoing());
 	cds_lfht_lookup(t, k->hval, loc_eq, k, &iter);
 	cur = cds_lfht_iter_get_node(&iter);
 	if (cur) {
@@ -236,7 +237,7 @@ again:
 	} else {
 		size_t n = loc_size(k);
 		l = real_malloc(sizeof(*l) + n);
-		if (!l) goto out_unlock;
+		if (!l) return l;
 		memcpy(l, k, sizeof(*l) + n);
 		l->mtx = mutex_assign();
 		l->age_total = 0;
@@ -252,7 +253,6 @@ again:
 			goto again;
 		}
 	}
-out_unlock:
 	return l;
 }
 

[PATCH 2/3] dump_to_file: check for backtrace_symbols errors

[Eric Wong]

We can also limit the scope of its return value since the
pointers inside the return value do not get freed.
---
 mwrap_core.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/mwrap_core.h b/mwrap_core.h
index 291a078..f3b25b7 100644
--- a/mwrap_core.h
+++ b/mwrap_core.h
@@ -624,16 +624,20 @@ static void *dump_to_file(struct dump_arg *a)
 		goto out_unlock;
 	cds_lfht_for_each_entry(t, &iter, l, hnode) {
 		const void *p = l->k;
-		char **s = 0;
 		if (l->total <= a->min) continue;
 
 		if (loc_is_addr(l)) {
-			s = backtrace_symbols(p, 1);
-			p = s[0];
+			char **s = backtrace_symbols(p, 1);
+			if (s) {
+				p = s[0];
+				free(s);
+			} else {
+				fprintf(stderr, "backtrace_symbols: %s\n",
+					strerror(errno));
+			}
 		}
 		fprintf(a->fp, "%16zu %12zu %s\n",
 			l->total, l->allocations, (const char *)p);
-		if (s) free(s);
 	}
 out_unlock:
 	rcu_read_unlock();

[PATCH 3/3] malloc_trim: clean up idle arenas immediately

[Eric Wong]

There's no need to worry about race conditions if operating on
idle arenas, so clean them up immediately rather than cleaning
them up after a pthread_create (which may never come).

We'll also inform active threads about the trim earlier so they
have more cycles to react to the lazy trim request.
---
 mymalloc.h | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/mymalloc.h b/mymalloc.h
index e789cf3..4904b74 100644
--- a/mymalloc.h
+++ b/mymalloc.h
@@ -150,23 +150,30 @@ static void remote_free_finish(mstate ms)
 
 int malloc_trim(size_t pad)
 {
-	mstate ms = ms_tsd;
+	mstate m;
+	int ret = 0;
 
 	CHECK(int, 0, pthread_mutex_lock(&global_mtx));
-	{ /* be lazy for sibling threads, readers are not synchronized */
-		mstate m;
-		cds_list_for_each_entry(m, &arenas_unused, arena_node)
-			uatomic_set(&m->trim_check, 0);
-		cds_list_for_each_entry(m, &arenas_active, arena_node)
-			uatomic_set(&m->trim_check, 0);
+
+	/* be lazy for active sibling threads, readers are not synchronized */
+	cds_list_for_each_entry(m, &arenas_active, arena_node)
+		uatomic_set(&m->trim_check, 0);
+
+	/* nobody is using idle arenas, clean immediately */
+	cds_list_for_each_entry(m, &arenas_unused, arena_node) {
+		m->trim_check = 0;
+		remote_free_finish(m);
+		ret |= sys_trim(m, pad);
 	}
+
 	CHECK(int, 0, pthread_mutex_unlock(&global_mtx));
 
-	if (ms) { /* trim our own arena immediately */
-		remote_free_finish(ms);
-		return sys_trim(ms, pad);
+	m = ms_tsd;
+	if (m) { /* trim our own arena immediately */
+		remote_free_finish(m);
+		ret |= sys_trim(m, pad);
 	}
-	return 0;
+	return ret;
 }
 
 static void remote_free_enqueue(mstate fm, void *mem)