[ltt-dev] [PATCH v2 1/3] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc()

Mathieu Desnoyers compudj at krystal.dyndns.org
Tue Nov 11 01:27:02 EST 2008


* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> This is step1 of:
> "switch marker list and marker activation (currently in /proc/ltt) to debugfs"
> 
> > >> Are you means we can control each channel's (interrupt, process, ...)
> > >> subbuf in future version?
> > >> (I think current version can only control low, med and high)
> > >> If this is true, maybe we need modify ltt_trace_create() before impl
> > >> ltt_control.
> > >>
> > > Yes, this is the goal.
> > So, IMHO, we can do it in following steps:
> >
> > Step1: modify ltt_trace_create(), .etc, separate ltt_trace_create() into
> >   ltt_trace_setup(), ltt_set_channelsize(), ltt_set_channelnum(),
> >   ltt_trace_alloc(). But still use old netlink_based ltt_control to make
> >   it simple.
> >   Although we modify ltt_set_channelsize() to support setting each channel's
> >   buffer, we make ltt_control.ko using old UI(only able to set low, med and
> >   high). So we don't need to modify other program such as user-mode lttctl.
> >
> 
> Yes. Just make sure a "destroy" deals correctly with a trace that is
> "setup" but not "allocated" yet.
> 
> > Step2: Write a new module named ltt-channel-control.
> >   ( I think name of ltt-channel-control will be better than ltt-control,
> >     because we already have a module named ltt-marker-control. )
> >   ltt-channel-control is designed on debugfs, as we discussed.
> >   We can test it by echo XX > /mnt/debugfs/ltt/...
> >
> 
> Yes, that would be perfect.
> 
> > Step3: modify user-mode programs to support ltt-channel-control.
> >   We also need to modify lttctl's command arguments, to support setup each
> >   channel's buffer. (Are we take out -Z, -X, -V, -B, -z, -x in lttctl?)
> >   I haven't read source of lttv, it is necessary to modify?
> >
> 
> Maybe we could replace lttctl by simple shell scripts ?
> 
> Yes, small modification of lttv would be required. There is a GUI module
> which executes lttctl to start/stop tracing. It would have to be
> modified so it uses the debugfs files instead.
> 
> Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>

Hi Zhao Lei,

Nice work, somme comments below,

