diff --git a/include/spdk/trace.h b/include/spdk/trace.h index 45a9d895c..d0f6cb8b7 100644 --- a/include/spdk/trace.h +++ b/include/spdk/trace.h @@ -190,8 +190,9 @@ void spdk_trace_set_tpoint_group_mask(uint64_t tpoint_group_mask); * human-readable format. * * \param shm_name Name of shared memory. + * \return 0 on success, else non-zero indicates a failure. */ -void spdk_trace_init(const char *shm_name); +int spdk_trace_init(const char *shm_name); /** * Unmap global trace memory structs. diff --git a/lib/event/app.c b/lib/event/app.c index fd9776987..3ac1e7fec 100644 --- a/lib/event/app.c +++ b/lib/event/app.c @@ -277,7 +277,7 @@ int spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, void *arg1, void *arg2) { - struct spdk_conf *config; + struct spdk_conf *config = NULL; struct spdk_conf_section *sp; char shm_name[64]; int rc; @@ -318,13 +318,11 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, rc = spdk_conf_read(config, opts->config_file); if (rc != 0) { SPDK_ERRLOG("Could not read config file %s\n", opts->config_file); - spdk_conf_free(config); - return 1; + goto app_start_conf_free_err; } if (spdk_conf_first_section(config) == NULL) { SPDK_ERRLOG("Invalid config file %s\n", opts->config_file); - spdk_conf_free(config); - return 1; + goto app_start_conf_free_err; } } spdk_conf_set_as_default(config); @@ -369,9 +367,7 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, if (spdk_env_init(&env_opts) < 0) { SPDK_ERRLOG("Unable to initialize SPDK env\n"); - spdk_log_close(); - spdk_conf_free(g_spdk_app.config); - return 1; + goto app_start_log_close_err; } SPDK_NOTICELOG("Total cores available: %d\n", spdk_env_get_core_count()); @@ -383,24 +379,25 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, */ if ((rc = spdk_reactors_init(opts->max_delay_us)) != 0) { SPDK_ERRLOG("Invalid reactor mask.\n"); - spdk_log_close(); - spdk_conf_free(g_spdk_app.config); - return 1; - } - - if ((rc = spdk_app_setup_signal_handlers(opts)) != 0) { - spdk_conf_free(g_spdk_app.config); - spdk_log_close(); - return 1; + goto app_start_log_close_err; } + /* + * Note the call to spdk_trace_init() is located here + * ahead of spdk_app_setup_signal_handlers(). + * That's because there is not an easy/direct clean + * way of unwinding alloc'd resources that can occur + * in spdk_app_setup_signal_handlers(). + */ if (opts->shm_id >= 0) { snprintf(shm_name, sizeof(shm_name), "/%s_trace.%d", opts->name, opts->shm_id); } else { snprintf(shm_name, sizeof(shm_name), "/%s_trace.pid%d", opts->name, (int)getpid()); } - spdk_trace_init(shm_name); + if (spdk_trace_init(shm_name) != 0) { + goto app_start_log_close_err; + } if (opts->tpoint_group_mask == NULL) { sp = spdk_conf_find_section(g_spdk_app.config, "Global"); @@ -424,6 +421,10 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, } } + if ((rc = spdk_app_setup_signal_handlers(opts)) != 0) { + goto app_start_trace_cleanup_err; + } + g_spdk_app.rc = 0; g_init_lcore = spdk_env_get_current_core(); g_app_start_fn = start_fn; @@ -437,6 +438,17 @@ spdk_app_start(struct spdk_app_opts *opts, spdk_event_fn start_fn, spdk_reactors_start(); return g_spdk_app.rc; + +app_start_trace_cleanup_err: + spdk_trace_cleanup(); + +app_start_log_close_err: + spdk_log_close(); + +app_start_conf_free_err: + spdk_conf_free(config); + + return 1; } void diff --git a/lib/trace/trace.c b/lib/trace/trace.c index 9102f242e..7c0d63f27 100644 --- a/lib/trace/trace.c +++ b/lib/trace/trace.c @@ -37,6 +37,7 @@ #include "spdk/string.h" #include "spdk/trace.h" +static int g_trace_fd = -1; static char g_shm_name[64]; static struct spdk_trace_histories *g_trace_histories; @@ -84,31 +85,30 @@ spdk_trace_record(uint16_t tpoint_id, uint16_t poller_id, uint32_t size, } } -void +int spdk_trace_init(const char *shm_name) { - int trace_fd; int i = 0; snprintf(g_shm_name, sizeof(g_shm_name), "%s", shm_name); - trace_fd = shm_open(shm_name, O_RDWR | O_CREAT, 0600); - if (trace_fd == -1) { + g_trace_fd = shm_open(shm_name, O_RDWR | O_CREAT, 0600); + if (g_trace_fd == -1) { fprintf(stderr, "could not shm_open spdk_trace\n"); fprintf(stderr, "errno=%d %s\n", errno, spdk_strerror(errno)); - exit(EXIT_FAILURE); + return 1; } - if (ftruncate(trace_fd, sizeof(*g_trace_histories)) != 0) { + if (ftruncate(g_trace_fd, sizeof(*g_trace_histories)) != 0) { fprintf(stderr, "could not truncate shm\n"); - exit(EXIT_FAILURE); + goto trace_init_err; } g_trace_histories = mmap(NULL, sizeof(*g_trace_histories), PROT_READ | PROT_WRITE, - MAP_SHARED, trace_fd, 0); + MAP_SHARED, g_trace_fd, 0); if (g_trace_histories == NULL) { fprintf(stderr, "could not mmap shm\n"); - exit(EXIT_FAILURE); + goto trace_init_err; } memset(g_trace_histories, 0, sizeof(*g_trace_histories)); @@ -122,6 +122,16 @@ spdk_trace_init(const char *shm_name) } spdk_trace_flags_init(); + + return 0; + +trace_init_err: + close(g_trace_fd); + g_trace_fd = -1; + shm_unlink(shm_name); + + return 1; + } void @@ -130,6 +140,7 @@ spdk_trace_cleanup(void) if (g_trace_histories) { munmap(g_trace_histories, sizeof(struct spdk_trace_histories)); g_trace_histories = NULL; + close(g_trace_fd); } shm_unlink(g_shm_name); }