[ltt-dev] [PATCH] ltt-channel: permanent eID
Mathieu Desnoyers
compudj at krystal.dyndns.org
Wed Dec 17 23:42:59 EST 2008
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>
Thanks for this description, comments below,
> changelog:
>
> Marker ID and its data must be permanent for text output, this patch
> introduce permanent eID.
>
> 1) per channel permanent eID
>
> Basic struct for store eID's data is struct marker_entry. When a channel
> is activated, all the struct marker_entry for this channel are permanent
> until this channel is deactivated. In this period, event if the marker
> is leaved or other things are happened, the eID is permanent.
>
Ok, so you keep the struct marker_entry around until the channel is
unused (all traces are destroyed, including that all in-kernel ascii
converters have released the trace) ?
> This patch introduces 4 API for permanent eID:
> int start_trace_channel(const char *channel); - activate a channel
> int stop_trace_channel(const char *channel); - deactivate a channel
Hrm, can we change the names for
get_trace_channel()
put_trace_channel() ?
start and stop sounds a bit misleading with the tracing start/stop
semantics.
Also, why do we have "waste" ids ? This is bad, because whenever
possible, we should fit IDs in ~28 IDs allowed for "compact" event ids.
If the IDs are larger than this, the tracer will have to generate an
extended header and waste trace space. Do we really _need_ such id waste?
> const char *get_name_from_eid(const char *channel, u16 eID); - get name from eID
> const char *get_fmt_from_eid(const char *channel, u16 eID); - get fmt from eID
>
> See ltt/ltt-tracer.c for the using of start/stop_trace_channel().
>
hrm, stop_trace_channel (or put_trace_channel) is in __destroy, but
should probably be in the kref desctructor, so we are sure that it's
only called with all references to the trace data, including the
in-kernel ascii converters, have released their reference.
> When a channel is deactivated(its trace_count come to 0), we will defrag eID
> for this channel. AND we use "trace_mark(metadata, core_marker_id," to tell
> userspace that id is reset.
Sounds good.
>
> 2) per channel struct marker_entry(permanent eID' data) management
>
> All struct marker_entry of a channel are threaded by struct list_head.
>
> It will help us remove a struct marker_entry quickly and help us manage
> entries quickly(defrag_id(), remove_chanel_marker_entries())
>
> We also call marker_update_probes() in defrag_id().
>
Hrm, aren't we taking the marker mutex twice here ? I am probably
missing something.
> Since struct marker_entry is permanent eID' data. We move the definition
> of struct marker_entry from marker.c to ltt-channel.h
>
BTW, I would not use the term "permanent" for this. Because permanent,
to me, really means "we allocate something that we will only free when
the machine reboots". We are simply extending the life of the the eIDs
for the duration of all references to those IDs.
Let's say that we extend the event ID lifetime to the overall event ID
references, which now includes trace references, which includes ascii
converters.
> 3) as previous code, we defrag channel index when all tracer is stopped.
>
> different: As described in 1), permanent eID will defraged when a
> channel is deactivated.
>
> It means this patch has more fine-grained ID management.
>
There are two cases here :
Tracing starts
add/remove markers many times
Tracing stops -> defrag
And
No tracing
add/remove markers many times -> defrag each time
You don't say much about what is done at marker add/remove when tracing
is inactive. Is there some compaction done ?
> 4) remove ltt_channel_mutex.
>
> Almost all function need lock_markers(), we should remove channel_mutex.
>
Yep, sounds simpler and those two are pretty much coupled anyway.
Mathieu
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
>
> Mathieu Desnoyers wrote:
> > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >> introduce permanent eID for text output, and change
> >> the ltt-channel's management.
> >>
> >
> > Hi Lai,
> >
> > I'd like to review this work, but unfortunately, some documentation is
> > missing. Would it be possible to describe :
> >
> > - The reason why each change is done.
> > - How each change is done.
> > - Modification done to locking (mutex changes, order of mutexes...)
> > - Why the "change to ltt-channel's management" is required.
> >
> > Both in a complete changelog message and with added comments in the
> > code.
> >
> > Thanks,
> >
> > Mathieu
> >
> >
> >> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >> ---
> >> include/linux/ltt-channels.h | 45 ++++++-
> >> include/linux/marker.h | 4
> >> kernel/marker.c | 95 +++------------
> >> ltt/ltt-channels.c | 271 +++++++++++++++++++++++++------------------
> >> ltt/ltt-tracer.c | 9 +
> >> 5 files changed, 230 insertions(+), 194 deletions(-)
> >> diff --git a/include/linux/ltt-channels.h b/include/linux/ltt-channels.h
> >> index ce19eda..edf959b 100644
> >> --- a/include/linux/ltt-channels.h
> >> +++ b/include/linux/ltt-channels.h
> >> @@ -8,8 +8,10 @@
> >> */
> >>
> >> #include <linux/limits.h>
> >> +#include <linux/marker.h>
> >>
> >> #define EVENTS_PER_CHANNEL 65536
> >> +#define EVENTS_MOST_WASTE_IDS 128
> >>
> >> struct ltt_trace_struct;
> >> struct rchan_buf;
> >> @@ -51,15 +53,51 @@ struct ltt_channel_struct {
> >> struct ltt_channel_setting {
> >> unsigned int subbuf_size;
> >> unsigned int subbuf_cnt;
> >> - struct kref kref; /* Number of references to structure content */
> >> + int trace_count;
> >> + int live_count;
> >> struct list_head list;
> >> + struct list_head marker_entry;
> >> unsigned int index; /* index of channel in trace channel array */
> >> u16 free_event_id; /* Next event ID to allocate */
> >> char name[PATH_MAX];
> >> };
> >>
> >> -int ltt_channels_register(const char *name);
> >> -int ltt_channels_unregister(const char *name);
> >> +/*
> >> + * Note about RCU :
> >> + * It is used to make sure every handler has finished using its private data
> >> + * between two consecutive operation (add or remove) on a given marker. It is
> >> + * also used to delay the free of multiple probes array until a quiescent state
> >> + * is reached.
> >> + * marker entries modifications are protected by the markers_mutex.
> >> + */
> >> +struct marker_entry {
> >> + struct hlist_node hlist;
> >> + char *format;
> >> + char *name;
> >> + /* Probe wrapper */
> >> + void (*call)(const struct marker *mdata, void *call_private, ...);
> >> + struct marker_probe_closure single;
> >> + struct marker_probe_closure *multi;
> >> + int refcount; /* Number of times armed. 0 if disarmed. */
> >> + struct rcu_head rcu;
> >> + void *oldptr;
> >> + int rcu_pending;
> >> + u16 channel_id;
> >> + u16 event_id;
> >> + unsigned char ptype:1;
> >> + unsigned char format_allocated:1;
> >> + unsigned char used:1;
> >> + struct list_head sibling; /* sibling of channel's maker_entry */
> >> + struct ltt_channel_setting *lcs;
> >> + char channel[0]; /* Contains channel'\0'name'\0'format'\0' */
> >> +};
> >> +int __makrer_set_channel(struct marker_entry *entry, const char *channel);
> >> +
> >> +int start_trace_channel(const char *channel);
> >> +int stop_trace_channel(const char *channel);
> >> +const char *get_name_from_eid(const char *channel, u16 eID);
> >> +const char *get_fmt_from_eid(const char *channel, u16 eID);
> >> +
> >> int ltt_channels_set_default(const char *name,
> >> unsigned int subbuf_size,
> >> unsigned int subbuf_cnt);
> >> @@ -69,7 +107,6 @@ struct ltt_channel_struct *ltt_channels_trace_alloc(unsigned int *nr_channels,
> >> int overwrite,
> >> int active);
> >> void ltt_channels_trace_free(struct ltt_channel_struct *channels);
> >> -int _ltt_channels_get_event_id(const char *channel, const char *name);
> >> int ltt_channels_get_event_id(const char *channel, const char *name);
> >>
> >> #endif /* _LTT_CHANNELS_H */
> >> diff --git a/include/linux/marker.h b/include/linux/marker.h
> >> index 32387e8..82e4f7c 100644
> >> --- a/include/linux/marker.h
> >> +++ b/include/linux/marker.h
> >> @@ -15,7 +15,6 @@
> >> #include <stdarg.h>
> >> #include <linux/types.h>
> >> #include <linux/immediate.h>
> >> -#include <linux/ltt-channels.h>
> >>
> >> struct module;
> >> struct task_struct;
> >> @@ -126,6 +125,7 @@ struct marker {
> >>
> >> extern void marker_update_probe_range(struct marker *begin,
> >> struct marker *end);
> >> +extern void marker_update_probes(void);
> >>
> >> #define GET_MARKER(channel, name) (__mark_##channel##_##name)
> >>
> >> @@ -199,8 +199,6 @@ static inline void marker_update_probe_range(struct marker *begin,
> >> extern void lock_markers(void);
> >> extern void unlock_markers(void);
> >>
> >> -extern void markers_compact_event_ids(void);
> >> -
> >> /* To be used for string format validity checking with gcc */
> >> static inline void __printf(1, 2) ___mark_check_format(const char *fmt, ...)
> >> {
> >> diff --git a/kernel/marker.c b/kernel/marker.c
> >> index b11a82a..cd38ba0 100644
> >> --- a/kernel/marker.c
> >> +++ b/kernel/marker.c
> >> @@ -22,6 +22,7 @@
> >> #include <linux/list.h>
> >> #include <linux/rcupdate.h>
> >> #include <linux/marker.h>
> >> +#include <linux/ltt-channels.h>
> >> #include <linux/err.h>
> >> #include <linux/slab.h>
> >> #include <linux/immediate.h>
> >> @@ -60,33 +61,6 @@ void unlock_markers(void)
> >> #define MARKER_TABLE_SIZE (1 << MARKER_HASH_BITS)
> >> static struct hlist_head marker_table[MARKER_TABLE_SIZE];
> >>
> >> -/*
> >> - * Note about RCU :
> >> - * It is used to make sure every handler has finished using its private data
> >> - * between two consecutive operation (add or remove) on a given marker. It is
> >> - * also used to delay the free of multiple probes array until a quiescent state
> >> - * is reached.
> >> - * marker entries modifications are protected by the markers_mutex.
> >> - */
> >> -struct marker_entry {
> >> - struct hlist_node hlist;
> >> - char *format;
> >> - char *name;
> >> - /* Probe wrapper */
> >> - void (*call)(const struct marker *mdata, void *call_private, ...);
> >> - struct marker_probe_closure single;
> >> - struct marker_probe_closure *multi;
> >> - int refcount; /* Number of times armed. 0 if disarmed. */
> >> - struct rcu_head rcu;
> >> - void *oldptr;
> >> - int rcu_pending;
> >> - u16 channel_id;
> >> - u16 event_id;
> >> - unsigned char ptype:1;
> >> - unsigned char format_allocated:1;
> >> - char channel[0]; /* Contains channel'\0'name'\0'format'\0' */
> >> -};
> >> -
> >> #ifdef CONFIG_MARKERS_USERSPACE
> >> static void marker_update_processes(void);
> >> #else
> >> @@ -474,7 +448,6 @@ static int remove_marker(const char *channel, const char *name)
> >> size_t channel_len = strlen(channel) + 1;
> >> size_t name_len = strlen(name) + 1;
> >> u32 hash;
> >> - int ret;
> >>
> >> hash = jhash(channel, channel_len-1, 0) ^ jhash(name, name_len-1, 0);
> >> head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
> >> @@ -488,11 +461,26 @@ static int remove_marker(const char *channel, const char *name)
> >> return -ENOENT;
> >> if (e->single.func != __mark_empty_function)
> >> return -EBUSY;
> >> +
> >> + if (e->lcs) {
> >> + e->lcs->live_count--;
> >> + if (!e->lcs->live_count && !e->lcs->trace_count) {
> >> + BUG_ON(!list_empty(&e->lcs->marker_entry));
> >> + list_del(&e->lcs->list);
> >> + kfree(e->lcs);
> >> + e->lcs = NULL;
> >> + }
> >> + }
> >> +
> >> + /* It will be released in remove_chanel_marker_entries() */
> >> + if (e->used && e->lcs && e->lcs->trace_count)
> >> + return 0;
> >> +
> >> hlist_del(&e->hlist);
> >> if (e->format_allocated)
> >> kfree(e->format);
> >> - ret = ltt_channels_unregister(e->channel);
> >> - WARN_ON(ret);
> >> + if (e->lcs)
> >> + list_del(&e->sibling);
> >> /* Make sure the call_rcu has been executed */
> >> if (e->rcu_pending)
> >> rcu_barrier_sched();
> >> @@ -540,6 +528,7 @@ static int set_marker(struct marker_entry *entry, struct marker *elem,
> >> if (ret)
> >> return ret;
> >> }
> >> + entry->used = 1;
> >>
> >> /*
> >> * probe_cb setup (statically known) is done here. It is
> >> @@ -695,7 +684,7 @@ void marker_update_probe_range(struct marker *begin,
> >> * Site effect : marker_set_format may delete the marker entry (creating a
> >> * replacement).
> >> */
> >> -static void marker_update_probes(void)
> >> +void marker_update_probes(void)
> >> {
> >> /* Core kernel markers */
> >> marker_update_probe_range(__start___markers, __stop___markers);
> >> @@ -738,18 +727,9 @@ int marker_probe_register(const char *channel, const char *name,
> >> ret = PTR_ERR(entry);
> >> if (ret)
> >> goto end;
> >> - ret = ltt_channels_register(channel);
> >> + ret = __makrer_set_channel(entry, channel);
> >> if (ret)
> >> goto error_remove_marker;
> >> - ret = ltt_channels_get_index_from_name(channel);
> >> - if (ret < 0)
> >> - goto error_unregister_channel;
> >> - entry->channel_id = ret;
> >> - ret = ltt_channels_get_event_id(channel, name);
> >> - if (ret < 0)
> >> - goto error_unregister_channel;
> >> - entry->event_id = ret;
> >> - ret = 0;
> >> trace_mark(metadata, core_marker_id,
> >> "channel %s name %s event_id %hu "
> >> "int #1u%zu long #1u%zu pointer #1u%zu "
> >> @@ -776,7 +756,7 @@ int marker_probe_register(const char *channel, const char *name,
> >> if (IS_ERR(old)) {
> >> ret = PTR_ERR(old);
> >> if (first_probe)
> >> - goto error_unregister_channel;
> >> + goto error_remove_marker;
> >> else
> >> goto end;
> >> }
> >> @@ -797,9 +777,6 @@ int marker_probe_register(const char *channel, const char *name,
> >> call_rcu_sched(&entry->rcu, free_old_closure);
> >> goto end;
> >>
> >> -error_unregister_channel:
> >> - ret_err = ltt_channels_unregister(channel);
> >> - WARN_ON(ret_err);
> >> error_remove_marker:
> >> ret_err = remove_marker(channel, name);
> >> WARN_ON(ret_err);
> >> @@ -997,34 +974,6 @@ void *marker_get_private_data(const char *channel, const char *name,
> >> }
> >> EXPORT_SYMBOL_GPL(marker_get_private_data);
> >>
> >> -/**
> >> - * markers_compact_event_ids: Compact markers event IDs and reassign channels
> >> - *
> >> - * Called when no channel users are active by the channel infrastructure.
> >> - * Called with lock_markers() and channel mutex held.
> >> - */
> >> -void markers_compact_event_ids(void)
> >> -{
> >> - struct marker_entry *entry;
> >> - unsigned int i;
> >> - struct hlist_head *head;
> >> - struct hlist_node *node;
> >> - int ret;
> >> -
> >> - for (i = 0; i < MARKER_TABLE_SIZE; i++) {
> >> - head = &marker_table[i];
> >> - hlist_for_each_entry(entry, node, head, hlist) {
> >> - ret = ltt_channels_get_index_from_name(entry->channel);
> >> - WARN_ON(ret < 0);
> >> - entry->channel_id = ret;
> >> - ret = _ltt_channels_get_event_id(entry->channel,
> >> - entry->name);
> >> - WARN_ON(ret < 0);
> >> - entry->event_id = ret;
> >> - }
> >> - }
> >> -}
> >> -
> >> #ifdef CONFIG_MODULES
> >>
> >> /**
> >> diff --git a/ltt/ltt-channels.c b/ltt/ltt-channels.c
> >> index c57e737..e876f04 100644
> >> --- a/ltt/ltt-channels.c
> >> +++ b/ltt/ltt-channels.c
> >> @@ -13,13 +13,12 @@
> >> #include <linux/ltt-channels.h>
> >> #include <linux/mutex.h>
> >> #include <linux/vmalloc.h>
> >> +#include <linux/marker.h>
> >> +#include <linux/err.h>
> >> +#include <linux/ltt-tracer.h>
> >> +
> >> +LIST_HEAD(ltt_channels);
> >>
> >> -/*
> >> - * ltt_channel_mutex may be nested inside the LTT trace mutex.
> >> - * ltt_channel_mutex mutex may be nested inside markers mutex.
> >> - */
> >> -static DEFINE_MUTEX(ltt_channel_mutex);
> >> -static LIST_HEAD(ltt_channels);
> >> /*
> >> * Index of next channel in array. Makes sure that as long as a trace channel is
> >> * allocated, no array index will be re-used when a channel is freed and then
> >> @@ -40,104 +39,165 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
> >> return NULL;
> >> }
> >>
> >> -/*
> >> - * Must be called when channel refcount falls to 0 _and_ also when the last
> >> - * trace is freed. This function is responsible for compacting the channel and
> >> - * event IDs when no users are active.
> >> - *
> >> - * Called with lock_markers() and channels mutex held.
> >> - */
> >> -static void release_channel_setting(struct kref *kref)
> >> -{
> >> - struct ltt_channel_setting *setting = container_of(kref,
> >> - struct ltt_channel_setting, kref);
> >> - struct ltt_channel_setting *iter;
> >> +static int _ltt_channels_get_event_id(struct ltt_channel_setting *setting,
> >> + const char *name);
> >>
> >> - if (atomic_read(&index_kref.refcount) == 0
> >> - && atomic_read(&setting->kref.refcount) == 0) {
> >> - list_del(&setting->list);
> >> - kfree(setting);
> >> +int __makrer_set_channel(struct marker_entry *entry, const char *channel)
> >> +{
> >> + struct ltt_channel_setting *setting = lookup_channel(channel);
> >> + if (!setting) {
> >> + setting = kzalloc(sizeof(*setting), GFP_KERNEL);
> >> + if (!setting)
> >> + return -ENOMEM;
> >> + list_add_tail(&setting->list, <t_channels);
> >> + strncpy(setting->name, channel, PATH_MAX-1);
> >> + setting->index = free_index++;
> >> + INIT_LIST_HEAD(&setting->marker_entry);
> >> + }else if (setting->free_event_id == EVENTS_PER_CHANNEL - 1)
> >> + return -ENOSPC;
> >> + setting->live_count++;
> >> + list_add(&entry->sibling, &setting->marker_entry);
> >> + entry->channel_id = setting->index;
> >> + entry->event_id = _ltt_channels_get_event_id(setting, entry->name);
> >> + return 0;
> >> +}
> >>
> >> - free_index = 0;
> >> - list_for_each_entry(iter, <t_channels, list) {
> >> - iter->index = free_index++;
> >> - iter->free_event_id = 0;
> >> +static void defrag_id(struct ltt_channel_setting *setting)
> >> +{
> >> + struct marker_entry *e;
> >> + int free_count = setting->free_event_id;
> >> + if (setting->live_count + EVENTS_MOST_WASTE_IDS > free_count)
> >> + return;
> >> + free_count = 0;
> >> + list_for_each_entry(e, &setting->marker_entry, sibling) {
> >> + if (e->event_id != free_count) {
> >> + trace_mark(metadata, core_marker_id,
> >> + "channel %s name %s event_id %hu "
> >> + "int #1u%zu long #1u%zu pointer #1u%zu "
> >> + "size_t #1u%zu alignment #1u%u",
> >> + e->channel, e->name, (u16)free_count,
> >> + sizeof(int), sizeof(long), sizeof(void *),
> >> + sizeof(size_t), ltt_get_alignment());
> >> }
> >> - markers_compact_event_ids();
> >> + e->event_id = free_count++;
> >> }
> >> + setting->free_event_id = (u16)free_count;
> >> + marker_update_probes();
> >> }
> >>
> >> -/*
> >> - * Perform channel index compaction when the last trace channel is freed.
> >> - *
> >> - * Called with lock_markers() and channels mutex held.
> >> - */
> >> -static void release_trace_channel(struct kref *kref)
> >> +static void remove_chanel_marker_entries(struct ltt_channel_setting *setting)
> >> {
> >> - struct ltt_channel_setting *iter, *n;
> >> + struct marker_entry *e, *n;
> >> +
> >> + list_for_each_entry_safe(e, n, &setting->marker_entry, sibling) {
> >> + if (e->single.func != __mark_empty_function)
> >> + continue;
> >>
> >> - list_for_each_entry_safe(iter, n, <t_channels, list)
> >> - release_channel_setting(&iter->kref);
> >> + hlist_del(&e->hlist);
> >> + if (e->format_allocated)
> >> + kfree(e->format);
> >> + list_del(&e->sibling);
> >> + /* Make sure the call_rcu has been executed */
> >> + if (e->rcu_pending)
> >> + rcu_barrier_sched();
> >> + kfree(e);
> >> + }
> >> }
> >>
> >> -/**
> >> - * ltt_channels_register: Register a trace channel.
> >> - *
> >> - * Uses refcounting.
> >> - */
> >> -int ltt_channels_register(const char *name)
> >> +int start_trace_channel(const char *channel)
> >> {
> >> + int ret = -ENOENT;
> >> struct ltt_channel_setting *setting;
> >> - int ret = 0;
> >>
> >> - mutex_lock(<t_channel_mutex);
> >> - setting = lookup_channel(name);
> >> + lock_markers();
> >> + setting = lookup_channel(channel);
> >> + if (setting) {
> >> + if (!setting->trace_count)
> >> + defrag_id(setting);
> >> + setting->trace_count++;
> >> + ret = 0;
> >> + }
> >> + unlock_markers();
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(start_trace_channel);
> >> +
> >> +int stop_trace_channel(const char *channel)
> >> +{
> >> + int ret = -ENOENT;
> >> + struct ltt_channel_setting *setting;
> >> +
> >> + lock_markers();
> >> + setting = lookup_channel(channel);
> >> if (setting) {
> >> - if (atomic_read(&setting->kref.refcount) == 0)
> >> - goto init_kref;
> >> - else {
> >> - kref_get(&setting->kref);
> >> - goto end;
> >> + setting->trace_count--;
> >> + if (!setting->trace_count)
> >> + remove_chanel_marker_entries(setting);
> >> + ret = 0;
> >> + if (!setting->live_count && !setting->trace_count) {
> >> + BUG_ON(!list_empty(&setting->marker_entry));
> >> + list_del(&setting->list);
> >> + kfree(setting);
> >> }
> >> }
> >> - setting = kzalloc(sizeof(*setting), GFP_KERNEL);
> >> - if (!setting) {
> >> - ret = -ENOMEM;
> >> - goto end;
> >> + unlock_markers();
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(stop_trace_channel);
> >> +
> >> +/* permanent! the return value is valid before stop_strace_channel() */
> >> +
> >> +const char *get_name_from_eid(const char *channel, u16 eID)
> >> +{
> >> + char *ret = ERR_PTR(-ENOENT);
> >> + struct ltt_channel_setting *setting;
> >> + struct marker_entry *e;
> >> +
> >> + lock_markers();
> >> + setting = lookup_channel(channel);
> >> + list_for_each_entry(e, &setting->marker_entry, sibling) {
> >> + if (e->event_id == eID) {
> >> + ret = e->name;
> >> + break;
> >> + }
> >> }
> >> - list_add(&setting->list, <t_channels);
> >> - strncpy(setting->name, name, PATH_MAX-1);
> >> - setting->index = free_index++;
> >> -init_kref:
> >> - kref_init(&setting->kref);
> >> -end:
> >> - mutex_unlock(<t_channel_mutex);
> >> + unlock_markers();
> >> return ret;
> >> }
> >> -EXPORT_SYMBOL_GPL(ltt_channels_register);
> >> +EXPORT_SYMBOL_GPL(get_name_from_eid);
> >>
> >> -/**
> >> - * ltt_channels_unregister(): Unregister a trace channel.
> >> - *
> >> - * Must be called with markers mutex held.
> >> - */
> >> -int ltt_channels_unregister(const char *name)
> >> +const char *get_fmt_from_eid(const char *channel, u16 eID)
> >> {
> >> + char *ret = ERR_PTR(-ENOENT);
> >> struct ltt_channel_setting *setting;
> >> - int ret = 0;
> >> + struct marker_entry *e;
> >>
> >> - mutex_lock(<t_channel_mutex);
> >> - setting = lookup_channel(name);
> >> - if (!setting || atomic_read(&setting->kref.refcount) == 0) {
> >> - ret = -ENOENT;
> >> - goto end;
> >> + lock_markers();
> >> + setting = lookup_channel(channel);
> >> + list_for_each_entry(e, &setting->marker_entry, sibling) {
> >> + if (e->event_id == eID) {
> >> + if (e->used)
> >> + ret = e->format;
> >> + break;
> >> + }
> >> }
> >> - kref_put(&setting->kref, release_channel_setting);
> >> -end:
> >> - mutex_unlock(<t_channel_mutex);
> >> + unlock_markers();
> >> return ret;
> >> }
> >> -EXPORT_SYMBOL_GPL(ltt_channels_unregister);
> >> +EXPORT_SYMBOL_GPL(get_fmt_from_eid);
> >> +
> >> +/*
> >> + * Must be called when channel refcount falls to 0 _and_ also when the last
> >> + * trace is freed. This function is responsible for compacting the channel
> >> + * index when no users are active.
> >> + */
> >> +static void release_channel_index(struct kref *kref)
> >> +{
> >> + struct ltt_channel_setting *setting;
> >> + free_index = 0;
> >> + list_for_each_entry(setting, <t_channels, list)
> >> + setting->index = free_index++;
> >> +}
> >>
> >> /**
> >> * ltt_channels_set_default: Set channel default behavior.
> >> @@ -149,16 +209,16 @@ int ltt_channels_set_default(const char *name,
> >> struct ltt_channel_setting *setting;
> >> int ret = 0;
> >>
> >> - mutex_lock(<t_channel_mutex);
> >> + lock_markers();
> >> setting = lookup_channel(name);
> >> - if (!setting || atomic_read(&setting->kref.refcount) == 0) {
> >> + if (!setting) {
> >> ret = -ENOENT;
> >> goto end;
> >> }
> >> setting->subbuf_size = subbuf_size;
> >> setting->subbuf_cnt = subbuf_cnt;
> >> end:
> >> - mutex_unlock(<t_channel_mutex);
> >> + unlock_markers();
> >> return ret;
> >> }
> >> EXPORT_SYMBOL_GPL(ltt_channels_set_default);
> >> @@ -174,7 +234,7 @@ const char *ltt_channels_get_name_from_index(unsigned int index)
> >> struct ltt_channel_setting *iter;
> >>
> >> list_for_each_entry(iter, <t_channels, list)
> >> - if (iter->index == index && atomic_read(&iter->kref.refcount))
> >> + if (iter->index == index)
> >> return iter->name;
> >> return NULL;
> >> }
> >> @@ -186,8 +246,7 @@ ltt_channels_get_setting_from_name(const char *name)
> >> struct ltt_channel_setting *iter;
> >>
> >> list_for_each_entry(iter, <t_channels, list)
> >> - if (!strcmp(iter->name, name)
> >> - && atomic_read(&iter->kref.refcount))
> >> + if (!strcmp(iter->name, name))
> >> return iter;
> >> return NULL;
> >> }
> >> @@ -228,7 +287,7 @@ struct ltt_channel_struct *ltt_channels_trace_alloc(unsigned int *nr_channels,
> >> struct ltt_channel_struct *channel = NULL;
> >> struct ltt_channel_setting *iter;
> >>
> >> - mutex_lock(<t_channel_mutex);
> >> + lock_markers();
> >> if (!free_index)
> >> goto end;
> >> if (!atomic_read(&index_kref.refcount))
> >> @@ -241,7 +300,7 @@ struct ltt_channel_struct *ltt_channels_trace_alloc(unsigned int *nr_channels,
> >> if (!channel)
> >> goto end;
> >> list_for_each_entry(iter, <t_channels, list) {
> >> - if (!atomic_read(&iter->kref.refcount))
> >> + if (!iter->live_count)
> >> continue;
> >> channel[iter->index].subbuf_size = iter->subbuf_size;
> >> channel[iter->index].subbuf_cnt = iter->subbuf_cnt;
> >> @@ -250,7 +309,7 @@ struct ltt_channel_struct *ltt_channels_trace_alloc(unsigned int *nr_channels,
> >> channel[iter->index].channel_name = iter->name;
> >> }
> >> end:
> >> - mutex_unlock(<t_channel_mutex);
> >> + unlock_markers();
> >> return channel;
> >> }
> >> EXPORT_SYMBOL_GPL(ltt_channels_trace_alloc);
> >> @@ -264,33 +323,18 @@ EXPORT_SYMBOL_GPL(ltt_channels_trace_alloc);
> >> void ltt_channels_trace_free(struct ltt_channel_struct *channels)
> >> {
> >> lock_markers();
> >> - mutex_lock(<t_channel_mutex);
> >> - kfree(channels);
> >> - kref_put(&index_kref, release_trace_channel);
> >> - mutex_unlock(<t_channel_mutex);
> >> + kref_put(&index_kref, release_channel_index);
> >> unlock_markers();
> >> + kfree(channels);
> >> }
> >> EXPORT_SYMBOL_GPL(ltt_channels_trace_free);
> >>
> >> -/**
> >> - * _ltt_channels_get_event_id: get next event ID for a marker
> >> - * @channel: channel name
> >> - * @name: event name
> >> - *
> >> - * Returns a unique event ID (for this channel) or < 0 on error.
> >> - * Must be called with channels mutex held.
> >> - */
> >> -int _ltt_channels_get_event_id(const char *channel, const char *name)
> >> +static int _ltt_channels_get_event_id(struct ltt_channel_setting *setting,
> >> + const char *name)
> >> {
> >> - struct ltt_channel_setting *setting;
> >> int ret;
> >>
> >> - setting = ltt_channels_get_setting_from_name(channel);
> >> - if (!setting) {
> >> - ret = -ENOENT;
> >> - goto end;
> >> - }
> >> - if (strcmp(channel, "metadata") == 0) {
> >> + if (strcmp(setting->name, "metadata") == 0) {
> >> if (strcmp(name, "core_marker_id") == 0)
> >> ret = 0;
> >> else if (strcmp(name, "core_marker_format") == 0)
> >> @@ -299,7 +343,7 @@ int _ltt_channels_get_event_id(const char *channel, const char *name)
> >> ret = -ENOENT;
> >> goto end;
> >> }
> >> - if (setting->free_event_id == EVENTS_PER_CHANNEL) {
> >> + if (setting->free_event_id == EVENTS_PER_CHANNEL - 1) {
> >> ret = -ENOSPC;
> >> goto end;
> >> }
> >> @@ -316,11 +360,14 @@ end:
> >> */
> >> int ltt_channels_get_event_id(const char *channel, const char *name)
> >> {
> >> - int ret;
> >> + int ret = -ENOENT;
> >> + struct ltt_channel_setting *setting;
> >>
> >> - mutex_lock(<t_channel_mutex);
> >> - ret = _ltt_channels_get_event_id(channel, name);
> >> - mutex_unlock(<t_channel_mutex);
> >> + lock_markers();
> >> + setting = ltt_channels_get_setting_from_name(channel);
> >> + if (setting)
> >> + ret = _ltt_channels_get_event_id(setting, name);
> >> + unlock_markers();
> >> return ret;
> >> }
> >>
> >> diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
> >> index af14135..71289f9 100644
> >> --- a/ltt/ltt-tracer.c
> >> +++ b/ltt/ltt-tracer.c
> >> @@ -772,6 +772,7 @@ int ltt_trace_alloc(const char *trace_name)
> >> subbuf_size = trace->channels[chan].subbuf_size;
> >> subbuf_cnt = trace->channels[chan].subbuf_cnt;
> >> prepare_chan_size_num(&subbuf_size, &subbuf_cnt);
> >> + start_trace_channel(channel_name);
> >> err = trace->ops->create_channel(trace_name, trace,
> >> trace->dentry.trace_root,
> >> channel_name,
> >> @@ -801,8 +802,10 @@ int ltt_trace_alloc(const char *trace_name)
> >>
> >> create_channel_error:
> >> for (chan--; chan >= 0; chan--)
> >> - if (trace->channels[chan].active)
> >> + if (trace->channels[chan].active) {
> >> trace->ops->remove_channel(&trace->channels[chan]);
> >> + stop_trace_channel(trace->channels[chan].channel_name);
> >> + }
> >>
> >> dirs_error:
> >> module_put(trace->transport->owner);
> >> @@ -901,8 +904,10 @@ static void __ltt_trace_destroy(struct ltt_trace_struct *trace)
> >>
> >> for (i = 0; i < trace->nr_channels; i++) {
> >> chan = &trace->channels[i];
> >> - if (chan->active)
> >> + if (chan->active) {
> >> trace->ops->remove_channel(chan);
> >> + stop_trace_channel(chan->channel_name);
> >> + }
> >> }
> >>
> >> kref_put(&trace->ltt_transport_kref, ltt_release_transport);
> >>
> >>
> >>
> >> _______________________________________________
> >> ltt-dev mailing list
> >> ltt-dev at lists.casi.polymtl.ca
> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >>
> >
>
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
More information about the lttng-dev
mailing list