From mboxrd@z Thu Jan  1 00:00:00 1970
From: mathieu.desnoyers@polymtl.ca (Mathieu Desnoyers)
Date: Tue, 2 Dec 2008 12:06:48 -0500
Subject: [ltt-dev] [PATCH 2/2] Make ltt support enable/disable each
	channel
In-Reply-To: <49351A77.5070808@cn.fujitsu.com>
References: <49351A0F.5060500@cn.fujitsu.com> <49351A77.5070808@cn.fujitsu.com>
Message-ID: <20081202170647.GD19095@Krystal>

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> It can reduce system loads when user only need to trace some
> channels.
> 
> Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>
> ---
>  include/linux/ltt-tracer.h       |    2 +
>  ltt/ltt-serialize.c              |    4 +
>  ltt/ltt-tracer.c                 |  268 +++++++++++++++++++++++++-------------
>  ltt/probes/ltt-type-serializer.c |    4 +
>  4 files changed, 190 insertions(+), 88 deletions(-)
> 
> diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
> index a18edd1..87696a9 100644
> --- a/include/linux/ltt-tracer.h
> +++ b/include/linux/ltt-tracer.h
> @@ -711,6 +711,8 @@ 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_set_channel_enable(const char *trace_name,
> +		const char *channel_name, unsigned enable);
>  int ltt_trace_set_channel_overwrite(const char *trace_name,
>  		const char *channel_name, unsigned overwrite);
>  int ltt_trace_alloc(const char *trace_name);
> diff --git a/ltt/ltt-serialize.c b/ltt/ltt-serialize.c
> index 3b31a30..9d7ce59 100644
> --- a/ltt/ltt-serialize.c
> +++ b/ltt/ltt-serialize.c
> @@ -633,6 +633,10 @@ notrace void ltt_vtrace(void *probe_data, void *call_data,
>  			rflags = 0;
>  #endif
>  		channel = ltt_get_channel_from_index(trace, channel_index);
> +		if (!channel)
> +			/* channel is NULL if it is not enabled */
> +			continue;
> +
>  		/* reserve space : header and data */
>  		ret = ltt_reserve_slot(trace, channel, &transport_data,
>  					data_size, &slot_size, &buf_offset,
> diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
> index 341074d..a14fd5f 100644
> --- a/ltt/ltt-tracer.c
> +++ b/ltt/ltt-tracer.c
> @@ -67,6 +67,69 @@ int (*ltt_statedump_functor)(struct ltt_trace_struct *trace) =
>  					ltt_statedump_default;
>  struct module *ltt_statedump_owner;
>  
> +/*
> + * 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;
> +	unsigned int def_subbufsize;
> +	unsigned int def_subbufcount;
> +} chan_infos[] = {
> +	[LTT_CHANNEL_CPU] = {
> +		LTT_CPU_CHANNEL,
> +		GET_CHANNEL_INDEX(cpu),
> +		LTT_DEFAULT_SUBBUF_SIZE_HIGH,
> +		LTT_DEFAULT_N_SUBBUFS_HIGH,
> +	},
> +	[LTT_CHANNEL_PROCESSES] = {
> +		LTT_PROCESSES_CHANNEL,
> +		GET_CHANNEL_INDEX(processes),
> +		LTT_DEFAULT_SUBBUF_SIZE_MED,
> +		LTT_DEFAULT_N_SUBBUFS_MED,
> +	},
> +	[LTT_CHANNEL_INTERRUPTS] = {
> +		LTT_INTERRUPTS_CHANNEL,
> +		GET_CHANNEL_INDEX(interrupts),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	[LTT_CHANNEL_NETWORK] = {
> +		LTT_NETWORK_CHANNEL,
> +		GET_CHANNEL_INDEX(network),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	[LTT_CHANNEL_MODULES] = {
> +		LTT_MODULES_CHANNEL,
> +		GET_CHANNEL_INDEX(modules),
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> +		LTT_DEFAULT_N_SUBBUFS_LOW,
> +	},
> +	[LTT_CHANNEL_METADATA] = {
> +		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;
> +}
> +
>  /**
>   * ltt_module_register : LTT module registration
>   * @name : module type
> @@ -216,13 +279,17 @@ EXPORT_SYMBOL_GPL(ltt_write_trace_header);
>  
>  static void trace_async_wakeup(struct ltt_trace_struct *trace)
>  {
> +	int i;
> +	struct ltt_channel_struct *chan;
> +
>  	/* Must check each channel for pending read wakeup */
> -	trace->ops->wakeup_channel(trace->channel.metadata);
> -	trace->ops->wakeup_channel(trace->channel.interrupts);
> -	trace->ops->wakeup_channel(trace->channel.processes);
> -	trace->ops->wakeup_channel(trace->channel.modules);
> -	trace->ops->wakeup_channel(trace->channel.network);
> -	trace->ops->wakeup_channel(trace->channel.cpu);
> +	for (i = 0; i < NR_LTT_CHANNELS; i++) {
> +		chan = *(struct ltt_channel_struct **)((char *)trace
> +			+ chan_infos[i].channel_index);

Hrm, this is the third place I see we have these ugly casts (it is also
in ltt_trace_alloc). Can we create a wrapper for it ? e.g. in
ltt-tracer.c :

static struct ltt_channel_struct **
ltt_get_trace_channel(struct ltt_trace_struct *trace, unsigned int idx)
{
  return (struct ltt_channel_struct **)
    ((char *)trace + chan_infos[idx].channel_index);
}
 


> +		/* chan is NULL if it is not traced */
> +		if (chan)
> +			trace->ops->wakeup_channel(chan);
> +	}
>  }
>  
>  /* Timer to send async wakeups to the readers */
> @@ -355,6 +422,14 @@ int ltt_trace_setup(const char *trace_name)
>  	}
>  
>  	strncpy(new_trace->trace_name, trace_name, NAME_MAX);
> +
> +	/*
> +	 * Datas in metadata channel(marker info) is necessary to be able to
> +	 * read the trace, it is enabled initially, and can't be disabled later.
> +	 */
> +	new_trace->setting.channels[LTT_CHANNEL_METADATA].flags |=
> +		CHANNEL_FLAG_ENABLE;
> +
>  	list_add(&new_trace->list, &ltt_traces.setup_head);
>  
>  	ltt_unlock_traces();
> @@ -412,69 +487,6 @@ traces_error:
>  }
>  EXPORT_SYMBOL_GPL(ltt_trace_set_type);
>  
> -/*
> - * 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;
> -	unsigned int def_subbufsize;
> -	unsigned int def_subbufcount;
> -} chan_infos[] = {
> -	[LTT_CHANNEL_CPU] = {
> -		LTT_CPU_CHANNEL,
> -		GET_CHANNEL_INDEX(cpu),
> -		LTT_DEFAULT_SUBBUF_SIZE_HIGH,
> -		LTT_DEFAULT_N_SUBBUFS_HIGH,
> -	},
> -	[LTT_CHANNEL_PROCESSES] = {
> -		LTT_PROCESSES_CHANNEL,
> -		GET_CHANNEL_INDEX(processes),
> -		LTT_DEFAULT_SUBBUF_SIZE_MED,
> -		LTT_DEFAULT_N_SUBBUFS_MED,
> -	},
> -	[LTT_CHANNEL_INTERRUPTS] = {
> -		LTT_INTERRUPTS_CHANNEL,
> -		GET_CHANNEL_INDEX(interrupts),
> -		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> -		LTT_DEFAULT_N_SUBBUFS_LOW,
> -	},
> -	[LTT_CHANNEL_NETWORK] = {
> -		LTT_NETWORK_CHANNEL,
> -		GET_CHANNEL_INDEX(network),
> -		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> -		LTT_DEFAULT_N_SUBBUFS_LOW,
> -	},
> -	[LTT_CHANNEL_MODULES] = {
> -		LTT_MODULES_CHANNEL,
> -		GET_CHANNEL_INDEX(modules),
> -		LTT_DEFAULT_SUBBUF_SIZE_LOW,
> -		LTT_DEFAULT_N_SUBBUFS_LOW,
> -	},
> -	[LTT_CHANNEL_METADATA] = {
> -		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)
>  {
> @@ -539,6 +551,52 @@ traces_error:
>  }
>  EXPORT_SYMBOL_GPL(ltt_trace_set_channel_subbufcount);
>  
> +int ltt_trace_set_channel_enable(const char *trace_name,
> +		const char *channel_name, unsigned enable)
> +{
> +	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;
> +	}
> +
> +	/*
> +	 * Datas in metadata channel(marker info) is necessary to be able to
> +	 * read the trace, we always enable this channel.
> +	 */
> +	if (!enable && channel == LTT_CHANNEL_METADATA) {
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
> +
> +	if (enable)
> +		trace->setting.channels[channel].flags |=
> +			CHANNEL_FLAG_ENABLE;
> +	else
> +		trace->setting.channels[channel].flags &=
> +			~CHANNEL_FLAG_ENABLE;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ltt_trace_set_channel_enable);
> +
>  int ltt_trace_set_channel_overwrite(const char *trace_name,
>  		const char *channel_name, unsigned overwrite)
>  {
> @@ -563,6 +621,12 @@ int ltt_trace_set_channel_overwrite(const char *trace_name,
>  		goto traces_error;
>  	}
>  
> +	/*
> +	 * 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.
> +	 */
>  	if (overwrite && channel == LTT_CHANNEL_METADATA) {
>  		err = -EINVAL;
>  		goto traces_error;
> @@ -635,14 +699,11 @@ int ltt_trace_alloc(const char *trace_name)
>  	 *   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.
> -	 */
>  	for (chan = 0; chan < NR_LTT_CHANNELS; chan++) {
> +		if (!(trace->setting.channels[chan].flags
> +			& CHANNEL_FLAG_ENABLE))
> +			continue;
> +
>  		subbuf_size = trace->setting.channels[chan].subbuf_size;
>  		subbuf_cnt = trace->setting.channels[chan].subbuf_cnt;
>  		prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> @@ -724,6 +785,10 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_METADATA_CHANNEL, 1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_METADATA_CHANNEL,
>  		is_channel_overwrite(LTT_CHANNEL_METADATA, mode));
> @@ -740,6 +805,11 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_INTERRUPTS_CHANNEL,
> +		1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_INTERRUPTS_CHANNEL,
>  		is_channel_overwrite(LTT_CHANNEL_INTERRUPTS, mode));
> @@ -756,6 +826,11 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_PROCESSES_CHANNEL,
> +		1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_PROCESSES_CHANNEL,
>  		is_channel_overwrite(LTT_CHANNEL_PROCESSES, mode));
> @@ -772,6 +847,10 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_MODULES_CHANNEL, 1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_MODULES_CHANNEL,
>  		is_channel_overwrite(LTT_CHANNEL_MODULES, mode));
> @@ -788,6 +867,10 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_NETWORK_CHANNEL, 1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_NETWORK_CHANNEL,
>  		is_channel_overwrite(LTT_CHANNEL_NETWORK, mode));
> @@ -804,6 +887,10 @@ static int ltt_trace_create(const char *trace_name, const char *trace_type,
>  	if (IS_ERR_VALUE(err))
>  		return err;
>  
> +	err = ltt_trace_set_channel_enable(trace_name, LTT_CPU_CHANNEL, 1);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	err = ltt_trace_set_channel_overwrite(trace_name,
>  		LTT_CPU_CHANNEL, is_channel_overwrite(LTT_CHANNEL_CPU, mode));
>  	if (IS_ERR_VALUE(err))
> @@ -855,12 +942,16 @@ traces_error:
>  /* Sleepable part of the destroy */
>  static void __ltt_trace_destroy(struct ltt_trace_struct	*trace)
>  {
> -	trace->ops->finish_channel(trace->channel.metadata);
> -	trace->ops->finish_channel(trace->channel.interrupts);
> -	trace->ops->finish_channel(trace->channel.processes);
> -	trace->ops->finish_channel(trace->channel.modules);
> -	trace->ops->finish_channel(trace->channel.network);
> -	trace->ops->finish_channel(trace->channel.cpu);
> +	int i;
> +	struct ltt_channel_struct *chan;
> +
> +	for (i = 0; i < NR_LTT_CHANNELS; i++) {
> +		chan = *(struct ltt_channel_struct **)((char *)trace
> +			+ chan_infos[i].channel_index);

Same here, wrapper should be used

> +		/* chan is NULL if it is not traced */
> +		if (chan)
> +			trace->ops->finish_channel(chan);
> +	}
>  
>  	flush_scheduled_work();
>  
> @@ -871,12 +962,13 @@ static void __ltt_trace_destroy(struct ltt_trace_struct	*trace)
>  	 */
>  	trace_async_wakeup(trace);
>  
> -	trace->ops->remove_channel(trace->channel.metadata);
> -	trace->ops->remove_channel(trace->channel.interrupts);
> -	trace->ops->remove_channel(trace->channel.processes);
> -	trace->ops->remove_channel(trace->channel.modules);
> -	trace->ops->remove_channel(trace->channel.network);
> -	trace->ops->remove_channel(trace->channel.cpu);
> +	for (i = 0; i < NR_LTT_CHANNELS; i++) {
> +		chan = *(struct ltt_channel_struct **)((char *)trace
> +			+ chan_infos[i].channel_index);

And here.

I'll merge your two patches and create the wrapper in a following patch.
It will be in the next LTTng version. Thanks!

Mathieu

> +		/* chan is NULL if it is not traced */
> +		if (chan)
> +			trace->ops->remove_channel(chan);
> +	}
>  
>  	kref_put(&trace->ltt_transport_kref, ltt_release_transport);
>  
> diff --git a/ltt/probes/ltt-type-serializer.c b/ltt/probes/ltt-type-serializer.c
> index 1f93181..317d329 100644
> --- a/ltt/probes/ltt-type-serializer.c
> +++ b/ltt/probes/ltt-type-serializer.c
> @@ -55,6 +55,10 @@ notrace void _ltt_specialized_trace(void *probe_data,
>  			rflags = 0;
>  #endif
>  		channel = ltt_get_channel_from_index(trace, channel_index);
> +		if (!channel)
> +			/* channel is NULL if it is not enabled */
> +			continue;
> +
>  		/* reserve space : header and data */
>  		ret = ltt_reserve_slot(trace, channel, &transport_data,
>  					data_size, &slot_size, &buf_offset,
> -- 
> 1.5.5.3
> 
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68