Get rid of the two trace lists and introduce a state into each trace. This simplifies things a bit as two lists don't have to be searched to know if a trace exists. Also, if traces pass through more states in the future new states will be easier to add.
Signed-off-by: Nils Carlson <[email protected]> --- libust/serialize.c | 3 +- libust/tracectl.c | 33 +++++++----------- libust/tracer.c | 81 ++++++++++++++++++++++++--------------------- libust/tracer.h | 14 +++++++- libust/tracercore.c | 3 +- libust/tracercore.h | 3 +- libust/type-serializer.c | 2 +- 7 files changed, 73 insertions(+), 66 deletions(-) diff --git a/libust/serialize.c b/libust/serialize.c index 8aa3f4b..f1620d1 100644 --- a/libust/serialize.c +++ b/libust/serialize.c @@ -685,7 +685,8 @@ notrace void ltt_vtrace(const struct marker *mdata, void *probe_data, va_end(args_copy); /* Iterate on each trace */ - cds_list_for_each_entry_rcu(trace, <t_traces.head, list) { + cds_list_for_each_entry_rcu(trace, <t_traces.trace_list, list) { + /* * Expect the filter to filter out events. If we get here, * we went through tracepoint activation as a first step. diff --git a/libust/tracectl.c b/libust/tracectl.c index 33c7280..628b19a 100644 --- a/libust/tracectl.c +++ b/libust/tracectl.c @@ -214,7 +214,7 @@ static void inform_consumer_daemon(const char *trace_name) ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (trace == NULL) { WARN("inform_consumer_daemon: could not find trace \"%s\"; it is probably already destroyed", trace_name); goto unlock_traces; @@ -266,7 +266,7 @@ static int get_buffer_shmid_pipe_fd(const char *trace_name, const char *ch_name, DBG("get_buffer_shmid_pipe_fd"); ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); ltt_unlock_traces(); if (trace == NULL) { @@ -298,7 +298,7 @@ static int get_subbuf_num_size(const char *trace_name, const char *ch_name, DBG("get_subbuf_size"); ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace(trace_name); ltt_unlock_traces(); if (!trace) { @@ -348,9 +348,9 @@ static int set_subbuf_size(const char *trace_name, const char *ch_name, } ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (trace == NULL) { - ERR("cannot find trace!"); + ERR("cannot find trace %s in state setup!", trace_name); retval = -ENODATA; goto unlock_traces; } @@ -386,7 +386,7 @@ static int set_subbuf_num(const char *trace_name, const char *ch_name, } ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (trace == NULL) { ERR("cannot find trace!"); retval = -ENODATA; @@ -421,7 +421,7 @@ static int get_subbuffer(const char *trace_name, const char *ch_name, *consumed_old = 0; ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (!trace) { DBG("Cannot find trace. It was likely destroyed by the user."); @@ -462,7 +462,7 @@ static int notify_buffer_mapped(const char *trace_name, DBG("get_buffer_fd"); ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (!trace) { retval = -ENODATA; @@ -504,7 +504,7 @@ static int put_subbuffer(const char *trace_name, const char *ch_name, DBG("put_subbuf"); ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (!trace) { retval = -ENODATA; @@ -557,7 +557,7 @@ static int force_subbuf_switch(const char *trace_name) int i, j, retval = 0; ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (!trace) { retval = -ENODATA; DBG("Cannot find trace. It was likely destroyed by the user."); @@ -1488,7 +1488,7 @@ static int trace_recording(void) ltt_lock_traces(); - cds_list_for_each_entry(trace, <t_traces.head, list) { + cds_list_for_each_entry(trace, <t_traces.trace_list, list) { if (trace->active) { retval = 1; break; @@ -1601,14 +1601,6 @@ static void ust_fork(void) /* Get the pid of the new process */ processpid = getpid(); - /* - * FIXME: This could be prettier, we loop over the list twice and - * following good locking practice should lock around the loop - */ - cds_list_for_each_entry_safe(trace, trace_tmp, <t_traces.head, list) { - ltt_trace_stop(trace->trace_name); - } - /* Delete all active connections, but leave them in the epoll set */ cds_list_for_each_entry_safe(sock, sock_tmp, &ust_socks, list) { ustcomm_del_sock(sock, 1); @@ -1618,7 +1610,8 @@ static void ust_fork(void) * FIXME: This could be prettier, we loop over the list twice and * following good locking practice should lock around the loop */ - cds_list_for_each_entry_safe(trace, trace_tmp, <t_traces.head, list) { + cds_list_for_each_entry_safe(trace, trace_tmp, <t_traces.trace_list, list) { + ltt_trace_stop(trace->trace_name); ltt_trace_destroy(trace->trace_name, 1); } diff --git a/libust/tracer.c b/libust/tracer.c index 3b4fae4..4cefc02 100644 --- a/libust/tracer.c +++ b/libust/tracer.c @@ -139,33 +139,37 @@ static void trace_async_wakeup(struct ust_trace *trace) } /** - * _ltt_trace_find - find a trace by given name. + * find_trace - find a trace by given name. * trace_name: trace name * * Returns a pointer to the trace structure, NULL if not found. + * Locking: Must be called with the traces mutex held. */ -struct ust_trace *_ltt_trace_find(const char *trace_name) +struct ust_trace *find_trace(const char *trace_name) { struct ust_trace *trace; - cds_list_for_each_entry(trace, <t_traces.head, list) + cds_list_for_each_entry(trace, <t_traces.trace_list, list) if (!strncmp(trace->trace_name, trace_name, NAME_MAX)) return trace; return NULL; } - -/* _ltt_trace_find_setup : - * find a trace in setup list by given name. +/** + * find_trace_in_state - find a trace by given name and state + * trace_name: trace name + * trace_state: trace state * * Returns a pointer to the trace structure, NULL if not found. + * Locking: Must be called with the traces mutex held. */ -struct ust_trace *_ltt_trace_find_setup(const char *trace_name) +struct ust_trace *find_trace_with_state(const char *trace_name, int trace_state) { struct ust_trace *trace; - cds_list_for_each_entry(trace, <t_traces.setup_head, list) - if (!strncmp(trace->trace_name, trace_name, NAME_MAX)) + cds_list_for_each_entry(trace, <t_traces.trace_list, list) + if (trace_state == trace->state && + !strncmp(trace->trace_name, trace_name, NAME_MAX)) return trace; return NULL; @@ -215,13 +219,7 @@ int _ltt_trace_setup(const char *trace_name) unsigned int chan; enum ltt_channels chantype; - if (_ltt_trace_find_setup(trace_name)) { - ERR("Trace name %s already used", trace_name); - err = -EEXIST; - goto traces_error; - } - - if (_ltt_trace_find(trace_name)) { + if (find_trace(trace_name)) { ERR("Trace name %s already used", trace_name); err = -EEXIST; goto traces_error; @@ -266,7 +264,9 @@ int _ltt_trace_setup(const char *trace_name) chan_infos[chantype].def_subbufcount; } - cds_list_add(&new_trace->list, <t_traces.setup_head); + new_trace->state = TRACE_SETUP; + + cds_list_add(&new_trace->list, <t_traces.trace_list); return 0; trace_free: @@ -300,7 +300,7 @@ int ltt_trace_set_type(const char *trace_name, const char *trace_type) ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (!trace) { ERR("Trace not found %s", trace_name); err = -ENOENT; @@ -338,9 +338,10 @@ int ltt_trace_set_channel_subbufsize(const char *trace_name, ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (!trace) { - ERR("Trace not found %s", trace_name); + ERR("Trace %s has not been setup or does not exist", + trace_name); err = -ENOENT; goto traces_error; } @@ -367,9 +368,10 @@ int ltt_trace_set_channel_subbufcount(const char *trace_name, ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (!trace) { - ERR("Trace not found %s", trace_name); + ERR("Trace %s has not been setup or does not exist", + trace_name); err = -ENOENT; goto traces_error; } @@ -396,9 +398,10 @@ int ltt_trace_set_channel_enable(const char *trace_name, ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (!trace) { - ERR("Trace not found %s", trace_name); + ERR("Trace %s has not been setup or does not exist", + trace_name); err = -ENOENT; goto traces_error; } @@ -436,9 +439,10 @@ int ltt_trace_set_channel_overwrite(const char *trace_name, ltt_lock_traces(); - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (!trace) { - ERR("Trace not found %s", trace_name); + ERR("Trace %s has not been setup or does not exist", + trace_name); err = -ENOENT; goto traces_error; } @@ -479,15 +483,17 @@ int ltt_trace_alloc(const char *trace_name) ltt_lock_traces(); - if (_ltt_trace_find(trace_name)) { /* Trace already allocated */ - err = 1; + trace = find_trace(trace_name); + if (!trace) { + ERR("Trace %s does not exist", + trace_name); + err = -ENOENT; goto traces_error; } - trace = _ltt_trace_find_setup(trace_name); - if (!trace) { - ERR("Trace not found %s", trace_name); - err = -ENOENT; + if (trace->state == TRACE_ALLOCATED) { + /* Trace was already allocated, return 1 */ + err = 1; goto traces_error; } @@ -528,8 +534,7 @@ int ltt_trace_alloc(const char *trace_name) } } - cds_list_del(&trace->list); - cds_list_add_rcu(&trace->list, <t_traces.head); + trace->state = TRACE_ALLOCATED; ltt_unlock_traces(); @@ -610,7 +615,7 @@ int ltt_trace_destroy(const char *trace_name, int drop) ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); if (trace) { err = _ltt_trace_destroy(trace); if (err) @@ -623,7 +628,7 @@ int ltt_trace_destroy(const char *trace_name, int drop) return 0; } - trace = _ltt_trace_find_setup(trace_name); + trace = find_trace_with_state(trace_name, TRACE_SETUP); if (trace) { _ltt_trace_free(trace); ltt_unlock_traces(); @@ -664,7 +669,7 @@ int ltt_trace_start(const char *trace_name) ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); err = _ltt_trace_start(trace); if (err) goto no_trace; @@ -716,7 +721,7 @@ int ltt_trace_stop(const char *trace_name) struct ust_trace *trace; ltt_lock_traces(); - trace = _ltt_trace_find(trace_name); + trace = find_trace_with_state(trace_name, TRACE_ALLOCATED); err = _ltt_trace_stop(trace); ltt_unlock_traces(); return err; diff --git a/libust/tracer.h b/libust/tracer.h index 2f489f6..16b1e1a 100644 --- a/libust/tracer.h +++ b/libust/tracer.h @@ -167,6 +167,12 @@ enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID }; #define CHANNEL_FLAG_ENABLE (1U<<0) #define CHANNEL_FLAG_OVERWRITE (1U<<1) +enum trace_states { + TRACE_CREATED, + TRACE_SETUP, + TRACE_ALLOCATED, +}; + /* Per-trace information - each trace/flight recorder represented by one */ struct ust_trace { /* First 32 bytes cache-hot cacheline */ @@ -179,6 +185,8 @@ struct ust_trace { u32 freq_scale; u64 start_freq; u64 start_tsc; + /* End of cache-hot section */ + int state; unsigned long long start_monotonic; struct timeval start_time; struct ltt_channel_setting *settings; @@ -409,7 +417,7 @@ union ltt_control_args { extern int _ltt_trace_setup(const char *trace_name); extern int ltt_trace_setup(const char *trace_name); -extern struct ust_trace *_ltt_trace_find_setup(const char *trace_name); + extern int ltt_trace_set_type(const char *trace_name, const char *trace_type); extern int ltt_trace_set_channel_subbufsize(const char *trace_name, const char *channel_name, unsigned int size); @@ -450,6 +458,8 @@ extern void ltt_dump_marker_state(struct ust_trace *trace); extern void ltt_lock_traces(void); extern void ltt_unlock_traces(void); -extern struct ust_trace *_ltt_trace_find(const char *trace_name); +extern struct ust_trace *find_trace(const char *trace_name); + +extern struct ust_trace *find_trace_with_state(const char *trace_name, int state); #endif /* _LTT_TRACER_H */ diff --git a/libust/tracercore.c b/libust/tracercore.c index 1e418b6..1532c88 100644 --- a/libust/tracercore.c +++ b/libust/tracercore.c @@ -22,8 +22,7 @@ /* Traces structures */ struct ltt_traces ltt_traces = { - .setup_head = CDS_LIST_HEAD_INIT(ltt_traces.setup_head), - .head = CDS_LIST_HEAD_INIT(ltt_traces.head), + .trace_list = CDS_LIST_HEAD_INIT(ltt_traces.trace_list), }; /* Traces list writer locking */ diff --git a/libust/tracercore.h b/libust/tracercore.h index 9673cca..c5c81fd 100644 --- a/libust/tracercore.h +++ b/libust/tracercore.h @@ -32,8 +32,7 @@ * list. */ struct ltt_traces { - struct cds_list_head setup_head; /* Pre-allocated traces list */ - struct cds_list_head head; /* Allocated Traces list */ + struct cds_list_head trace_list; /* Allocated Traces list */ unsigned int num_active_traces; /* Number of active traces */ } ____cacheline_aligned; diff --git a/libust/type-serializer.c b/libust/type-serializer.c index dcaea1e..352f9be 100644 --- a/libust/type-serializer.c +++ b/libust/type-serializer.c @@ -60,7 +60,7 @@ void _ltt_specialized_trace(const struct marker *mdata, void *probe_data, * Iterate on each trace, typically small number of active traces, * list iteration with prefetch is usually slower. */ - cds_list_for_each_entry_rcu(trace, <t_traces.head, list) { + cds_list_for_each_entry_rcu(trace, <t_traces.trace_list, list) { if (unlikely(!trace->active)) continue; //ust// if (unlikely(!ltt_run_filter(trace, eID))) -- 1.7.1 _______________________________________________ ltt-dev mailing list [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
