Inline the storage for thread name in prof_tdata_t.

The previous approach managed the thread name in a separate buffer, which causes
races because the thread name update (triggered by new samples) can happen at
the same time as prof dumping (which reads the thread names) -- these two
operations are under separate locks to avoid blocking each other.  Implemented
the thread name storage as part of the tdata struct, which resolves the lifetime
issue and also avoids internal alloc / dalloc during prof_sample.
This commit is contained in:
Qi Wang 2023-03-28 18:02:34 -07:00 committed by Qi Wang
parent 6cab460a45
commit ce0b7ab6c8
11 changed files with 120 additions and 103 deletions

View File

@ -18,7 +18,6 @@ bool prof_bt_keycomp(const void *k1, const void *k2);
bool prof_data_init(tsd_t *tsd); bool prof_data_init(tsd_t *tsd);
prof_tctx_t *prof_lookup(tsd_t *tsd, prof_bt_t *bt); prof_tctx_t *prof_lookup(tsd_t *tsd, prof_bt_t *bt);
char *prof_thread_name_alloc(tsd_t *tsd, const char *thread_name);
int prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name); int prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name);
void prof_unbias_map_init(); void prof_unbias_map_init();
void prof_dump_impl(tsd_t *tsd, write_cb_t *prof_dump_write, void *cbopaque, void prof_dump_impl(tsd_t *tsd, write_cb_t *prof_dump_write, void *cbopaque,

View File

@ -38,6 +38,22 @@ prof_gdump_get_unlocked(void) {
return prof_gdump_val; return prof_gdump_val;
} }
JEMALLOC_ALWAYS_INLINE void
prof_thread_name_assert(prof_tdata_t *tdata) {
if (!config_debug) {
return;
}
prof_active_assert();
bool terminated = false;
for (unsigned i = 0; i < PROF_THREAD_NAME_MAX_LEN; i++) {
if (tdata->thread_name[i] == '\0') {
terminated = true;
}
}
assert(terminated);
}
JEMALLOC_ALWAYS_INLINE prof_tdata_t * JEMALLOC_ALWAYS_INLINE prof_tdata_t *
prof_tdata_get(tsd_t *tsd, bool create) { prof_tdata_get(tsd_t *tsd, bool create) {
prof_tdata_t *tdata; prof_tdata_t *tdata;
@ -59,6 +75,10 @@ prof_tdata_get(tsd_t *tsd, bool create) {
assert(tdata == NULL || tdata->attached); assert(tdata == NULL || tdata->attached);
} }
if (tdata != NULL) {
prof_thread_name_assert(tdata);
}
return tdata; return tdata;
} }
@ -255,4 +275,18 @@ prof_free(tsd_t *tsd, const void *ptr, size_t usize,
} }
} }
JEMALLOC_ALWAYS_INLINE bool
prof_thread_name_empty(prof_tdata_t *tdata) {
prof_active_assert();
return (tdata->thread_name[0] == '\0');
}
JEMALLOC_ALWAYS_INLINE void
prof_thread_name_clear(prof_tdata_t *tdata) {
prof_active_assert();
tdata->thread_name[0] = '\0';
}
#endif /* JEMALLOC_INTERNAL_PROF_INLINES_H */ #endif /* JEMALLOC_INTERNAL_PROF_INLINES_H */

View File