> ---
>  include/linux/ltt-core.h   |    3 +-
>  include/linux/ltt-tracer.h |   52 +++--
>  ltt/ltt-core.c             |    1 +
>  ltt/ltt-tracer.c           |  590 ++++++++++++++++++++++++++++++++------------
>  4 files changed, 475 insertions(+), 171 deletions(-)
>  mode change 100644 => 100755 ltt/ltt-tracer.c
> 
> diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
> index 1681e82..7e5ee8b 100644
> --- a/include/linux/ltt-core.h
> +++ b/include/linux/ltt-core.h
> @@ -17,7 +17,8 @@
>   * list.
>   */
>  struct ltt_traces {
> -	struct list_head head;		/* Traces list */
> +	struct list_head setup_head;	/* Pre-allocated traces list */
> +	struct list_head head;		/* Allocated Traces list */
>  	unsigned int num_active_traces;	/* Number of active traces */
>  } ____cacheline_aligned;
>  
> diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
> index 277d42f..c4ac07c 100644
> --- a/include/linux/ltt-tracer.h
> +++ b/include/linux/ltt-tracer.h
> @@ -197,6 +197,23 @@ struct ltt_transport {
>  	struct ltt_trace_ops ops;
>  };
>  
> +/*
> + * First and last channels in ltt_trace_struct.
> + */
> +#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
> +#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
> +#define ltt_channel_index_end()	\
> +	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
> +
> +enum ltt_channels {
> +	LTT_CHANNEL_CPU,
> +	LTT_CHANNEL_PROCESSES,
> +	LTT_CHANNEL_INTERRUPTS,
> +	LTT_CHANNEL_NETWORK,
> +	LTT_CHANNEL_MODULES,
> +	LTT_CHANNEL_METADATA,
> +	NR_LTT_CHANNELS,
> +};
>  
>  enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID };
>  
> @@ -218,6 +235,12 @@ struct ltt_trace_struct {
>  		struct ltt_channel_struct	*modules;
>  		struct ltt_channel_struct	*metadata;
>  	} channel;
> +	struct {
> +		struct {
> +			unsigned subbuf_size;
> +			unsigned subbuf_cnt;
> +		} channels[NR_LTT_CHANNELS];
> +	} setting;
>  	u32 freq_scale;
>  	u64 start_freq;
>  	u64 start_tsc;
> @@ -236,23 +259,6 @@ struct ltt_trace_struct {
>  	char trace_name[NAME_MAX];
>  } ____cacheline_aligned;
>  
> -/*
> - * First and last channels in ltt_trace_struct.
> - */
> -#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
> -#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
> -#define ltt_channel_index_end()	\
> -	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
> -
> -enum ltt_channels {
> -	LTT_CHANNEL_CPU,
> -	LTT_CHANNEL_PROCESSES,
> -	LTT_CHANNEL_INTERRUPTS,
> -	LTT_CHANNEL_NETWORK,
> -	LTT_CHANNEL_MODULES,
> -	LTT_CHANNEL_METADATA,
> -};
> -
>  /* Hardcoded event headers
>   *
>   * event header for a trace with active heartbeat : 27 bits timestamps
> @@ -690,6 +696,18 @@ union ltt_control_args {
>  	} new_trace;
>  };
>  
> +int ltt_trace_setup(const char *trace_name);
> +int ltt_trace_set_type(const char *trace_name, const char *trace_type);
> +int ltt_trace_set_mode(const char *trace_name, enum trace_mode mode);
> +int ltt_trace_set_channel_subbufsize(const char *trace_name,
> +		const char *channel_name, unsigned size);
> +int ltt_trace_set_channel_subbufcount(const char *trace_name,
> +		const char *channel_name, unsigned cnt);
> +int ltt_trace_alloc(const char *trace_name);
> +int ltt_trace_destroy(const char *trace_name);
> +int ltt_trace_start(const char *trace_name);
> +int ltt_trace_stop(const char *trace_name);
> +
>  extern int ltt_control(enum ltt_control_msg msg, const char *trace_name,
>  		const char *trace_type, union ltt_control_args args);
>  
> diff --git a/ltt/ltt-core.c b/ltt/ltt-core.c
> index abd3d6d..aaad358 100644
> --- a/ltt/ltt-core.c
> +++ b/ltt/ltt-core.c
> @@ -12,6 +12,7 @@
>  
>  /* Traces structures */
>  struct ltt_traces ltt_traces = {
> +	.setup_head = LIST_HEAD_INIT(ltt_traces.setup_head),
>  	.head = LIST_HEAD_INIT(ltt_traces.head),
>  };
>  EXPORT_SYMBOL(ltt_traces);
> diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
> old mode 100644
> new mode 100755
> index e86eb6d..e1aa838
> --- a/ltt/ltt-tracer.c
> +++ b/ltt/ltt-tracer.c
> @@ -268,31 +268,20 @@ static struct ltt_trace_struct *_ltt_trace_find(const char *trace_name)
>  	return NULL;
>  }
>  
> -/* This function must be called with traces semaphore held. */
> -static int _ltt_trace_create(const char *trace_name, enum trace_mode mode,
> -				struct ltt_trace_struct *new_trace)
> +/* _ltt_trace_find_setup :
> + * find a trace in setup list by given name.
> + *
> + * Returns a pointer to the trace structure, NULL if not found.
> + */
> +static struct ltt_trace_struct *_ltt_trace_find_setup(const char *trace_name)
>  {
> -	int err = EPERM;
> +	struct ltt_trace_struct *trace;
>  
> -	if (_ltt_trace_find(trace_name) != NULL) {
> -		printk(KERN_ERR "LTT : Trace %s already exists\n", trace_name);
> -		err = EEXIST;
> -		goto traces_error;
> -	}
> -	if (list_empty(&ltt_traces.head)) {
> -		probe_id_defrag();
> -		mod_timer(&ltt_async_wakeup_timer,
> -				jiffies + LTT_PERCPU_TIMER_INTERVAL);
> -		set_kernel_trace_flag_all_tasks();
> -	}
> -	list_add_rcu(&new_trace->list, &ltt_traces.head);
> -	synchronize_sched();
> -	/* Everything went fine, finish creation */
> -	return 0;
> +	list_for_each_entry(trace, &ltt_traces.setup_head, list)
> +		if (!strncmp(trace->trace_name, trace_name, NAME_MAX))
> +			return trace;
>  
> -	/* Error handling */
> -traces_error:
> -	return err;
> +	return NULL;
>  }
>  
>  /**
> @@ -334,172 +323,443 @@ static inline void prepare_chan_size_num(unsigned *subbuf_size,
>  	WARN_ON(hweight32(*n_subbufs) != 1);
>  }
>  
> -static int ltt_trace_create(const char *trace_name, const char *trace_type,
> -		enum trace_mode mode,
> -		unsigned subbuf_size_low, unsigned n_subbufs_low,
> -		unsigned subbuf_size_med, unsigned n_subbufs_med,
> -		unsigned subbuf_size_high, unsigned n_subbufs_high)
> +int ltt_trace_setup(const char *trace_name)
>  {
>  	int err = 0;
> -	struct ltt_trace_struct *new_trace, *trace;
> -	unsigned long flags;
> -	struct ltt_transport *tran, *transport = NULL;
> +	struct ltt_trace_struct *new_trace = NULL;
>  
> -	prepare_chan_size_num(&subbuf_size_low, &n_subbufs_low,
> -		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	ltt_lock_traces();
>  
> -	prepare_chan_size_num(&subbuf_size_med, &n_subbufs_med,
> -		LTT_DEFAULT_SUBBUF_SIZE_MED, LTT_DEFAULT_N_SUBBUFS_MED);
> +	if (_ltt_trace_find_setup(trace_name)) {
> +		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> +				trace_name);
> +		err = -EEXIST;
> +		goto traces_error;
> +	}
>  
> -	prepare_chan_size_num(&subbuf_size_high, &n_subbufs_high,
> -		LTT_DEFAULT_SUBBUF_SIZE_HIGH, LTT_DEFAULT_N_SUBBUFS_HIGH);
> +	if (_ltt_trace_find(trace_name)) {
> +		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> +				trace_name);
> +		err = -EEXIST;
> +		goto traces_error;
> +	}
>  
>  	new_trace = kzalloc(sizeof(struct ltt_trace_struct), GFP_KERNEL);
>  	if (!new_trace) {
>  		printk(KERN_ERR
>  			"LTT : Unable to allocate memory for trace %s\n",
>  			trace_name);
> -		err = ENOMEM;
> -		goto traces_error;
> +		err = -ENOMEM;
> +		goto trace_free;
>  	}
>  
> -	kref_init(&new_trace->kref);
> -	kref_init(&new_trace->ltt_transport_kref);
> -	init_waitqueue_head(&new_trace->kref_wq);
> -	new_trace->active = 0;
>  	strncpy(new_trace->trace_name, trace_name, NAME_MAX);
> -	new_trace->mode = mode;
> -	get_trace_clock();
> -	new_trace->freq_scale = trace_clock_freq_scale();
> +	list_add(&new_trace->list, &ltt_traces.setup_head);
> +
> +	ltt_unlock_traces();
> +
> +	return 0;
> +
> +trace_free:
> +	kfree(new_trace);
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_setup);
> +
> +/* must be called from within a traces lock. */
> +static void _ltt_trace_free(struct ltt_trace_struct *trace)
> +{
> +	list_del(&trace->list);
> +	kfree(trace);
> +}
> +
> +int ltt_trace_set_type(const char *trace_name, const char *trace_type)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	struct ltt_transport *tran_iter, *transport = NULL;
>  
>  	ltt_lock_traces();
> -	list_for_each_entry(tran, &ltt_transport_list, node) {
> -		if (!strcmp(tran->name, trace_type)) {
> -			transport = tran;
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	list_for_each_entry(tran_iter, &ltt_transport_list, node) {
> +		if (!strcmp(tran_iter->name, trace_type)) {
> +			transport = tran_iter;
>  			break;
>  		}
>  	}
> -
>  	if (!transport) {
> -		err = EINVAL;
>  		printk(KERN_ERR	"LTT : Transport %s is not present.\n",
>  			trace_type);
> -		ltt_unlock_traces();
> -		goto trace_error;
> +		err = -EINVAL;
> +		goto traces_error;
>  	}
>  
> -	if (!try_module_get(transport->owner)) {
> -		err = ENODEV;
> -		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
> -		ltt_unlock_traces();
> -		goto trace_error;
> +	trace->transport = transport;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_set_type);
> +
> +int ltt_trace_set_mode(const char *trace_name, enum trace_mode mode)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
>  	}
>  
> -	trace = _ltt_trace_find(trace_name);
> -	if (trace) {
> -		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> -			trace_name);
> -		err = EEXIST;
> -		goto trace_error;
> +	trace->mode = mode;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_set_mode);
> +
> +/*
> + * Todo:
> + * Make similar function in channel.c,
> + * so it will be useful for both ltt-tracer.c and ltt-marker-control.c
> + */
> +/*
> + * Its order is MUST be same with enum ltt_channels
> + */
> +struct chan_info_struct {
> +	const char *name;
> +	unsigned int channel_index;

You can probably remove the channel_index, because it is known through
offsetof within the array.

> +	unsigned int def_subbufsize;
> +	unsigned int def_subbufcount;
> +} chan_infos[] = {

A declaration like this seems to work :


int blah[] = {
        [0] = 1,
        [1] = 2,
        [5] = 3,
        [4] = 4,
};

So I think you can populate the array by using :

struct chan_info_struct {
	const char *name;
	unsigned int channel_index;
	unsigned int def_subbufsize;
	unsigned int def_subbufcount;
} chan_infos[] = {
  [GET_CHANNEL_INDEX(cpu)] = {
    LTT_CPU_CHANNEL,
    LTT_DEFAULT_SUBBUF_SIZE_HIGH,
    LTT_DEFAULT_N_SUBBUFS_HIGH,
  },
  .........
};

So you don't have to force the correct declaration order.

> +	{
> +		LTT_CPU_CHANNEL,
> +		GET_CHANNEL_INDEX(cpu),
> +		LTT_DEFAULT_SUBBUF_SIZE_HIGH,
> +		LTT_DEFAULT_N_SUBBUFS_HIGH,
> +	},
> +	{
> +		LTT_PROCESSES_CHANNEL,
> +		GET_CHANNEL_INDEX(processes),
> +		LTT_DEFAULT_SUBBUF_SIZE_MED,
> +		LTT_DEFAULT_N_SUBBUFS_MED,
> +	},
> +	{
> +		LTT_INTERRUPTS_CHANNEL,
> +		GET_CHANNEL_INDEX(interrupts),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	{
> +		LTT_NETWORK_CHANNEL,
> +		GET_CHANNEL_INDEX(network),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	{
> +		LTT_MODULES_CHANNEL,
> +		GET_CHANNEL_INDEX(modules),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	{
> +		LTT_METADATA_CHANNEL,
> +		GET_CHANNEL_INDEX(metadata),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +};
> +
> +static enum ltt_channels get_channel_type_from_name(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(chan_infos); i++)
> +		if (!strcmp(name, chan_infos[i].name))
> +			return (enum ltt_channels)i;
> +
> +	return NR_LTT_CHANNELS;
> +}
> +
> +int ltt_trace_set_channel_subbufsize(const char *trace_name,
> +		const char *channel_name, unsigned size)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	enum ltt_channels channel;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
>  	}
>  
> -	new_trace->transport = transport;
> -	new_trace->ops = &transport->ops;
> +	channel = get_channel_type_from_name(channel_name);
> +	if (channel == NR_LTT_CHANNELS) {
> +		printk(KERN_ERR "LTT : Channel %s is not present.\n",
> +			channel_name);
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
>  
> -	err = new_trace->ops->create_dirs(new_trace);
> -	if (err)
> +	trace->setting.channels[channel].subbuf_size = size;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_set_channel_subbufsize);
> +
> +int ltt_trace_set_channel_subbufcount(const char *trace_name,
> +		const char *channel_name, unsigned cnt)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	enum ltt_channels channel;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	channel = get_channel_type_from_name(channel_name);
> +	if (channel == NR_LTT_CHANNELS) {
> +		printk(KERN_ERR "LTT : Channel %s is not present.\n",
> +			channel_name);
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
> +
> +	trace->setting.channels[channel].subbuf_cnt = cnt;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_set_channel_subbufcount);
> +
> +int ltt_trace_alloc(const char *trace_name)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	int subbuf_size, subbuf_cnt;
> +	unsigned long flags;
> +	int chan;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	kref_init(&trace->kref);
> +	kref_init(&trace->ltt_transport_kref);
> +	init_waitqueue_head(&trace->kref_wq);
> +	trace->active = 0;
> +	get_trace_clock();
> +	trace->freq_scale = trace_clock_freq_scale();
> +
> +	if (!trace->transport) {
> +		printk(KERN_ERR "LTT : Transport is not set.\n");
> +		err = -EINVAL;
> +		goto transport_error;
> +	}
> +	if (!try_module_get(trace->transport->owner)) {
> +		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
> +		err = -ENODEV;
> +		goto transport_error;
> +	}
> +	trace->ops = &trace->transport->ops;
> +
> +	err = trace->ops->create_dirs(trace);
> +	if (err) {
> +		printk(KERN_ERR	"LTT : Can't create dir for trace %s.\n",
> +			trace_name);
>  		goto dirs_error;
> +	}
>  
>  	local_irq_save(flags);
> -	new_trace->start_freq = trace_clock_frequency();
> -	new_trace->start_tsc = trace_clock_read64();
> -	do_gettimeofday(&new_trace->start_time);
> +	trace->start_freq = trace_clock_frequency();
> +	trace->start_tsc = trace_clock_read64();
> +	do_gettimeofday(&trace->start_time);
>  	local_irq_restore(flags);
>  
>  	/*
> +	 * Todo:
> +	 *   Config each channel's default buffersize/cnt, instead of
> +	 *   LTT_DEFAULT_SUBBUF_SIZE_LOW, ...
> +	 */
> +
> +	/*
>  	 * Always put the metadata channel in non-overwrite mode :
>  	 * This is a very low traffic channel and it can't afford to have its
>  	 * data overwritten : this data (marker info) is necessary to be
>  	 * able to read the trace.
>  	 */
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> -			LTT_METADATA_CHANNEL,
> -			&new_trace->channel.metadata, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_METADATA, mode));
> -	if (err != 0)
> -		goto metadata_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> -			LTT_INTERRUPTS_CHANNEL,
> -			&new_trace->channel.interrupts, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_INTERRUPTS, mode));
> -	if (err != 0)
> -		goto interrupts_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> -			LTT_PROCESSES_CHANNEL,
> -			&new_trace->channel.processes, subbuf_size_med,
> -			n_subbufs_med,
> -			is_channel_overwrite(LTT_CHANNEL_PROCESSES, mode));
> -	if (err != 0)
> -		goto processes_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> -			LTT_MODULES_CHANNEL,
> -			&new_trace->channel.modules, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_MODULES, mode));
> -	if (err != 0)
> -		goto modules_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> -			LTT_NETWORK_CHANNEL,
> -			&new_trace->channel.network, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_NETWORK, mode));
> -	if (err != 0)
> -		goto network_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.trace_root,
> -			LTT_CPU_CHANNEL,
> -			&new_trace->channel.cpu, subbuf_size_high,
> -			n_subbufs_high,
> -			is_channel_overwrite(LTT_CHANNEL_CPU, mode));
> -	if (err != 0)
> -		goto cpu_error;
> -
> -	err = _ltt_trace_create(trace_name, mode, new_trace);
> -
> -	if (err != 0)
> -		goto lock_create_error;
> +	for (chan = 0; chan < NR_LTT_CHANNELS; chan++) {
> +		subbuf_size = trace->setting.channels[chan].subbuf_size;
> +		subbuf_cnt = trace->setting.channels[chan].subbuf_cnt;
> +		prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +			chan_infos[chan].def_subbufsize,
> +			chan_infos[chan].def_subbufcount);
> +		err = trace->ops->create_channel(trace_name, trace,
> +				trace->dentry.control_root,
> +				chan_infos[chan].name,
> +				(struct ltt_channel_struct **)((char *)trace
> +				+ chan_infos[chan].channel_index),
> +				subbuf_size,
> +				subbuf_cnt,
> +				is_channel_overwrite(chan, trace->mode));
> +		if (err != 0) {
> +			printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +				chan_infos[chan].name);
> +			goto create_channel_error;
> +		}
> +	}
> +
> +	list_del(&trace->list);
> +	if (list_empty(&ltt_traces.head)) {
> +		probe_id_defrag();
> +		mod_timer(&ltt_async_wakeup_timer,
> +				jiffies + LTT_PERCPU_TIMER_INTERVAL);
> +		set_kernel_trace_flag_all_tasks();
> +	}
> +	list_add_rcu(&trace->list, &ltt_traces.head);
> +	synchronize_sched();
> +
>  	ltt_unlock_traces();
> -	return err;
>  
> -lock_create_error:
> -	new_trace->ops->remove_channel(new_trace->channel.cpu);
> -cpu_error:
> -	new_trace->ops->remove_channel(new_trace->channel.network);
> -network_error:
> -	new_trace->ops->remove_channel(new_trace->channel.modules);
> -modules_error:
> -	new_trace->ops->remove_channel(new_trace->channel.processes);
> -processes_error:
> -	new_trace->ops->remove_channel(new_trace->channel.interrupts);
> -interrupts_error:
> -	new_trace->ops->remove_channel(new_trace->channel.metadata);
> -metadata_error:
> -	kref_put(&new_trace->ltt_transport_kref, ltt_release_transport);
> +	return 0;
> +
> +create_channel_error:
> +	for (chan--; chan >= 0; chan--)
> +		trace->ops->remove_channel(*(struct ltt_channel_struct **)
> +			((char *)trace + chan_infos[chan].channel_index));

