[ltt-dev] [PATCH 1/2] Add a data pointer to all tracepoint probe callbacks

Nils Carlson nils.carlson at ericsson.com
Tue Aug 31 09:52:55 EDT 2010


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 |   76 +++++++++++++++++++++++-----------
 include/ust/ust_trace.h  |    5 +-
 lgpl-relicensing.txt     |    1 +
 libust-initializer.c     |    5 +-
 libust/tracepoint.c      |  103 ++++++++++++++++++++++++++-------------------
 5 files changed, 118 insertions(+), 72 deletions(-)

diff --git a/include/ust/tracepoint.h b/include/ust/tracepoint.h
index 1a91b79..47171f6 100644
--- a/include/ust/tracepoint.h
+++ b/include/ust/tracepoint.h
@@ -33,10 +33,17 @@
 struct module;
 struct tracepoint;
 
+struct probe {
+	void *func;
+	void *data;
+};
+
 struct tracepoint {
 	const char *name;		/* Tracepoint name */
 	DEFINE_IMV(char, state);	/* State. */
-	void **funcs;
+	void (*regfunc)(void);
+	void (*unregfunc)(void);
+	struct probe *probes;
 } __attribute__((aligned(32)));		/*
 					 * Aligned on 32 bytes because it is
 					 * globally visible and gcc happily
@@ -58,16 +65,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)			\
@@ -75,11 +86,11 @@ struct tracepoint {
 		if (!generic) {						\
 			if (unlikely(imv_read(__tracepoint_##name.state))) \
 				__DO_TRACE(&__tracepoint_##name,	\
-					TP_PROTO(proto), TP_ARGS(args));	\
+					   TP_PROTO(proto), TP_ARGS(args)); \
 		} else {						\
 			if (unlikely(_imv_read(__tracepoint_##name.state))) \
 				__DO_TRACE(&__tracepoint_##name,	\
-					TP_PROTO(proto), TP_ARGS(args));	\
+					   TP_PROTO(proto), TP_ARGS(args)); \
 		}							\
 	} while (0)
 
@@ -93,37 +104,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 }
+	{ __tpstrtab_##name, 0, reg, unreg, 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 +167,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