[ltt-dev] [PATCH 2/2] Make ltt support enable/disable each channel
Zhaolei
zhaolei at cn.fujitsu.com
Tue Dec 2 19:46:06 EST 2008
* From: "Mathieu Desnoyers" <mathieu.desnoyers at 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, <t_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
>
>
More information about the lttng-dev
mailing list