[ltt-dev] [PATCH] ltt-channel: permanent eID

Mathieu Desnoyers compudj at krystal.dyndns.org
Wed Dec 17 14:40:17 EST 2008


* 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, &ltt_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, &ltt_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, &ltt_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(&ltt_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, &ltt_channels);
> -	strncpy(setting->name, name, PATH_MAX-1);
> -	setting->index = free_index++;
> -init_kref:
> -	kref_init(&setting->kref);
> -end:
> -	mutex_unlock(&ltt_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(&ltt_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(&ltt_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, &ltt_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(&ltt_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(&ltt_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, &ltt_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, &ltt_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(&ltt_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, &ltt_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(&ltt_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(&ltt_channel_mutex);
> -	kfree(channels);
> -	kref_put(&index_kref, release_trace_channel);
> -	mutex_unlock(&ltt_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(&ltt_channel_mutex);
> -	ret = _ltt_channels_get_event_id(channel, name);
> -	mutex_unlock(&ltt_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