From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Wed, 3 Dec 2008 12:34:21 -0500 Subject: [ltt-dev] [PATCH 1/1] Set ltt_active_marker.channel to enum ltt_channels In-Reply-To: <493694A4.7000004@cn.fujitsu.com> References: <4936947C.1000204@cn.fujitsu.com> <493694A4.7000004@cn.fujitsu.com> Message-ID: <20081203173421.GA5069@Krystal> * Zhaolei (zhaolei at cn.fujitsu.com) wrote: > This way is more simple, and can get rid of following ugly statement: > > + chan = *(struct ltt_channel_struct **)((char *)trace > > + + chan_infos[i].channel_index); > > Signed-off-by: Zhao Lei Merged, will be in LTTng 0.62. Thanks ! Mathieu > --- > include/linux/ltt-tracer.h | 54 +++++++++---------------------------- > ltt/ltt-marker-control.c | 53 +++++++++++++++++-------------------- > ltt/ltt-relay-locked.c | 11 ++++--- > ltt/ltt-relay.c | 11 ++++--- > ltt/ltt-serialize.c | 6 ++-- > ltt/ltt-tracer.c | 22 +++------------ > ltt/probes/ltt-type-serializer.c | 6 ++-- > 7 files changed, 60 insertions(+), 103 deletions(-) > > diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h > index 87696a9..005b353 100644 > --- a/include/linux/ltt-tracer.h > +++ b/include/linux/ltt-tracer.h > @@ -83,13 +83,23 @@ struct ltt_probe_private_data { > */ > }; > > +enum ltt_channels { > + LTT_CHANNEL_CPU, > + LTT_CHANNEL_PROCESSES, > + LTT_CHANNEL_INTERRUPTS, > + LTT_CHANNEL_NETWORK, > + LTT_CHANNEL_MODULES, > + LTT_CHANNEL_METADATA, > + NR_LTT_CHANNELS, > +}; > + > struct ltt_active_marker { > struct list_head node; /* active markers list */ > const char *name; > const char *format; > struct ltt_available_probe *probe; > uint16_t id; > - uint16_t channel; > + enum ltt_channels channel_id; > }; > > extern void ltt_vtrace(void *probe_data, void *call_data, > @@ -203,24 +213,6 @@ struct ltt_transport { > struct ltt_trace_ops ops; > }; > > -/* > - * First and last channels in ltt_trace_struct. > - */ > -#define ltt_channel_index_size() sizeof(struct ltt_channel_struct *) > -#define ltt_channel_index_begin() GET_CHANNEL_INDEX(cpu) > -#define ltt_channel_index_end() \ > - (GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size()) > - > -enum ltt_channels { > - LTT_CHANNEL_CPU, > - LTT_CHANNEL_PROCESSES, > - LTT_CHANNEL_INTERRUPTS, > - LTT_CHANNEL_NETWORK, > - LTT_CHANNEL_MODULES, > - LTT_CHANNEL_METADATA, > - NR_LTT_CHANNELS, > -}; > - > enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID }; > > #define CHANNEL_FLAG_ENABLE (1U<<0) > @@ -233,17 +225,7 @@ struct ltt_trace_struct { > struct ltt_trace_ops *ops; > int active; > /* Second 32 bytes cache-hot cacheline */ > - struct { > - /* > - * Changing the order requires to change ltt_channel_index*() */ > - struct ltt_channel_struct *cpu; > - struct ltt_channel_struct *processes; > - struct ltt_channel_struct *interrupts; > - struct ltt_channel_struct *network; > - /* End of second 32 bytes cache-hot cacheline */ > - struct ltt_channel_struct *modules; > - struct ltt_channel_struct *metadata; > - } channel; > + struct ltt_channel_struct *channels[NR_LTT_CHANNELS]; > struct { > struct { > unsigned subbuf_size; > @@ -394,16 +376,6 @@ static inline size_t ltt_subbuffer_header_size(void) > return offsetof(struct ltt_subbuffer_header, header_end); > } > > -/* Get the offset of the channel in the ltt_trace_struct */ > -#define GET_CHANNEL_INDEX(chan) \ > - offsetof(struct ltt_trace_struct, channel.chan) > - > -static inline struct ltt_channel_struct *ltt_get_channel_from_index( > - struct ltt_trace_struct *trace, unsigned int index) > -{ > - return *(struct ltt_channel_struct **)((void *)trace+index); > -} > - > /* > * ltt_get_header_size > * > @@ -745,7 +717,7 @@ void ltt_release_transport(struct kref *kref); > extern int ltt_probe_register(struct ltt_available_probe *pdata); > extern int ltt_probe_unregister(struct ltt_available_probe *pdata); > extern int ltt_marker_connect(const char *mname, const char *pname, > - enum marker_id id, uint16_t channel, int user); > + enum marker_id id, enum ltt_channels channel_id, int user); > extern int ltt_marker_disconnect(const char *mname, const char *pname, > int user); > extern void ltt_dump_marker_state(struct ltt_trace_struct *trace); > diff --git a/ltt/ltt-marker-control.c b/ltt/ltt-marker-control.c > index 087292e..ae46646 100644 > --- a/ltt/ltt-marker-control.c > +++ b/ltt/ltt-marker-control.c > @@ -65,14 +65,13 @@ static struct proc_dir_entry *pentry; > static struct chan_name_info { > enum ltt_channels channels; > const char *name; > - unsigned int channel_index; > } channel_names[] = { > - { LTT_CHANNEL_CPU, "cpu", GET_CHANNEL_INDEX(cpu) }, > - { LTT_CHANNEL_PROCESSES, "processes", GET_CHANNEL_INDEX(processes) }, > - { LTT_CHANNEL_INTERRUPTS, "interrupts", GET_CHANNEL_INDEX(interrupts) }, > - { LTT_CHANNEL_NETWORK, "network", GET_CHANNEL_INDEX(network) }, > - { LTT_CHANNEL_MODULES, "modules", GET_CHANNEL_INDEX(modules) }, > - { LTT_CHANNEL_METADATA, "metadata", GET_CHANNEL_INDEX(metadata) }, > + { LTT_CHANNEL_CPU, "cpu" }, > + { LTT_CHANNEL_PROCESSES, "processes" }, > + { LTT_CHANNEL_INTERRUPTS, "interrupts" }, > + { LTT_CHANNEL_NETWORK, "network" }, > + { LTT_CHANNEL_MODULES, "modules" }, > + { LTT_CHANNEL_METADATA, "metadata" }, > }; > > static struct id_name_info { > @@ -104,7 +103,7 @@ static struct ltt_available_probe *get_probe_from_name(const char *pname) > return NULL; > } > > -static int get_channel_index_from_name(const char *name) > +static enum ltt_channels get_channel_id_from_name(const char *name) > { > struct chan_name_info *info; > > @@ -113,7 +112,7 @@ static int get_channel_index_from_name(const char *name) > for (info = channel_names; > info < channel_names + ARRAY_SIZE(channel_names); info++) { > if (!strcmp(name, info->name)) > - return info->channel_index; > + return info->channels; > } > return -ENOENT; > } > @@ -270,7 +269,7 @@ EXPORT_SYMBOL_GPL(ltt_probe_unregister); > * Only allow _only_ probe instance to be connected to a marker. > */ > int ltt_marker_connect(const char *mname, const char *pname, > - enum marker_id id, uint16_t channel, int user) > + enum marker_id id, enum ltt_channels channel_id, int user) > > { > int ret; > @@ -305,7 +304,7 @@ int ltt_marker_connect(const char *mname, const char *pname, > } > pdata->probe = probe; > pdata->id = assign_id(id); > - pdata->channel = channel; > + pdata->channel_id = channel_id; > /* > * ID has priority over channel in case of conflict. > */ > @@ -464,7 +463,7 @@ end: > * Beware : race between ID and channel set. See comment in do_set_id. > */ > static int do_set_channel(const char *name, const char *pname, > - uint16_t new_channel, int user) > + enum ltt_channels channel_id, int user) > { > struct ltt_active_marker *pdata; > struct ltt_available_probe *probe; > @@ -473,7 +472,7 @@ static int do_set_channel(const char *name, const char *pname, > /* > * Do not let userspace mess with core markers. > */ > - if (user && GET_CHANNEL_INDEX(metadata) == new_channel) > + if (user && LTT_CHANNEL_METADATA == channel_id) > return -EPERM; > > ltt_lock_traces(); > @@ -500,7 +499,7 @@ static int do_set_channel(const char *name, const char *pname, > ret = marker_probe_unregister(name, probe->probe_func, pdata); > if (ret) > goto end; > - pdata->channel = new_channel; > + pdata->channel_id = channel_id; > if (pdata->id >= MARKER_CORE_IDS) { > ret = _check_id_avail(MARKER_ID_DYNAMIC); > if (!ret) > @@ -608,7 +607,7 @@ static ssize_t ltt_write(struct file *file, const char __user *buffer, > char *kbuf; > char *iter, *marker_action, *arg[4]; > ssize_t ret; > - int channel_index; > + enum ltt_channels channel_id; > enum marker_id n_id; > int i; > > @@ -648,13 +647,12 @@ static ssize_t ltt_write(struct file *file, const char __user *buffer, > ret = n_id; > goto end; > } > - channel_index = get_channel_index_from_name(arg[3]); > - if (channel_index < 0) { > - ret = channel_index; > + channel_id = get_channel_id_from_name(arg[3]); > + if (channel_id < 0) { > + ret = channel_id; > goto end; > } > - ret = ltt_marker_connect(arg[0], arg[1], n_id, > - (uint16_t)channel_index, 1); > + ret = ltt_marker_connect(arg[0], arg[1], n_id, channel_id, 1); > if (ret) > goto end; > } else if (!strcmp(marker_action, "disconnect")) { > @@ -680,13 +678,12 @@ static ssize_t ltt_write(struct file *file, const char __user *buffer, > if (ret) > goto end; > } else if (!strcmp(marker_action, "set_channel")) { > - channel_index = get_channel_index_from_name(arg[2]); > - if (channel_index < 0) { > - ret = channel_index; > + channel_id = get_channel_id_from_name(arg[2]); > + if (channel_id < 0) { > + ret = channel_id; > goto end; > } > - ret = do_set_channel(arg[0], arg[1], > - (uint16_t)channel_index, 1); > + ret = do_set_channel(arg[0], arg[1], channel_id, 1); > if (ret) > goto end; > } else { > @@ -815,12 +812,10 @@ static int __init marker_control_init(void) > ret = ltt_probe_register(&default_probe); > BUG_ON(ret); > ret = ltt_marker_connect("core_marker_format", DEFAULT_PROBE, > - MARKER_ID_SET_MARKER_FORMAT, GET_CHANNEL_INDEX(metadata), > - 0); > + MARKER_ID_SET_MARKER_FORMAT, LTT_CHANNEL_METADATA, 0); > BUG_ON(ret); > ret = ltt_marker_connect("core_marker_id", DEFAULT_PROBE, > - MARKER_ID_SET_MARKER_ID, GET_CHANNEL_INDEX(metadata), > - 0); > + MARKER_ID_SET_MARKER_ID, LTT_CHANNEL_METADATA, 0); > BUG_ON(ret); > pentry->proc_fops = <t_fops; > > diff --git a/ltt/ltt-relay-locked.c b/ltt/ltt-relay-locked.c > index 77ee327..2707949 100644 > --- a/ltt/ltt-relay-locked.c > +++ b/ltt/ltt-relay-locked.c > @@ -1488,7 +1488,8 @@ static notrace void ltt_relay_commit_slot( > * specific threshold value, we reenable preemption and block. > */ > static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > - unsigned int index, size_t data_size, struct user_dbg_data *dbg) > + enum ltt_channels channel_id, size_t data_size, > + struct user_dbg_data *dbg) > { > struct rchan *rchan; > struct ltt_channel_buf_struct *ltt_buf; > @@ -1497,7 +1498,7 @@ static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > int cpu; > DECLARE_WAITQUEUE(wait, current); > > - channel = ltt_get_channel_from_index(trace, index); > + channel = trace->channels[channel_id]; > rchan = channel->trans_channel_data; > cpu = smp_processor_id(); > relay_buf = rchan->buf[cpu]; > @@ -1544,15 +1545,15 @@ static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > } > > static void ltt_relay_print_user_errors(struct ltt_trace_struct *trace, > - unsigned int index, size_t data_size, struct user_dbg_data *dbg, > - int cpu) > + enum ltt_channels channel_id, size_t data_size, > + struct user_dbg_data *dbg, int cpu) > { > struct rchan *rchan; > struct ltt_channel_buf_struct *ltt_buf; > struct ltt_channel_struct *channel; > struct rchan_buf *relay_buf; > > - channel = ltt_get_channel_from_index(trace, index); > + channel = trace->channels[channel_id]; > rchan = channel->trans_channel_data; > relay_buf = rchan->buf[cpu]; > ltt_buf = percpu_ptr(channel->buf, cpu); > diff --git a/ltt/ltt-relay.c b/ltt/ltt-relay.c > index b6d4304..1cb9162 100644 > --- a/ltt/ltt-relay.c > +++ b/ltt/ltt-relay.c > @@ -1513,7 +1513,8 @@ static notrace void ltt_relay_commit_slot( > * specific threshold value, we reenable preemption and block. > */ > static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > - unsigned int index, size_t data_size, struct user_dbg_data *dbg) > + enum ltt_channels channel_id, size_t data_size, > + struct user_dbg_data *dbg) > { > struct rchan *rchan; > struct ltt_channel_buf_struct *ltt_buf; > @@ -1522,7 +1523,7 @@ static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > int cpu; > DECLARE_WAITQUEUE(wait, current); > > - channel = ltt_get_channel_from_index(trace, index); > + channel = trace->channels[channel_id]; > rchan = channel->trans_channel_data; > cpu = smp_processor_id(); > relay_buf = rchan->buf[cpu]; > @@ -1566,15 +1567,15 @@ static int ltt_relay_user_blocking(struct ltt_trace_struct *trace, > } > > static void ltt_relay_print_user_errors(struct ltt_trace_struct *trace, > - unsigned int index, size_t data_size, struct user_dbg_data *dbg, > - int cpu) > + enum ltt_channels channel_id, size_t data_size, > + struct user_dbg_data *dbg, int cpu) > { > struct rchan *rchan; > struct ltt_channel_buf_struct *ltt_buf; > struct ltt_channel_struct *channel; > struct rchan_buf *relay_buf; > > - channel = ltt_get_channel_from_index(trace, index); > + channel = trace->channels[channel_id]; > rchan = channel->trans_channel_data; > relay_buf = rchan->buf[cpu]; > ltt_buf = percpu_ptr(channel->buf, cpu); > diff --git a/ltt/ltt-serialize.c b/ltt/ltt-serialize.c > index 9d7ce59..a9a37e2 100644 > --- a/ltt/ltt-serialize.c > +++ b/ltt/ltt-serialize.c > @@ -565,7 +565,7 @@ notrace void ltt_vtrace(void *probe_data, void *call_data, > struct ltt_active_marker *pdata; > uint16_t eID; > size_t data_size, slot_size; > - int channel_index; > + enum ltt_channels channel_id; > struct ltt_channel_struct *channel; > struct ltt_trace_struct *trace, *dest_trace = NULL; > struct rchan_buf *buf; > @@ -592,7 +592,7 @@ notrace void ltt_vtrace(void *probe_data, void *call_data, > > pdata = (struct ltt_active_marker *)probe_data; > eID = pdata->id; > - channel_index = pdata->channel; > + channel_id = pdata->channel_id; > closure.callbacks = pdata->probe->callbacks; > > if (unlikely(private_data)) { > @@ -632,7 +632,7 @@ notrace void ltt_vtrace(void *probe_data, void *call_data, > else > rflags = 0; > #endif > - channel = ltt_get_channel_from_index(trace, channel_index); > + channel = trace->channels[channel_id]; > if (!channel) > /* channel is NULL if it is not enabled */ > continue; > diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c > index a14fd5f..474ffdd 100644 > --- a/ltt/ltt-tracer.c > +++ b/ltt/ltt-tracer.c > @@ -77,43 +77,36 @@ struct module *ltt_statedump_owner; > */ > 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, > }, > @@ -284,8 +277,7 @@ static void trace_async_wakeup(struct ltt_trace_struct *trace) > > /* Must check each channel for pending read wakeup */ > for (i = 0; i < NR_LTT_CHANNELS; i++) { > - chan = *(struct ltt_channel_struct **)((char *)trace > - + chan_infos[i].channel_index); > + chan = trace->channels[i]; > /* chan is NULL if it is not traced */ > if (chan) > trace->ops->wakeup_channel(chan); > @@ -712,8 +704,7 @@ int ltt_trace_alloc(const char *trace_name) > err = trace->ops->create_channel(trace_name, trace, > trace->dentry.control_root, > chan_infos[chan].name, > - (struct ltt_channel_struct **)((char *)trace > - + chan_infos[chan].channel_index), > + &trace->channels[chan], > subbuf_size, > subbuf_cnt, > trace->setting.channels[chan].flags > @@ -741,8 +732,7 @@ int ltt_trace_alloc(const char *trace_name) > > create_channel_error: > for (chan--; chan >= 0; chan--) > - trace->ops->remove_channel(*(struct ltt_channel_struct **) > - ((char *)trace + chan_infos[chan].channel_index)); > + trace->ops->remove_channel(trace->channels[chan]); > > dirs_error: > module_put(trace->transport->owner); > @@ -946,8 +936,7 @@ static void __ltt_trace_destroy(struct ltt_trace_struct *trace) > 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 = trace->channels[i]; > /* chan is NULL if it is not traced */ > if (chan) > trace->ops->finish_channel(chan); > @@ -963,8 +952,7 @@ static void __ltt_trace_destroy(struct ltt_trace_struct *trace) > trace_async_wakeup(trace); > > for (i = 0; i < NR_LTT_CHANNELS; i++) { > - chan = *(struct ltt_channel_struct **)((char *)trace > - + chan_infos[i].channel_index); > + chan = trace->channels[i]; > /* chan is NULL if it is not traced */ > if (chan) > trace->ops->remove_channel(chan); > diff --git a/ltt/probes/ltt-type-serializer.c b/ltt/probes/ltt-type-serializer.c > index 317d329..ae48323 100644 > --- a/ltt/probes/ltt-type-serializer.c > +++ b/ltt/probes/ltt-type-serializer.c > @@ -16,7 +16,7 @@ notrace void _ltt_specialized_trace(void *probe_data, > struct ltt_active_marker *pdata; > uint16_t eID; > size_t slot_size; > - int channel_index; > + enum ltt_channels channel_id; > struct ltt_channel_struct *channel; > struct ltt_trace_struct *trace; > struct rchan_buf *buf; > @@ -38,7 +38,7 @@ notrace void _ltt_specialized_trace(void *probe_data, > > pdata = (struct ltt_active_marker *)probe_data; > eID = pdata->id; > - channel_index = pdata->channel; > + channel_id = pdata->channel_id; > > /* Iterate on each trace */ > list_for_each_entry_rcu(trace, <t_traces.head, list) { > @@ -54,7 +54,7 @@ notrace void _ltt_specialized_trace(void *probe_data, > else > rflags = 0; > #endif > - channel = ltt_get_channel_from_index(trace, channel_index); > + channel = trace->channels[channel_id]; > if (!channel) > /* channel is NULL if it is not enabled */ > continue; > -- > 1.5.5.3 > > > > _______________________________________________ > ltt-dev mailing list > ltt-dev at lists.casi.polymtl.ca > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > -- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68