[ltt-dev] [PATCH 0/2] Make lttng support enable/disable trace channel

[ltt-dev] [PATCH 0/2] Make lttng support enable/disable trace channel

[Zhaolei]

Hello, Mathieu

As we discussed in "Re: [ltt-dev] Make lttctl use new interface",
we need make lttctl select some of channel to trace.

At first step, we need make lttng support link part of channels into a trace.
It is implemented by this patch.

step2: make ltt-trace-control support this function.
step3: make user-more lttctl support this function.

B.R.
Zhaolei

[ltt-dev] [PATCH 1/2] Move ltt_trace_struct.channels[].overwrite to ltt_trace_struct.channels[].flags

[Zhaolei]

So we can save many attribute bits in it.
For support new "ENABLED" bit.

Signed-off-by: Zhao lei <zhaolei at cn.fujitsu.com>
---
 include/linux/ltt-tracer.h |    5 ++++-
 ltt/ltt-tracer.c           |   10 ++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
index 32aad93..a18edd1 100644
--- a/include/linux/ltt-tracer.h
+++ b/include/linux/ltt-tracer.h
@@ -223,6 +223,9 @@ enum ltt_channels {
 
 enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID };
 
+#define CHANNEL_FLAG_ENABLE	(1U<<0)
+#define CHANNEL_FLAG_OVERWRITE	(1U<<1)
+
 /* Per-trace information - each trace/flight recorder represented by one */
 struct ltt_trace_struct {
 	/* First 32 bytes cache-hot cacheline */
@@ -245,7 +248,7 @@ struct ltt_trace_struct {
 		struct {
 			unsigned subbuf_size;
 			unsigned subbuf_cnt;
-			unsigned overwrite;
+			unsigned flags;
 		} channels[NR_LTT_CHANNELS];
 	} setting;
 	u32 freq_scale;
diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
index 41408f3..341074d 100644
--- a/ltt/ltt-tracer.c
+++ b/ltt/ltt-tracer.c
@@ -568,7 +568,12 @@ int ltt_trace_set_channel_overwrite(const char *trace_name,
 		goto traces_error;
 	}
 
-	trace->setting.channels[channel].overwrite = !!overwrite;
+	if (overwrite)
+		trace->setting.channels[channel].flags |=
+			CHANNEL_FLAG_OVERWRITE;
+	else
+		trace->setting.channels[channel].flags &=
+			~CHANNEL_FLAG_OVERWRITE;
 
 traces_error:
 	ltt_unlock_traces();
@@ -650,7 +655,8 @@ int ltt_trace_alloc(const char *trace_name)
 				+ chan_infos[chan].channel_index),
 				subbuf_size,
 				subbuf_cnt,
-				trace->setting.channels[chan].overwrite);
+				trace->setting.channels[chan].flags
+					& CHANNEL_FLAG_OVERWRITE);
 		if (err != 0) {
 			printk(KERN_ERR	"LTT : Can't create channel %s.\n",
 				chan_infos[chan].name);
-- 
1.5.5.3

[ltt-dev] [PATCH 2/2] Make ltt support enable/disable each channel

[Zhaolei]

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);
+		/* 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);
+		/* 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);
+		/* 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

[ltt-dev] [PATCH 2/2] Make ltt support enable/disable each channel

[Mathieu Desnoyers]

* 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

[ltt-dev] [PATCH 2/2] Make ltt support enable/disable each channel

[Zhaolei]

* From: "Mathieu Desnoyers" <mathieu.desnoyers@polymtl.ca>
>* 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);
> }
Hello, Mathieu

Thanks for your revice and merge.

IMHO, It is ugly, one of reason is because we use "offset of channel's pointer in ltt_trace_struct"
as ltt_active_marker.channel.

I think we can use other way to link a channel to marker, for example:
set ltt_active_marker.channel = struct ltt_channel_struct *.
or
set ltt_active_marker.channel = enum ltt_channels.

We can change it at same time we make channel dynamic(user can create new channel).

B.R.
Zhaolei
> 
> 
> 
>> + /* 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
> 
>

[ltt-dev] [PATCH 2/2] Make ltt support enable/disable each channel

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> * From: "Mathieu Desnoyers" <mathieu.desnoyers@polymtl.ca>
> >* 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);
> > }
> Hello, Mathieu
> 
> Thanks for your revice and merge.
> 
> IMHO, It is ugly, one of reason is because we use "offset of channel's pointer in ltt_trace_struct"
> as ltt_active_marker.channel.
> 
> I think we can use other way to link a channel to marker, for example:
> set ltt_active_marker.channel = struct ltt_channel_struct *.
> or
> set ltt_active_marker.channel = enum ltt_channels.
> 
> We can change it at same time we make channel dynamic(user can create new channel).
> 

Ideally, if we can use a struct ltt_channel_struct * directly, this
would remove the need for having a base + offset.

Yes, that would be good. Feel free to try implementing this improvement
in the new LTTng 0.61.

Mathieu

> B.R.
> Zhaolei
> > 
> > 
> > 
> >> + /* 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
> > 
> >
> 

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