From 18c8b52afa69f39481ebb75711b2f30b11693f9d Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 12 Aug 2022 05:56:58 +0000 Subject: [PATCH] trace: allocate shm filesize based on number of cores used Previously we would always allocate the shm file based on max (128) cores which is unnecessary. So use spdk_env APIs to only allocate shm file size based on the cores we might possible use. With default settings, an shm file was 135MB before this change, now an app using cores 0-7 will just use about 9MB. A lot of the trace-related code depended on there *always* being a history for every core, even unused ones, so a few additional changes were needed, mainly the trace_parser library. Tested by starting an app using a 0x4 core mask and enabling a trace mask, generating some events, then checking both the size of the shm file and that spdk_trace works properly with the resulting file. Signed-off-by: Jim Harris Change-Id: Ie868b3e3658d6f82b2fea37cb87453e8a9e0abc4 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14044 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Konrad Sztyber Reviewed-by: Shuhei Matsumoto --- app/trace_record/trace_record.c | 40 +++++++++++++++++++++++++++------ include/spdk/trace.h | 10 +++++---- lib/trace/trace.c | 26 +++++++++++++++------ lib/trace_parser/trace.cpp | 12 +++++----- mk/spdk.unittest.mk | 2 +- 5 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/trace_record/trace_record.c b/app/trace_record/trace_record.c index 2c2aeea80..52cb1de0b 100644 --- a/app/trace_record/trace_record.c +++ b/app/trace_record/trace_record.c @@ -24,6 +24,7 @@ static uint64_t g_histories_size; struct lcore_trace_record_ctx { char lcore_file[TRACE_PATH_MAX]; int fd; + bool valid; struct spdk_trace_history *in_history; struct spdk_trace_history *out_history; @@ -94,11 +95,15 @@ input_trace_file_mmap(struct aggr_trace_record_ctx *ctx, const char *shm_name) ctx->trace_histories = (struct spdk_trace_histories *)history_ptr; for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { - ctx->lcore_ports[i].in_history = spdk_get_per_lcore_history(ctx->trace_histories, i); + struct spdk_trace_history *history; - if (g_verbose) { + history = spdk_get_per_lcore_history(ctx->trace_histories, i); + ctx->lcore_ports[i].in_history = history; + ctx->lcore_ports[i].valid = (history != NULL); + + if (g_verbose && history) { printf("Number of trace entries for lcore (%d): %ju\n", i, - ctx->lcore_ports[i].in_history->num_entries); + history->num_entries); } } @@ -149,6 +154,10 @@ output_trace_files_prepare(struct aggr_trace_record_ctx *ctx, const char *aggr_p for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { port_ctx = &ctx->lcore_ports[i]; + if (!port_ctx->valid) { + continue; + } + port_ctx->fd = open(port_ctx->lcore_file, flags, 0600); if (port_ctx->fd < 0) { fprintf(stderr, "Could not open lcore file %s.\n", port_ctx->lcore_file); @@ -432,6 +441,7 @@ trace_files_aggregate(struct aggr_trace_record_ctx *ctx) uint64_t lcore_offsets[SPDK_TRACE_MAX_LCORE + 1]; int rc, i; ssize_t len = 0; + uint64_t current_offset; uint64_t len_sum; ctx->out_fd = open(ctx->out_file, flags, 0600); @@ -453,11 +463,17 @@ trace_files_aggregate(struct aggr_trace_record_ctx *ctx) } /* Update and append lcore offsets converged trace file */ - lcore_offsets[0] = sizeof(struct spdk_trace_flags); - for (i = 1; i < (int)SPDK_COUNTOF(lcore_offsets); i++) { - lcore_offsets[i] = spdk_get_trace_history_size(ctx->lcore_ports[i - 1].num_entries) + - lcore_offsets[i - 1]; + current_offset = sizeof(struct spdk_trace_flags); + for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { + lcore_port = &ctx->lcore_ports[i]; + if (lcore_port->valid) { + lcore_offsets[i] = current_offset; + current_offset += spdk_get_trace_history_size(lcore_port->num_entries); + } else { + lcore_offsets[i] = 0; + } } + lcore_offsets[SPDK_TRACE_MAX_LCORE] = current_offset; rc = cont_write(ctx->out_fd, lcore_offsets, sizeof(lcore_offsets)); if (rc < 0) { @@ -469,6 +485,10 @@ trace_files_aggregate(struct aggr_trace_record_ctx *ctx) for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { lcore_port = &ctx->lcore_ports[i]; + if (!lcore_port->valid) { + continue; + } + lcore_port->out_history->num_entries = lcore_port->num_entries; rc = cont_write(ctx->out_fd, lcore_port->out_history, sizeof(struct spdk_trace_history)); if (rc < 0) { @@ -493,6 +513,9 @@ trace_files_aggregate(struct aggr_trace_record_ctx *ctx) } } + /* Clear rc so that the last cont_write() doesn't get interpreted as a failure. */ + rc = 0; + if (len_sum != lcore_port->num_entries * sizeof(struct spdk_trace_entry)) { fprintf(stderr, "Len of lcore trace file doesn't match number of entries for lcore\n"); } @@ -639,6 +662,9 @@ main(int argc, char **argv) for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { lcore_port = &ctx.lcore_ports[i]; + if (!lcore_port->valid) { + continue; + } rc = lcore_trace_record(lcore_port); if (rc) { break; diff --git a/include/spdk/trace.h b/include/spdk/trace.h index 8ba901292..b8e20f164 100644 --- a/include/spdk/trace.h +++ b/include/spdk/trace.h @@ -157,16 +157,18 @@ spdk_get_trace_histories_size(struct spdk_trace_histories *trace_histories) static inline struct spdk_trace_history * spdk_get_per_lcore_history(struct spdk_trace_histories *trace_histories, unsigned lcore) { - char *lcore_history_offset; + uint64_t lcore_history_offset; if (lcore >= SPDK_TRACE_MAX_LCORE) { return NULL; } - lcore_history_offset = (char *)trace_histories; - lcore_history_offset += trace_histories->flags.lcore_history_offsets[lcore]; + lcore_history_offset = trace_histories->flags.lcore_history_offsets[lcore]; + if (lcore_history_offset == 0) { + return NULL; + } - return (struct spdk_trace_history *)lcore_history_offset; + return (struct spdk_trace_history *)(((char *)trace_histories) + lcore_history_offset); } void _spdk_trace_record(uint64_t tsc, uint16_t tpoint_id, uint16_t poller_id, diff --git a/lib/trace/trace.c b/lib/trace/trace.c index f1fe8c019..7409d1a6b 100644 --- a/lib/trace/trace.c +++ b/lib/trace/trace.c @@ -11,6 +11,7 @@ #include "spdk/util.h" #include "spdk/barrier.h" #include "spdk/log.h" +#include "spdk/cpuset.h" static int g_trace_fd = -1; static char g_shm_name[64]; @@ -142,20 +143,24 @@ _spdk_trace_record(uint64_t tsc, uint16_t tpoint_id, uint16_t poller_id, uint32_ int spdk_trace_init(const char *shm_name, uint64_t num_entries) { - int i = 0; + uint32_t i = 0; int histories_size; - uint64_t lcore_offsets[SPDK_TRACE_MAX_LCORE + 1]; + uint64_t lcore_offsets[SPDK_TRACE_MAX_LCORE + 1] = { 0 }; + struct spdk_cpuset cpuset = {}; /* 0 entries requested - skip trace initialization */ if (num_entries == 0) { return 0; } - lcore_offsets[0] = sizeof(struct spdk_trace_flags); - for (i = 1; i < (int)SPDK_COUNTOF(lcore_offsets); i++) { - lcore_offsets[i] = spdk_get_trace_history_size(num_entries) + lcore_offsets[i - 1]; + spdk_cpuset_zero(&cpuset); + histories_size = sizeof(struct spdk_trace_flags); + SPDK_ENV_FOREACH_CORE(i) { + spdk_cpuset_set_cpu(&cpuset, i, true); + lcore_offsets[i] = histories_size; + histories_size += spdk_get_trace_history_size(num_entries); } - histories_size = lcore_offsets[SPDK_TRACE_MAX_LCORE]; + lcore_offsets[SPDK_TRACE_MAX_LCORE] = histories_size; snprintf(g_shm_name, sizeof(g_shm_name), "%s", shm_name); @@ -202,6 +207,10 @@ spdk_trace_init(const char *shm_name, uint64_t num_entries) struct spdk_trace_history *lcore_history; g_trace_flags->lcore_history_offsets[i] = lcore_offsets[i]; + if (lcore_offsets[i] == 0) { + continue; + } + assert(spdk_cpuset_get_cpu(&cpuset, i)); lcore_history = spdk_get_per_lcore_history(g_trace_histories, i); lcore_history->lcore = i; lcore_history->num_entries = num_entries; @@ -228,7 +237,7 @@ trace_init_err: void spdk_trace_cleanup(void) { - bool unlink; + bool unlink = true; int i; struct spdk_trace_history *lcore_history; @@ -243,6 +252,9 @@ spdk_trace_cleanup(void) */ for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { lcore_history = spdk_get_per_lcore_history(g_trace_histories, i); + if (lcore_history == NULL) { + continue; + } unlink = lcore_history->entries[0].tsc == 0; if (!unlink) { break; diff --git a/lib/trace_parser/trace.cpp b/lib/trace_parser/trace.cpp index 64b97122c..593004cb3 100644 --- a/lib/trace_parser/trace.cpp +++ b/lib/trace_parser/trace.cpp @@ -97,9 +97,8 @@ spdk_trace_parser::entry_count(uint16_t lcore) const } history = spdk_get_per_lcore_history(_histories, lcore); - assert(history); - return history->num_entries; + return history == NULL ? 0 : history->num_entries; } spdk_trace_entry_buffer * @@ -340,8 +339,7 @@ spdk_trace_parser::init(const spdk_trace_parser_opts *opts) if (opts->lcore == SPDK_TRACE_MAX_LCORE) { for (i = 0; i < SPDK_TRACE_MAX_LCORE; i++) { history = spdk_get_per_lcore_history(_histories, i); - assert(history); - if (history->num_entries == 0 || history->entries[0].tsc == 0) { + if (history == NULL || history->num_entries == 0 || history->entries[0].tsc == 0) { continue; } @@ -349,7 +347,11 @@ spdk_trace_parser::init(const spdk_trace_parser_opts *opts) } } else { history = spdk_get_per_lcore_history(_histories, opts->lcore); - assert(history); + if (history == NULL) { + SPDK_ERRLOG("Trace file %s has no trace history for lcore %d\n", + opts->filename, opts->lcore); + return false; + } if (history->num_entries > 0 && history->entries[0].tsc != 0) { populate_events(history, history->num_entries); } diff --git a/mk/spdk.unittest.mk b/mk/spdk.unittest.mk index 60307eb20..dbd051360 100644 --- a/mk/spdk.unittest.mk +++ b/mk/spdk.unittest.mk @@ -25,7 +25,7 @@ CFLAGS += -ffunction-sections CFLAGS += -DSPDK_UNIT_TEST=1 LDFLAGS += -Wl,--gc-sections -SPDK_LIB_LIST += thread util log trace +SPDK_LIB_LIST += thread trace util log LIBS += -lcunit $(SPDK_STATIC_LIB_LINKER_ARGS)