@ -156,9 +156,6 @@ struct prof_tdata_s {
*/ */
uint64_t thr_discrim; uint64_t thr_discrim;
/* Included in heap profile dumps if non-NULL. */
char *thread_name;
bool attached; bool attached;
bool expired; bool expired;
@ -179,6 +176,9 @@ struct prof_tdata_s {
*/ */
ckh_t bt2tctx; ckh_t bt2tctx;
/* Included in heap profile dumps if has content. */
char thread_name[PROF_THREAD_NAME_MAX_LEN];
/* State used to avoid dumping while operating on prof internals. */ /* State used to avoid dumping while operating on prof internals. */
bool enq; bool enq;
bool enq_idump; bool enq_idump;

View File

@ -77,4 +77,7 @@ typedef struct prof_recent_s prof_recent_t;
/* Default number of recent allocations to record. */ /* Default number of recent allocations to record. */
#define PROF_RECENT_ALLOC_MAX_DEFAULT 0 #define PROF_RECENT_ALLOC_MAX_DEFAULT 0
/* Thread name storage size limit. */
#define PROF_THREAD_NAME_MAX_LEN 16
#endif /* JEMALLOC_INTERNAL_PROF_TYPES_H */ #endif /* JEMALLOC_INTERNAL_PROF_TYPES_H */

View File

@ -2384,13 +2384,13 @@ thread_prof_name_ctl(tsd_t *tsd, const size_t *mib,
READ_XOR_WRITE(); READ_XOR_WRITE();
if (newp != NULL) { if (newp != NULL) {
if (newlen != sizeof(const char *)) { const char *newval = *(const char **)newp;
if (newlen != sizeof(const char *) || newval == NULL) {
ret = EINVAL; ret = EINVAL;
goto label_return; goto label_return;
} }
if ((ret = prof_thread_name_set(tsd, *(const char **)newp)) != if ((ret = prof_thread_name_set(tsd, newval)) != 0) {
0) {
goto label_return; goto label_return;
} }
} else { } else {

View File

@ -415,11 +415,14 @@ prof_tdata_t *
prof_tdata_reinit(tsd_t *tsd, prof_tdata_t *tdata) { prof_tdata_reinit(tsd_t *tsd, prof_tdata_t *tdata) {
uint64_t thr_uid = tdata->thr_uid; uint64_t thr_uid = tdata->thr_uid;
uint64_t thr_discrim = tdata->thr_discrim + 1; uint64_t thr_discrim = tdata->thr_discrim + 1;
char *thread_name = (tdata->thread_name != NULL) ?
prof_thread_name_alloc(tsd, tdata->thread_name) : NULL;
bool active = tdata->active; bool active = tdata->active;
/* Keep a local copy of the thread name, before detaching. */
prof_thread_name_assert(tdata);
char thread_name[PROF_THREAD_NAME_MAX_LEN];
strncpy(thread_name, tdata->thread_name, PROF_THREAD_NAME_MAX_LEN);
prof_tdata_detach(tsd, tdata); prof_tdata_detach(tsd, tdata);
return prof_tdata_init_impl(tsd, thr_uid, thr_discrim, thread_name, return prof_tdata_init_impl(tsd, thr_uid, thr_discrim, thread_name,
active); active);
} }
@ -464,15 +467,15 @@ prof_active_set(tsdn_t *tsdn, bool active) {
const char * const char *
prof_thread_name_get(tsd_t *tsd) { prof_thread_name_get(tsd_t *tsd) {
static const char *prof_thread_name_dummy = "";
assert(tsd_reentrancy_level_get(tsd) == 0); assert(tsd_reentrancy_level_get(tsd) == 0);
prof_tdata_t *tdata = prof_tdata_get(tsd, true);
prof_tdata_t *tdata;
tdata = prof_tdata_get(tsd, true);
if (tdata == NULL) { if (tdata == NULL) {
return ""; return prof_thread_name_dummy;
} }
return (tdata->thread_name != NULL ? tdata->thread_name : "");
return tdata->thread_name;
} }
int int

View File

@ -441,64 +441,30 @@ prof_bt_count(void) {
return bt_count; return bt_count;
} }
char * static void
prof_thread_name_alloc(tsd_t *tsd, const char *thread_name) { prof_thread_name_write_tdata(prof_tdata_t *tdata, const char *thread_name) {
char *ret; strncpy(tdata->thread_name, thread_name, PROF_THREAD_NAME_MAX_LEN);
size_t size; tdata->thread_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0';
if (thread_name == NULL) {
return NULL;
}
size = strlen(thread_name) + 1;
ret = iallocztm(tsd_tsdn(tsd), size, sz_size2index(size), false, NULL,
true, arena_get(TSDN_NULL, 0, true), true);
if (ret == NULL) {
return NULL;
}
memcpy(ret, thread_name, size);
ret[size - 1] = '\0';
return ret;
} }
int int
prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name) { prof_thread_name_set_impl(tsd_t *tsd, const char *thread_name) {
assert(tsd_reentrancy_level_get(tsd) == 0); assert(tsd_reentrancy_level_get(tsd) == 0);
assert(thread_name != NULL);
prof_tdata_t *tdata; for (unsigned i = 0; thread_name[i] != '\0'; i++) {
unsigned i;
char *s;
tdata = prof_tdata_get(tsd, true);
if (tdata == NULL) {
return EAGAIN;
}
/* Validate input. */
if (thread_name == NULL) {
return EFAULT;
}
for (i = 0; thread_name[i] != '\0'; i++) {
char c = thread_name[i]; char c = thread_name[i];
if (!isgraph(c) && !isblank(c)) { if (!isgraph(c) && !isblank(c)) {
return EFAULT; return EINVAL;
} }
} }
s = prof_thread_name_alloc(tsd, thread_name); prof_tdata_t *tdata = prof_tdata_get(tsd, true);
if (s == NULL) { if (tdata == NULL) {
return EAGAIN; return ENOMEM;
} }
char *old_thread_name = tdata->thread_name; prof_thread_name_write_tdata(tdata, thread_name);
tdata->thread_name = s;
if (old_thread_name != NULL) {
idalloctm(tsd_tsdn(tsd), old_thread_name, /* tcache */ NULL,
/* alloc_ctx */ NULL, /* is_internal */ true,
/* slow_path */ true);
}
return 0; return 0;
} }
@ -949,7 +915,7 @@ prof_tdata_dump_iter(prof_tdata_tree_t *tdatas_ptr, prof_tdata_t *tdata,
tdata->thr_uid); tdata->thr_uid);
prof_dump_print_cnts(arg->prof_dump_write, arg->cbopaque, prof_dump_print_cnts(arg->prof_dump_write, arg->cbopaque,
&tdata->cnt_summed); &tdata->cnt_summed);
if (tdata->thread_name != NULL) { if (!prof_thread_name_empty(tdata)) {
arg->prof_dump_write(arg->cbopaque, " "); arg->prof_dump_write(arg->cbopaque, " ");
arg->prof_dump_write(arg->cbopaque, tdata->thread_name); arg->prof_dump_write(arg->cbopaque, tdata->thread_name);
} }
@ -1179,10 +1145,15 @@ prof_tdata_init_impl(tsd_t *tsd, uint64_t thr_uid, uint64_t thr_discrim,
tdata->lock = prof_tdata_mutex_choose(thr_uid); tdata->lock = prof_tdata_mutex_choose(thr_uid);
tdata->thr_uid = thr_uid; tdata->thr_uid = thr_uid;
tdata->thr_discrim = thr_discrim; tdata->thr_discrim = thr_discrim;
tdata->thread_name = thread_name;
tdata->attached = true; tdata->attached = true;
tdata->expired = false; tdata->expired = false;
tdata->tctx_uid_next = 0; tdata->tctx_uid_next = 0;
if (thread_name == NULL) {
prof_thread_name_clear(tdata);
} else {
prof_thread_name_write_tdata(tdata, thread_name);
}
prof_thread_name_assert(tdata);
if (ckh_new(tsd, &tdata->bt2tctx, PROF_CKH_MINITEMS, prof_bt_hash, if (ckh_new(tsd, &tdata->bt2tctx, PROF_CKH_MINITEMS, prof_bt_hash,
prof_bt_keycomp)) { prof_bt_keycomp)) {
@ -1230,13 +1201,8 @@ prof_tdata_destroy_locked(tsd_t *tsd, prof_tdata_t *tdata,
malloc_mutex_assert_not_owner(tsd_tsdn(tsd), tdata->lock); malloc_mutex_assert_not_owner(tsd_tsdn(tsd), tdata->lock);
tdata_tree_remove(&tdatas, tdata); tdata_tree_remove(&tdatas, tdata);
assert(prof_tdata_should_destroy_unlocked(tdata, even_if_attached)); assert(prof_tdata_should_destroy_unlocked(tdata, even_if_attached));
if (tdata->thread_name != NULL) {
idalloctm(tsd_tsdn(tsd), tdata->thread_name, NULL, NULL, true,
true);
}
ckh_delete(tsd, &tdata->bt2tctx); ckh_delete(tsd, &tdata->bt2tctx);
idalloctm(tsd_tsdn(tsd), tdata, NULL, NULL, true, true); idalloctm(tsd_tsdn(tsd), tdata, NULL, NULL, true, true);
} }

View File

@ -243,8 +243,7 @@ prof_try_log(tsd_t *tsd, size_t usize, prof_info_t *prof_info) {
iallocztm(tsd_tsdn(tsd), sz, sz_size2index(sz), false, NULL, true, iallocztm(tsd_tsdn(tsd), sz, sz_size2index(sz), false, NULL, true,
arena_get(TSDN_NULL, 0, true), true); arena_get(TSDN_NULL, 0, true), true);
const char *prod_thr_name = (tctx->tdata->thread_name == NULL)? const char *prod_thr_name = tctx->tdata->thread_name;
"" : tctx->tdata->thread_name;
const char *cons_thr_name = prof_thread_name_get(tsd); const char *cons_thr_name = prof_thread_name_get(tsd);
prof_bt_t bt; prof_bt_t bt;

View File

@ -495,7 +495,7 @@ prof_recent_alloc_dump_node(emitter_t *emitter, prof_recent_t *node) {
&node->alloc_tctx->thr_uid); &node->alloc_tctx->thr_uid);
prof_tdata_t *alloc_tdata = node->alloc_tctx->tdata; prof_tdata_t *alloc_tdata = node->alloc_tctx->tdata;
assert(alloc_tdata != NULL); assert(alloc_tdata != NULL);
if (alloc_tdata->thread_name != NULL) { if (!prof_thread_name_empty(alloc_tdata)) {
emitter_json_kv(emitter, "alloc_thread_name", emitter_json_kv(emitter, "alloc_thread_name",
emitter_type_string, &alloc_tdata->thread_name); emitter_type_string, &alloc_tdata->thread_name);
} }
@ -511,7 +511,7 @@ prof_recent_alloc_dump_node(emitter_t *emitter, prof_recent_t *node) {
emitter_type_uint64, &node->dalloc_tctx->thr_uid); emitter_type_uint64, &node->dalloc_tctx->thr_uid);
prof_tdata_t *dalloc_tdata = node->dalloc_tctx->tdata; prof_tdata_t *dalloc_tdata = node->dalloc_tctx->tdata;
assert(dalloc_tdata != NULL); assert(dalloc_tdata != NULL);
if (dalloc_tdata->thread_name != NULL) { if (!prof_thread_name_empty(dalloc_tdata)) {
emitter_json_kv(emitter, "dalloc_thread_name", emitter_json_kv(emitter, "dalloc_thread_name",
emitter_type_string, &dalloc_tdata->thread_name); emitter_type_string, &dalloc_tdata->thread_name);
} }

View File

@ -462,12 +462,17 @@ prof_sys_thread_name_read_t *JET_MUTABLE prof_sys_thread_name_read =
void void
prof_sys_thread_name_fetch(tsd_t *tsd) { prof_sys_thread_name_fetch(tsd_t *tsd) {
#define THREAD_NAME_MAX_LEN 16 prof_tdata_t *tdata = prof_tdata_get(tsd, true);
char buf[THREAD_NAME_MAX_LEN]; if (tdata == NULL) {
if (!prof_sys_thread_name_read(buf, THREAD_NAME_MAX_LEN)) { return;
prof_thread_name_set_impl(tsd, buf);
} }
#undef THREAD_NAME_MAX_LEN
if (prof_sys_thread_name_read(tdata->thread_name,
PROF_THREAD_NAME_MAX_LEN) != 0) {
prof_thread_name_clear(tdata);
}
tdata->thread_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0';
} }
int int

View File

@ -14,8 +14,6 @@ mallctl_thread_name_get_impl(const char *thread_name_expected, const char *func,
expect_str_eq(thread_name_old, thread_name_expected, expect_str_eq(thread_name_old, thread_name_expected,
"%s():%d: Unexpected thread.prof.name value", func, line); "%s():%d: Unexpected thread.prof.name value", func, line);
} }
#define mallctl_thread_name_get(a) \
mallctl_thread_name_get_impl(a, __func__, __LINE__)
static void static void
mallctl_thread_name_set_impl(const char *thread_name, const char *func, mallctl_thread_name_set_impl(const char *thread_name, const char *func,
@ -26,51 +24,59 @@ mallctl_thread_name_set_impl(const char *thread_name, const char *func,
func, line); func, line);
mallctl_thread_name_get_impl(thread_name, func, line); mallctl_thread_name_get_impl(thread_name, func, line);
} }
#define mallctl_thread_name_get(a) \
mallctl_thread_name_get_impl(a, __func__, __LINE__)
#define mallctl_thread_name_set(a) \ #define mallctl_thread_name_set(a) \
mallctl_thread_name_set_impl(a, __func__, __LINE__) mallctl_thread_name_set_impl(a, __func__, __LINE__)
TEST_BEGIN(test_prof_thread_name_validation) { TEST_BEGIN(test_prof_thread_name_validation) {
const char *thread_name;
test_skip_if(!config_prof); test_skip_if(!config_prof);
test_skip_if(opt_prof_sys_thread_name); test_skip_if(opt_prof_sys_thread_name);
mallctl_thread_name_get(""); mallctl_thread_name_get("");
mallctl_thread_name_set("hi there");
const char *test_name1 = "test case1";
mallctl_thread_name_set(test_name1);
/* Test name longer than the max len. */
char long_name[] =
"test case longer than expected; test case longer than expected";
expect_zu_gt(strlen(long_name), PROF_THREAD_NAME_MAX_LEN,
"Long test name not long enough");
const char *test_name_long = long_name;
expect_d_eq(mallctl("thread.prof.name", NULL, NULL,
(void *)&test_name_long, sizeof(test_name_long)), 0,
"Unexpected mallctl failure from thread.prof.name");
/* Long name cut to match. */
long_name[PROF_THREAD_NAME_MAX_LEN - 1] = '\0';
mallctl_thread_name_get(test_name_long);
/* NULL input shouldn't be allowed. */ /* NULL input shouldn't be allowed. */
thread_name = NULL; const char *test_name2 = NULL;
expect_d_eq(mallctl("thread.prof.name", NULL, NULL, expect_d_eq(mallctl("thread.prof.name", NULL, NULL,
(void *)&thread_name, sizeof(thread_name)), EFAULT, (void *)&test_name2, sizeof(test_name2)), EINVAL,
"Unexpected mallctl result writing \"%s\" to thread.prof.name", "Unexpected mallctl result writing to thread.prof.name");
thread_name);
/* '\n' shouldn't be allowed. */ /* '\n' shouldn't be allowed. */
thread_name = "hi\nthere"; const char *test_name3 = "test\ncase";
expect_d_eq(mallctl("thread.prof.name", NULL, NULL, expect_d_eq(mallctl("thread.prof.name", NULL, NULL,
(void *)&thread_name, sizeof(thread_name)), EFAULT, (void *)&test_name3, sizeof(test_name3)), EINVAL,
"Unexpected mallctl result writing \"%s\" to thread.prof.name", "Unexpected mallctl result writing \"%s\" to thread.prof.name",
thread_name); test_name3);
/* Simultaneous read/write shouldn't be allowed. */ /* Simultaneous read/write shouldn't be allowed. */
{
const char *thread_name_old; const char *thread_name_old;
size_t sz; size_t sz = sizeof(thread_name_old);
expect_d_eq(mallctl("thread.prof.name", (void *)&thread_name_old, &sz,
sz = sizeof(thread_name_old); (void *)&test_name1, sizeof(test_name1)), EPERM,
expect_d_eq(mallctl("thread.prof.name", "Unexpected mallctl result from thread.prof.name");
(void *)&thread_name_old, &sz, (void *)&thread_name,
sizeof(thread_name)), EPERM,
"Unexpected mallctl result writing \"%s\" to "
"thread.prof.name", thread_name);
}
mallctl_thread_name_set(""); mallctl_thread_name_set("");
} }
TEST_END TEST_END
#define NTHREADS 4
#define NRESET 25
static void * static void *
thd_start(void *varg) { thd_start(void *varg) {
unsigned thd_ind = *(unsigned *)varg; unsigned thd_ind = *(unsigned *)varg;
@ -82,6 +88,7 @@ thd_start(void *varg) {
mallctl_thread_name_get(""); mallctl_thread_name_get("");
mallctl_thread_name_set(thread_name); mallctl_thread_name_set(thread_name);
#define NRESET 25
for (i = 0; i < NRESET; i++) { for (i = 0; i < NRESET; i++) {
expect_d_eq(mallctl("prof.reset", NULL, NULL, NULL, 0), 0, expect_d_eq(mallctl("prof.reset", NULL, NULL, NULL, 0), 0,
"Unexpected error while resetting heap profile data"); "Unexpected error while resetting heap profile data");
@ -92,12 +99,14 @@ thd_start(void *varg) {
mallctl_thread_name_set(""); mallctl_thread_name_set("");
return NULL; return NULL;
#undef NRESET
} }
TEST_BEGIN(test_prof_thread_name_threaded) { TEST_BEGIN(test_prof_thread_name_threaded) {
test_skip_if(!config_prof); test_skip_if(!config_prof);
test_skip_if(opt_prof_sys_thread_name); test_skip_if(opt_prof_sys_thread_name);
#define NTHREADS 4
thd_t thds[NTHREADS]; thd_t thds[NTHREADS];
unsigned thd_args[NTHREADS]; unsigned thd_args[NTHREADS];
unsigned i; unsigned i;
@ -109,10 +118,9 @@ TEST_BEGIN(test_prof_thread_name_threaded) {
for (i = 0; i < NTHREADS; i++) { for (i = 0; i < NTHREADS; i++) {
thd_join(thds[i], NULL); thd_join(thds[i], NULL);
} }
#undef NTHREADS
} }
TEST_END TEST_END
#undef NTHREADS
#undef NRESET
int int
main(void) { main(void) {