[ltt-dev] [UST PATCH] libust: Replace the the two trace lists with a single list
Nils Carlson
nils.carlson at ericsson.com
Thu Mar 17 06:35:33 EDT 2011
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 <nils.carlson at ericsson.com>
---
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
More information about the lttng-dev
mailing list