That looks a bit weird. Is there a neater way to do this without that
many casts ?

Thanks,

Mathieu

> +
>  dirs_error:
> -	module_put(transport->owner);
> -trace_error:
> -	kref_put(&new_trace->kref, ltt_release_trace);
> -	wake_up_interruptible(&new_trace->kref_wq);
> -	ltt_unlock_traces();
> +	module_put(trace->transport->owner);
> +transport_error:
>  	put_trace_clock();
>  traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_alloc);
> +
> +/*
> + * It is worked as a wrapper for current version of ltt_control.ko.
> + * We will make a new ltt_control based on debugfs, and control each channel's
> + * buffer.
> + */
> +static int ltt_trace_create(const char *trace_name, const char *trace_type,
> +		enum trace_mode mode,
> +		unsigned subbuf_size_low, unsigned n_subbufs_low,
> +		unsigned subbuf_size_med, unsigned n_subbufs_med,
> +		unsigned subbuf_size_high, unsigned n_subbufs_high)
> +{
> +	int err = 0;
> +
> +	err = ltt_trace_setup(trace_name);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_type(trace_name, trace_type);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_mode(trace_name, mode);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_METADATA_CHANNEL, subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_METADATA_CHANNEL, n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_INTERRUPTS_CHANNEL, subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_INTERRUPTS_CHANNEL, n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_PROCESSES_CHANNEL, subbuf_size_med);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_PROCESSES_CHANNEL, n_subbufs_med);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_MODULES_CHANNEL,
> +		subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_MODULES_CHANNEL,
> +		n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_NETWORK_CHANNEL,
> +		subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_NETWORK_CHANNEL,
> +		n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_CPU_CHANNEL,
> +		subbuf_size_high);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_CPU_CHANNEL,
> +		n_subbufs_high);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_alloc(trace_name);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	return err;
>  }
>  
> @@ -581,26 +841,42 @@ static void __ltt_trace_destroy(struct ltt_trace_struct	*trace)
>  	kref_put(&trace->kref, ltt_release_trace);
>  }
>  
> -static int ltt_trace_destroy(const char *trace_name)
> +int ltt_trace_destroy(const char *trace_name)
>  {
>  	int err = 0;
>  	struct ltt_trace_struct *trace;
>  
>  	ltt_lock_traces();
> +
>  	trace = _ltt_trace_find(trace_name);
> -	err = _ltt_trace_destroy(trace);
> -	if (err)
> -		goto error;
> -	ltt_unlock_traces();
> -	__ltt_trace_destroy(trace);
> -	put_trace_clock();
> -	return err;
> +	if (trace) {
> +		err = _ltt_trace_destroy(trace);
> +		if (err)
> +			goto error;
> +
> +		ltt_unlock_traces();
> +
> +		__ltt_trace_destroy(trace);
> +		put_trace_clock();
> +
> +		return 0;
> +	}
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (trace) {
> +		_ltt_trace_free(trace);
> +		ltt_unlock_traces();
> +		return 0;
> +	}
> +
> +	err = ENOENT;
>  
>  	/* Error handling */
>  error:
>  	ltt_unlock_traces();
>  	return err;
>  }
> +EXPORT_SYMBOL_GPL(ltt_trace_destroy);
>  
>  /* must be called from within a traces lock. */
>  static int _ltt_trace_start(struct ltt_trace_struct *trace)
> @@ -630,7 +906,7 @@ traces_error:
>  	return err;
>  }
>  
> -static int ltt_trace_start(const char *trace_name)
> +int ltt_trace_start(const char *trace_name)
>  {
>  	int err = 0;
>  	struct ltt_trace_struct *trace;
> @@ -670,6 +946,7 @@ no_trace:
>  	ltt_unlock_traces();
>  	return err;
>  }
> +EXPORT_SYMBOL_GPL(ltt_trace_start);
>  
>  /* must be called from within traces lock */
>  static int _ltt_trace_stop(struct ltt_trace_struct *trace)
> @@ -697,7 +974,7 @@ traces_error:
>  	return err;
>  }
>  
> -static int ltt_trace_stop(const char *trace_name)
> +int ltt_trace_stop(const char *trace_name)
>  {
>  	int err = 0;
>  	struct ltt_trace_struct *trace;
> @@ -708,6 +985,7 @@ static int ltt_trace_stop(const char *trace_name)
>  	ltt_unlock_traces();
>  	return err;
>  }
> +EXPORT_SYMBOL_GPL(ltt_trace_stop);
>  
>  /**
>   * ltt_control : Trace control in-kernel API
> @@ -820,6 +1098,12 @@ static void __exit ltt_exit(void)
>  		_ltt_trace_destroy(trace);
>  		__ltt_trace_destroy(trace);
>  	}
> +	/* free traces in pre-alloc status */
> +	list_for_each_safe(pos, n, &ltt_traces.setup_head) {
> +		trace = container_of(pos, struct ltt_trace_struct, list);
> +		_ltt_trace_free(trace);
> +	}
> +
>  	ltt_unlock_traces();
>  }
>  
> -- 
> 1.5.5.3
> 
> 
> 
> 
> _______________________________________________
> 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