* [ltt-dev] [PATCH 1/1] Set ltt_active_marker.channel to enum ltt_channels
2008-12-03 14:16 ` [ltt-dev] [PATCH 1/1] " Zhaolei
@ 2008-12-03 17:34 ` Mathieu Desnoyers
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers @ 2008-12-03 17:34 UTC (permalink / raw)
* 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 <zhaolei at cn.fujitsu.com>
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
^ permalink raw reply [flat|nested] 3+ messages in thread