[ltt-dev] [PATCH 1/2] Add a data pointer to all tracepoint probe callbacks
Nils Carlson
nils at as68123.uab.ericsson.se
Thu Sep 2 05:08:54 EDT 2010
This is just a clean up of the earlier patch, removed some things that
aren't necessary yet.
Should have labeled v2. Sorry. :-/
/Nils
On Thu, 2 Sep 2010, Nils Carlson wrote:
> RCU now dereferences an array of probe structs containing data pointers and
> function pointers, and passes the data pointer as the first argument to the
> probe.
>
> Due to this change void prototypes are no longer allowed, which has incurred
> changes to libust-initializer.c .
>
> This patch also fixes a problem where non-existent tracepoints are present
> in the __tracepoints section and with a name pointer pointing at NULL.
> ---
> include/ust/tracepoint.h | 68 +++++++++++++++++++++---------
> include/ust/ust_trace.h | 5 +-
> lgpl-relicensing.txt | 1 +
> libust-initializer.c | 5 +-
> libust/tracepoint.c | 103 ++++++++++++++++++++++++++-------------------
> 5 files changed, 113 insertions(+), 69 deletions(-)
>
> diff --git a/include/ust/tracepoint.h b/include/ust/tracepoint.h
> index 1a91b79..9c7c091 100644
> --- a/include/ust/tracepoint.h
> +++ b/include/ust/tracepoint.h
> @@ -33,10 +33,15 @@
> struct module;
> struct tracepoint;
>
> +struct probe {
> + void *func;
> + void *data;
> +};
> +
> struct tracepoint {
> const char *name; /* Tracepoint name */
> DEFINE_IMV(char, state); /* State. */
> - void **funcs;
> + struct probe *probes;
> } __attribute__((aligned(32))); /*
> * Aligned on 32 bytes because it is
> * globally visible and gcc happily
> @@ -58,16 +63,20 @@ struct tracepoint {
> */
> #define __DO_TRACE(tp, proto, args) \
> do { \
> - void **it_func; \
> + struct probe *it_probe_ptr; \
> + void *it_func; \
> + void *__data; \
> \
> - rcu_read_lock(); /*ust rcu_read_lock_sched_notrace(); */ \
> - it_func = rcu_dereference((tp)->funcs); \
> - if (it_func) { \
> + rcu_read_lock(); \
> + it_probe_ptr = rcu_dereference((tp)->probes); \
> + if (it_probe_ptr) { \
> do { \
> - ((void(*)(proto))(*it_func))(args); \
> - } while (*(++it_func)); \
> + it_func = (it_probe_ptr)->func; \
> + __data = (it_probe_ptr)->data; \
> + ((void(*)(proto))(it_func))(args); \
> + } while ((++it_probe_ptr)->func); \
> } \
> - rcu_read_unlock(); /*ust rcu_read_unlock_sched_notrace(); */ \
> + rcu_read_unlock(); \
> } while (0)
>
> #define __CHECK_TRACE(name, generic, proto, args) \
> @@ -93,37 +102,47 @@ struct tracepoint {
> * If generic is true, a variable read is used.
> * If generic is false, immediate values are used.
> */
> -#define DECLARE_TRACE(name, proto, args) \
> +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> extern struct tracepoint __tracepoint_##name; \
> static inline void trace_##name(proto) \
> { \
> - __CHECK_TRACE(name, 0, TP_PROTO(proto), TP_ARGS(args)); \
> + __CHECK_TRACE(name, 0, TP_PROTO(data_proto), \
> + TP_ARGS(data_args)); \
> } \
> static inline void _trace_##name(proto) \
> { \
> - __CHECK_TRACE(name, 1, TP_PROTO(proto), TP_ARGS(args)); \
> + __CHECK_TRACE(name, 1, TP_PROTO(data_proto), \
> + TP_ARGS(data_args)); \
> } \
> - static inline int register_trace_##name(void (*probe)(proto)) \
> + static inline int \
> + register_trace_##name(void (*probe)(data_proto), void *data) \
> { \
> - return tracepoint_probe_register(#name, (void *)probe); \
> + return tracepoint_probe_register(#name, (void *)probe, \
> + data); \
> + \
> } \
> - static inline int unregister_trace_##name(void (*probe)(proto)) \
> + static inline int \
> + unregister_trace_##name(void (*probe)(data_proto), void *data) \
> { \
> - return tracepoint_probe_unregister(#name, (void *)probe);\
> + return tracepoint_probe_unregister(#name, (void *)probe, \
> + data); \
> }
>
> -#define DEFINE_TRACE(name) \
> +#define DEFINE_TRACE_FN(name, reg, unreg) \
> static const char __tpstrtab_##name[] \
> __attribute__((section("__tracepoints_strings"))) = #name; \
> struct tracepoint __tracepoint_##name \
> __attribute__((section("__tracepoints"), aligned(32))) = \
> { __tpstrtab_##name, 0, NULL }
>
> +#define DEFINE_TRACE(name) \
> + DEFINE_TRACE_FN(name, NULL, NULL);
> +
> extern void tracepoint_update_probe_range(struct tracepoint *begin,
> struct tracepoint *end);
>
> #else /* !CONFIG_TRACEPOINTS */
> -#define DECLARE_TRACE(name, proto, args) \
> +#define __DECLARE_TRACE(name, proto, args) \
> static inline void trace_##name(proto) \
> { } \
> static inline void _trace_##name(proto) \
> @@ -146,20 +165,27 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
> { }
> #endif /* CONFIG_TRACEPOINTS */
>
> +#define DECLARE_TRACE(name, proto, args) \
> + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> + PARAMS(void *__data, proto), \
> + PARAMS(__data, args))
> +
> /*
> * Connect a probe to a tracepoint.
> * Internal API, should not be used directly.
> */
> -extern int tracepoint_probe_register(const char *name, void *probe);
> +extern int tracepoint_probe_register(const char *name, void *probe, void *data);
>
> /*
> * Disconnect a probe from a tracepoint.
> * Internal API, should not be used directly.
> */
> -extern int tracepoint_probe_unregister(const char *name, void *probe);
> +extern int tracepoint_probe_unregister(const char *name, void *probe, void *data);
>
> -extern int tracepoint_probe_register_noupdate(const char *name, void *probe);
> -extern int tracepoint_probe_unregister_noupdate(const char *name, void *probe);
> +extern int tracepoint_probe_register_noupdate(const char *name, void *probe,
> + void *data);
> +extern int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
> + void *data);
> extern void tracepoint_probe_update_all(void);
>
> struct tracepoint_iter {
> diff --git a/include/ust/ust_trace.h b/include/ust/ust_trace.h
> index 098c5f8..e1b0257 100644
> --- a/include/ust/ust_trace.h
> +++ b/include/ust/ust_trace.h
> @@ -58,7 +58,7 @@
> struct trace_raw_##name { \
> tstruct \
> }; \
> - static void trace_printf_##name(proto) \
> + static void trace_printf_##name(void *dummy, proto) \
> { \
> struct trace_raw_##name entry_struct, *__entry; \
> __entry = &entry_struct; \
> @@ -68,8 +68,9 @@
> } \
> static void __attribute__((constructor)) init_##name() \
> { \
> + void *dummy; \
> printf("connecting tracepoint " #name "\n"); \
> - register_trace_##name(trace_printf_##name); \
> + register_trace_##name(trace_printf_##name, dummy); \
> }
>
>
> diff --git a/lgpl-relicensing.txt b/lgpl-relicensing.txt
> index 0393851..753f453 100644
> --- a/lgpl-relicensing.txt
> +++ b/lgpl-relicensing.txt
> @@ -10,6 +10,7 @@ Steven Rostedt <rostedt at goodmis.org>
> Frederic Weisbecker <fweisbec at gmail.com>
> Xiao Guangrong <xiaogunagrong at cn.fujitsu.com>
> Josh Stone <jistone at redhat.com>
> +Lai Jiangshan <laijs at cn.fujitsu.com>
>
> The following copyright holders have agreed to LGPLv2.1+ re-licensing
> of their contributions to linux/include/stringify.h.
> diff --git a/libust-initializer.c b/libust-initializer.c
> index ec39932..390218e 100644
> --- a/libust-initializer.c
> +++ b/libust-initializer.c
> @@ -18,13 +18,14 @@
> * avoid this.
> */
>
> -DECLARE_TRACE(ust_dummytp, TP_PROTO(void), TP_ARGS());
> +DECLARE_TRACE(ust_dummytp, TP_PROTO(int anint), TP_ARGS(anint));
> DEFINE_TRACE(ust_dummytp);
>
> void dummy_libust_initializer_func(void)
> {
> + int i;
> trace_mark(ust, dummymark, MARK_NOARGS);
> - trace_ust_dummytp();
> + trace_ust_dummytp(i);
> }
>
> MARKER_LIB;
> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
> index 6155e29..620a009 100644
> --- a/libust/tracepoint.c
> +++ b/libust/tracepoint.c
> @@ -59,7 +59,7 @@ static struct hlist_head tracepoint_table[TRACEPOINT_TABLE_SIZE];
> */
> struct tracepoint_entry {
> struct hlist_node hlist;
> - void **funcs;
> + struct probe *probes;
> int refcount; /* Number of times armed. 0 if disarmed. */
> char name[0];
> };
> @@ -69,12 +69,12 @@ struct tp_probes {
> //ust// struct rcu_head rcu;
> struct list_head list;
> } u;
> - void *probes[0];
> + struct probe probes[0];
> };
>
> static inline void *allocate_probes(int count)
> {
> - struct tp_probes *p = malloc(count * sizeof(void *)
> + struct tp_probes *p = malloc(count * sizeof(struct probe)
> + sizeof(struct tp_probes));
> return p == NULL ? NULL : p->probes;
> }
> @@ -99,27 +99,29 @@ static void debug_print_probes(struct tracepoint_entry *entry)
> {
> int i;
>
> - if (!tracepoint_debug || !entry->funcs)
> + if (!tracepoint_debug || !entry->probes)
> return;
>
> - for (i = 0; entry->funcs[i]; i++)
> - DBG("Probe %d : %p", i, entry->funcs[i]);
> + for (i = 0; entry->probes[i].func; i++)
> + DBG("Probe %d : %p", i, entry->probes[i].func);
> }
>
> static void *
> -tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
> +tracepoint_entry_add_probe(struct tracepoint_entry *entry,
> + void *probe, void *data)
> {
> int nr_probes = 0;
> - void **old, **new;
> + struct probe *old, *new;
>
> WARN_ON(!probe);
>
> debug_print_probes(entry);
> - old = entry->funcs;
> + old = entry->probes;
> if (old) {
> /* (N -> N+1), (N != 0, 1) probes */
> - for (nr_probes = 0; old[nr_probes]; nr_probes++)
> - if (old[nr_probes] == probe)
> + for (nr_probes = 0; old[nr_probes].func; nr_probes++)
> + if (old[nr_probes].func == probe &&
> + old[nr_probes].data == data)
> return ERR_PTR(-EEXIST);
> }
> /* + 2 : one for new probe, one for NULL func */
> @@ -127,36 +129,40 @@ tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
> if (new == NULL)
> return ERR_PTR(-ENOMEM);
> if (old)
> - memcpy(new, old, nr_probes * sizeof(void *));
> - new[nr_probes] = probe;
> - new[nr_probes + 1] = NULL;
> + memcpy(new, old, nr_probes * sizeof(struct probe));
> + new[nr_probes].func = probe;
> + new[nr_probes].data = data;
> + new[nr_probes + 1].func = NULL;
> entry->refcount = nr_probes + 1;
> - entry->funcs = new;
> + entry->probes = new;
> debug_print_probes(entry);
> return old;
> }
>
> static void *
> -tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe)
> +tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe,
> + void *data)
> {
> int nr_probes = 0, nr_del = 0, i;
> - void **old, **new;
> + struct probe *old, *new;
>
> - old = entry->funcs;
> + old = entry->probes;
>
> if (!old)
> return ERR_PTR(-ENOENT);
>
> debug_print_probes(entry);
> /* (N -> M), (N > 1, M >= 0) probes */
> - for (nr_probes = 0; old[nr_probes]; nr_probes++) {
> - if ((!probe || old[nr_probes] == probe))
> + for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
> + if ((!probe ||
> + old[nr_probes].func == probe &&
> + old[nr_probes].data == data))
> nr_del++;
> }
>
> if (nr_probes - nr_del == 0) {
> /* N -> 0, (N > 1) */
> - entry->funcs = NULL;
> + entry->probes = NULL;
> entry->refcount = 0;
> debug_print_probes(entry);
> return old;
> @@ -167,12 +173,13 @@ tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe)
> new = allocate_probes(nr_probes - nr_del + 1);
> if (new == NULL)
> return ERR_PTR(-ENOMEM);
> - for (i = 0; old[i]; i++)
> - if ((probe && old[i] != probe))
> + for (i = 0; old[i].func; i++)
> + if (probe &&
> + (old[i].func != probe || old[i].data != data))
> new[j++] = old[i];
> - new[nr_probes - nr_del] = NULL;
> + new[nr_probes - nr_del].func = NULL;
> entry->refcount = nr_probes - nr_del;
> - entry->funcs = new;
> + entry->probes = new;
> }
> debug_print_probes(entry);
> return old;
> @@ -225,7 +232,7 @@ static struct tracepoint_entry *add_tracepoint(const char *name)
> if (!e)
> return ERR_PTR(-ENOMEM);
> memcpy(&e->name[0], name, name_len);
> - e->funcs = NULL;
> + e->probes = NULL;
> e->refcount = 0;
> hlist_add_head(&e->hlist, head);
> return e;
> @@ -256,7 +263,7 @@ static void set_tracepoint(struct tracepoint_entry **entry,
> * include/linux/tracepoints.h. A matching smp_read_barrier_depends()
> * is used.
> */
> - rcu_assign_pointer(elem->funcs, (*entry)->funcs);
> + rcu_assign_pointer(elem->probes, (*entry)->probes);
> elem->state__imv = active;
> }
>
> @@ -269,7 +276,7 @@ static void set_tracepoint(struct tracepoint_entry **entry,
> static void disable_tracepoint(struct tracepoint *elem)
> {
> elem->state__imv = 0;
> - rcu_assign_pointer(elem->funcs, NULL);
> + rcu_assign_pointer(elem->probes, NULL);
> }
>
> /**
> @@ -280,13 +287,17 @@ static void disable_tracepoint(struct tracepoint *elem)
> * Updates the probe callback corresponding to a range of tracepoints.
> */
> void tracepoint_update_probe_range(struct tracepoint *begin,
> - struct tracepoint *end)
> + struct tracepoint *end)
> {
> struct tracepoint *iter;
> struct tracepoint_entry *mark_entry;
>
> pthread_mutex_lock(&tracepoints_mutex);
> for (iter = begin; iter < end; iter++) {
> + if (!iter->name) {
> + disable_tracepoint(iter);
> + continue;
> + }
> mark_entry = get_tracepoint(iter->name);
> if (mark_entry) {
> set_tracepoint(&mark_entry, iter,
> @@ -324,18 +335,19 @@ static void tracepoint_update_probes(void)
> //ust// module_imv_update();
> }
>
> -static void *tracepoint_add_probe(const char *name, void *probe)
> +static struct probe *
> +tracepoint_add_probe(const char *name, void *probe, void *data)
> {
> struct tracepoint_entry *entry;
> - void *old;
> + struct probe *old;
>
> entry = get_tracepoint(name);
> if (!entry) {
> entry = add_tracepoint(name);
> if (IS_ERR(entry))
> - return entry;
> + return (struct probe *)entry;
> }
> - old = tracepoint_entry_add_probe(entry, probe);
> + old = tracepoint_entry_add_probe(entry, probe, data);
> if (IS_ERR(old) && !entry->refcount)
> remove_tracepoint(entry);
> return old;
> @@ -349,12 +361,12 @@ static void *tracepoint_add_probe(const char *name, void *probe)
> * Returns 0 if ok, error value on error.
> * The probe address must at least be aligned on the architecture pointer size.
> */
> -int tracepoint_probe_register(const char *name, void *probe)
> +int tracepoint_probe_register(const char *name, void *probe, void *data)
> {
> void *old;
>
> pthread_mutex_lock(&tracepoints_mutex);
> - old = tracepoint_add_probe(name, probe);
> + old = tracepoint_add_probe(name, probe, data);
> pthread_mutex_unlock(&tracepoints_mutex);
> if (IS_ERR(old))
> return PTR_ERR(old);
> @@ -365,7 +377,7 @@ int tracepoint_probe_register(const char *name, void *probe)
> }
> //ust// EXPORT_SYMBOL_GPL(tracepoint_probe_register);
>
> -static void *tracepoint_remove_probe(const char *name, void *probe)
> +static void *tracepoint_remove_probe(const char *name, void *probe, void *data)
> {
> struct tracepoint_entry *entry;
> void *old;
> @@ -373,7 +385,7 @@ static void *tracepoint_remove_probe(const char *name, void *probe)
> entry = get_tracepoint(name);
> if (!entry)
> return ERR_PTR(-ENOENT);
> - old = tracepoint_entry_remove_probe(entry, probe);
> + old = tracepoint_entry_remove_probe(entry, probe, data);
> if (IS_ERR(old))
> return old;
> if (!entry->refcount)
> @@ -385,18 +397,19 @@ static void *tracepoint_remove_probe(const char *name, void *probe)
> * tracepoint_probe_unregister - Disconnect a probe from a tracepoint
> * @name: tracepoint name
> * @probe: probe function pointer
> + * @probe: probe data pointer
> *
> * We do not need to call a synchronize_sched to make sure the probes have
> * finished running before doing a module unload, because the module unload
> * itself uses stop_machine(), which insures that every preempt disabled section
> * have finished.
> */
> -int tracepoint_probe_unregister(const char *name, void *probe)
> +int tracepoint_probe_unregister(const char *name, void *probe, void *data)
> {
> void *old;
>
> pthread_mutex_lock(&tracepoints_mutex);
> - old = tracepoint_remove_probe(name, probe);
> + old = tracepoint_remove_probe(name, probe, data);
> pthread_mutex_unlock(&tracepoints_mutex);
> if (IS_ERR(old))
> return PTR_ERR(old);
> @@ -427,12 +440,13 @@ static void tracepoint_add_old_probes(void *old)
> *
> * caller must call tracepoint_probe_update_all()
> */
> -int tracepoint_probe_register_noupdate(const char *name, void *probe)
> +int tracepoint_probe_register_noupdate(const char *name, void *probe,
> + void *data)
> {
> void *old;
>
> pthread_mutex_lock(&tracepoints_mutex);
> - old = tracepoint_add_probe(name, probe);
> + old = tracepoint_add_probe(name, probe, data);
> if (IS_ERR(old)) {
> pthread_mutex_unlock(&tracepoints_mutex);
> return PTR_ERR(old);
> @@ -450,12 +464,13 @@ int tracepoint_probe_register_noupdate(const char *name, void *probe)
> *
> * caller must call tracepoint_probe_update_all()
> */
> -int tracepoint_probe_unregister_noupdate(const char *name, void *probe)
> +int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
> + void *data)
> {
> void *old;
>
> pthread_mutex_lock(&tracepoints_mutex);
> - old = tracepoint_remove_probe(name, probe);
> + old = tracepoint_remove_probe(name, probe, data);
> if (IS_ERR(old)) {
> pthread_mutex_unlock(&tracepoints_mutex);
> return PTR_ERR(old);
> @@ -662,7 +677,7 @@ int tracepoint_register_lib(struct tracepoint *tracepoints_start, int tracepoint
> lib_update_tracepoints();
>
> DBG("just registered a tracepoints section from %p and having %d tracepoints", tracepoints_start, tracepoints_count);
> -
> +
> return 0;
> }
>
> --
> 1.7.1
>
>
More information about the lttng-dev
mailing list