[RFC patch 27/41] Markers use dynamic channels

Mathieu Desnoyers
2009-03-05T23:31:55Z

[-- Attachment #1: markers-use-dynamic-channels.patch --]
[-- Type: text/plain, Size: 22070 bytes --]

Make marker infrastructure use dynamic channels, adding a new (first) parameter
to trace_mark( : the channel name where the data must be sent.
Switch to per-channel marker IDs.
Marker IDs are now managed by marker infrastructure rather than LTTng.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
include/linux/marker.h | 101 +++++++++++++++++------------
kernel/marker.c | 168 +++++++++++++++++++++++++++++++++++++++----------
2 files changed, 195 insertions(+), 74 deletions(-)

Index: linux-2.6-lttng/include/linux/marker.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/marker.h 2009-02-06 15:44:53.000000000 -0500
+++ linux-2.6-lttng/include/linux/marker.h 2009-02-06 15:52:11.000000000 -0500
@@ -15,6 +15,7 @@
#include <stdarg.h>
#include <linux/types.h>
#include <linux/immediate.h>
+#include <linux/ltt-channels.h>

struct module;
struct marker;
@@ -40,6 +41,7 @@ struct marker_probe_closure {
};

struct marker {
+ const char *channel; /* Name of channel where to send data */
const char *name; /* Marker name */
const char *format; /* Marker format string, describing the
* variable argument list.
@@ -47,6 +49,8 @@ struct marker {
DEFINE_IMV(char, state);/* Immediate value state. */
char ptype; /* probe type : 0 : single, 1 : multi */
/* Probe wrapper */
+ u16 chan_id; /* Numeric channel identifier, dynamic */
+ u16 event_id; /* Numeric event identifier, dynamic */
void (*call)(const struct marker *mdata, void *call_private, ...);
struct marker_probe_closure single;
struct marker_probe_closure *multi;
@@ -56,21 +60,25 @@ struct marker {

#ifdef CONFIG_MARKERS

-#define _DEFINE_MARKER(name, tp_name_str, tp_cb, format) \
- static const char __mstrtab_##name[] \
+#define _DEFINE_MARKER(channel, name, tp_name_str, tp_cb, format) \
+ static const char __mstrtab_##channel##_##name[] \
__attribute__((section("__markers_strings"))) \
- = #name "\0" format; \
- static struct marker __mark_##name \
+ = #channel "\0" #name "\0" format; \
+ static struct marker __mark_##channel##_##name \
__attribute__((section("__markers"), aligned(8))) = \
- { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)], \
- 0, 0, marker_probe_cb, { __mark_empty_function, NULL},\
+ { __mstrtab_##channel##_##name, \
+ &__mstrtab_##channel##_##name[sizeof(#channel)], \
+ &__mstrtab_##channel##_##name[sizeof(#channel) + \
+ sizeof(#name)], \
+ 0, 0, 0, 0, marker_probe_cb, \
+ { __mark_empty_function, NULL}, \
NULL, tp_name_str, tp_cb }

-#define DEFINE_MARKER(name, format) \
- _DEFINE_MARKER(name, NULL, NULL, format)
+#define DEFINE_MARKER(channel, name, format) \
+ _DEFINE_MARKER(channel, name, NULL, NULL, format)

-#define DEFINE_MARKER_TP(name, tp_name, tp_cb, format) \
- _DEFINE_MARKER(name, #tp_name, tp_cb, format)
+#define DEFINE_MARKER_TP(channel, name, tp_name, tp_cb, format) \
+ _DEFINE_MARKER(channel, name, #tp_name, tp_cb, format)

/*
* Make sure the alignment of the structure in the __markers section will
@@ -81,45 +89,49 @@ struct marker {
* If generic is true, a variable read is used.
* If generic is false, immediate values are used.
*/
-#define __trace_mark(generic, name, call_private, format, args...) \
+#define __trace_mark(generic, channel, name, call_private, format, args...) \
do { \
- DEFINE_MARKER(name, format); \
+ DEFINE_MARKER(channel, name, format); \
__mark_check_format(format, ## args); \
if (!generic) { \
- if (unlikely(imv_read(__mark_##name.state))) \
- (*__mark_##name.call) \
- (&__mark_##name, call_private, \
- ## args); \
+ if (unlikely(imv_read( \
+ __mark_##channel##_##name.state))) \
+ (*__mark_##channel##_##name.call) \
+ (&__mark_##channel##_##name, \
+ call_private, ## args); \
} else { \
- if (unlikely(_imv_read(__mark_##name.state))) \
- (*__mark_##name.call) \
- (&__mark_##name, call_private, \
- ## args); \
+ if (unlikely(_imv_read( \
+ __mark_##channel##_##name.state))) \
+ (*__mark_##channel##_##name.call) \
+ (&__mark_##channel##_##name, \
+ call_private, ## args); \
} \
} while (0)

-#define __trace_mark_tp(name, call_private, tp_name, tp_cb, format, args...) \
+#define __trace_mark_tp(channel, name, call_private, tp_name, tp_cb, \
+ format, args...) \
do { \
void __check_tp_type(void) \
{ \
register_trace_##tp_name(tp_cb); \
} \
- DEFINE_MARKER_TP(name, tp_name, tp_cb, format); \
+ DEFINE_MARKER_TP(channel, name, tp_name, tp_cb, format);\
__mark_check_format(format, ## args); \
- (*__mark_##name.call)(&__mark_##name, call_private, \
- ## args); \
+ (*__mark_##channel##_##name.call)(&__mark_##channel##_##name, \
+ call_private, ## args); \
} while (0)

extern void marker_update_probe_range(struct marker *begin,
struct marker *end);

-#define GET_MARKER(name) (__mark_##name)
+#define GET_MARKER(channel, name) (__mark_##channel##_##name)

#else /* !CONFIG_MARKERS */
-#define DEFINE_MARKER(name, tp_name, tp_cb, format)
-#define __trace_mark(generic, name, call_private, format, args...) \
+#define DEFINE_MARKER(channel, name, tp_name, tp_cb, format)
+#define __trace_mark(generic, channel, name, call_private, format, args...) \
__mark_check_format(format, ## args)
-#define __trace_mark_tp(name, call_private, tp_name, tp_cb, format, args...) \
+#define __trace_mark_tp(channel, name, call_private, tp_name, tp_cb, \
+ format, args...) \
do { \
void __check_tp_type(void) \
{ \
@@ -130,11 +142,12 @@ extern void marker_update_probe_range(st
static inline void marker_update_probe_range(struct marker *begin,
struct marker *end)
{ }
-#define GET_MARKER(name)
+#define GET_MARKER(channel, name)
#endif /* CONFIG_MARKERS */

/**
* trace_mark - Marker using code patching
+ * @channel: marker channel (where to send the data), not quoted.
* @name: marker name, not quoted.
* @format: format string
* @args...: variable argument list
@@ -142,11 +155,12 @@ static inline void marker_update_probe_r
* Places a marker using optimized code patching technique (imv_read())
* to be enabled when immediate values are present.
*/
-#define trace_mark(name, format, args...) \
- __trace_mark(0, name, NULL, format, ## args)
+#define trace_mark(channel, name, format, args...) \
+ __trace_mark(0, channel, name, NULL, format, ## args)

/**
* _trace_mark - Marker using variable read
+ * @channel: marker channel (where to send the data), not quoted.
* @name: marker name, not quoted.
* @format: format string
* @args...: variable argument list
@@ -156,11 +170,12 @@ static inline void marker_update_probe_r
* modification based enabling is not welcome. (__init and __exit functions,
* lockdep, some traps, printk).
*/
-#define _trace_mark(name, format, args...) \
- __trace_mark(1, name, NULL, format, ## args)
+#define _trace_mark(channel, name, format, args...) \
+ __trace_mark(1, channel, name, NULL, format, ## args)

/**
* trace_mark_tp - Marker in a tracepoint callback
+ * @channel: marker channel (where to send the data), not quoted.
* @name: marker name, not quoted.
* @tp_name: tracepoint name, not quoted.
* @tp_cb: tracepoint callback. Should have an associated global symbol so it
@@ -170,14 +185,19 @@ static inline void marker_update_probe_r
*
* Places a marker in a tracepoint callback.
*/
-#define trace_mark_tp(name, tp_name, tp_cb, format, args...) \
- __trace_mark_tp(name, NULL, tp_name, tp_cb, format, ## args)
+#define trace_mark_tp(channel, name, tp_name, tp_cb, format, args...) \
+ __trace_mark_tp(channel, name, NULL, tp_name, tp_cb, format, ## args)

/**
* MARK_NOARGS - Format string for a marker with no argument.
*/
#define MARK_NOARGS " "

+extern void lock_markers(void);
+extern void unlock_markers(void);
+
+extern void markers_compact_event_ids(void);
+
/* To be used for string format validity checking with gcc */
static inline void __printf(1, 2) ___mark_check_format(const char *fmt, ...)
{
@@ -198,13 +218,13 @@ extern void marker_probe_cb(const struct
* Connect a probe to a marker.
* private data pointer must be a valid allocated memory address, or NULL.
*/
-extern int marker_probe_register(const char *name, const char *format,
- marker_probe_func *probe, void *probe_private);
+extern int marker_probe_register(const char *channel, const char *name,
+ const char *format, marker_probe_func *probe, void *probe_private);

/*
* Returns the private data given to marker_probe_register.
*/
-extern int marker_probe_unregister(const char *name,
+extern int marker_probe_unregister(const char *channel, const char *name,
marker_probe_func *probe, void *probe_private);
/*
* Unregister a marker by providing the registered private data.
@@ -212,8 +232,8 @@ extern int marker_probe_unregister(const
extern int marker_probe_unregister_private_data(marker_probe_func *probe,
void *probe_private);

-extern void *marker_get_private_data(const char *name, marker_probe_func *probe,
- int num);
+extern void *marker_get_private_data(const char *channel, const char *name,
+ marker_probe_func *probe, int num);

/*
* marker_synchronize_unregister must be called between the last marker probe
@@ -235,5 +255,6 @@ extern void marker_iter_stop(struct mark
extern void marker_iter_reset(struct marker_iter *iter);
extern int marker_get_iter_range(struct marker **marker, struct marker *begin,
struct marker *end);
+extern int is_marker_enabled(const char *channel, const char *name);

#endif
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
--- linux-2.6-lttng.orig/kernel/marker.c 2009-02-06 15:44:53.000000000 -0500
+++ linux-2.6-lttng/kernel/marker.c 2009-02-06 15:52:18.000000000 -0500
@@ -38,6 +38,16 @@ static const int marker_debug;
*/
static DEFINE_MUTEX(markers_mutex);

+void lock_markers(void)
+{
+ mutex_lock(&markers_mutex);
+}
+
+void unlock_markers(void)
+{
+ mutex_unlock(&markers_mutex);
+}
+
/*
* Marker hash table, containing the active markers.
* Protected by module_mutex.
@@ -57,6 +67,7 @@ static struct hlist_head marker_table[MA
struct marker_entry {
struct hlist_node hlist;
char *format;
+ char *name;
/* Probe wrapper */
void (*call)(const struct marker *mdata, void *call_private, ...);
struct marker_probe_closure single;
@@ -65,9 +76,11 @@ struct marker_entry {
struct rcu_head rcu;
void *oldptr;
int rcu_pending;
+ u16 chan_id;
+ u16 event_id;
unsigned char ptype:1;
unsigned char format_allocated:1;
- char name[0]; /* Contains name'\0'format'\0' */
+ char channel[0]; /* Contains channel'\0'name'\0'format'\0' */
};

/**
@@ -205,6 +218,13 @@ static void free_old_closure(struct rcu_
{
struct marker_entry *entry = container_of(head,
struct marker_entry, rcu);
+ int ret;
+
+ /* Single probe removed */
+ if (!entry->ptype) {
+ ret = ltt_channels_unregister(entry->channel);
+ WARN_ON(ret);
+ }
kfree(entry->oldptr);
/* Make sure we free the data before setting the pending flag to 0 */
smp_wmb();
@@ -354,16 +374,19 @@ marker_entry_remove_probe(struct marker_
* Must be called with markers_mutex held.
* Returns NULL if not present.
*/
-static struct marker_entry *get_marker(const char *name)
+static struct marker_entry *get_marker(const char *channel, const char *name)
{
struct hlist_head *head;
struct hlist_node *node;
struct marker_entry *e;
- u32 hash = jhash(name, strlen(name), 0);
+ size_t channel_len = strlen(channel) + 1;
+ size_t name_len = strlen(name) + 1;
+ u32 hash;

+ hash = jhash(channel, channel_len-1, 0) ^ jhash(name, name_len-1, 0);
head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
hlist_for_each_entry(e, node, head, hlist) {
- if (!strcmp(name, e->name))
+ if (!strcmp(channel, e->channel) && !strcmp(name, e->name))
return e;
}
return NULL;
@@ -373,22 +396,25 @@ static struct marker_entry *get_marker(c
* Add the marker to the marker hash table. Must be called with markers_mutex
* held.
*/
-static struct marker_entry *add_marker(const char *name, const char *format)
+static struct marker_entry *add_marker(const char *channel, const char *name,
+ const char *format)
{
struct hlist_head *head;
struct hlist_node *node;
struct marker_entry *e;
+ size_t channel_len = strlen(channel) + 1;
size_t name_len = strlen(name) + 1;
size_t format_len = 0;
- u32 hash = jhash(name, name_len-1, 0);
+ u32 hash;

+ hash = jhash(channel, channel_len-1, 0) ^ jhash(name, name_len-1, 0);
if (format)
format_len = strlen(format) + 1;
head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
hlist_for_each_entry(e, node, head, hlist) {
- if (!strcmp(name, e->name)) {
+ if (!strcmp(channel, e->channel) && !strcmp(name, e->name)) {
printk(KERN_NOTICE
- "Marker %s busy\n", name);
+ "Marker %s.%s busy\n", channel, name);
return ERR_PTR(-EBUSY); /* Already there */
}
}
@@ -396,13 +422,16 @@ static struct marker_entry *add_marker(c
* Using kmalloc here to allocate a variable length element. Could
* cause some memory fragmentation if overused.
*/
- e = kmalloc(sizeof(struct marker_entry) + name_len + format_len,
- GFP_KERNEL);
+ e = kmalloc(sizeof(struct marker_entry)
+ + channel_len + name_len + format_len,
+ GFP_KERNEL);
if (!e)
return ERR_PTR(-ENOMEM);
- memcpy(&e->name[0], name, name_len);
+ memcpy(e->channel, channel, channel_len);
+ e->name = &e->channel[channel_len];
+ memcpy(e->name, name, name_len);
if (format) {
- e->format = &e->name[name_len];
+ e->format = &e->name[channel_len + name_len];
memcpy(e->format, format, format_len);
if (strcmp(e->format, MARK_NOARGS) == 0)
e->call = marker_probe_cb_noarg;
@@ -435,12 +464,14 @@ static int remove_marker(const char *nam
struct hlist_node *node;
struct marker_entry *e;
int found = 0;
- size_t len = strlen(name) + 1;
- u32 hash = jhash(name, len-1, 0);
+ size_t channel_len = strlen(channel) + 1;
+ size_t name_len = strlen(name) + 1;
+ u32 hash;

+ hash = jhash(channel, channel_len-1, 0) ^ jhash(name, name_len-1, 0);
head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
hlist_for_each_entry(e, node, head, hlist) {
- if (!strcmp(name, e->name)) {
+ if (!strcmp(channel, e->channel) && !strcmp(name, e->name)) {
found = 1;
break;
}
@@ -665,6 +696,7 @@ static void marker_update_probes(void)

/**
* marker_probe_register - Connect a probe to a marker
+ * @channel: marker channel
* @name: marker name
* @format: format string
* @probe: probe handler
@@ -674,27 +706,43 @@ static void marker_update_probes(void)
* Returns 0 if ok, error value on error.
* The probe address must at least be aligned on the architecture pointer size.
*/
-int marker_probe_register(const char *name, const char *format,
- marker_probe_func *probe, void *probe_private)
+int marker_probe_register(const char *channel, const char *name,
+ const char *format, marker_probe_func *probe,
+ void *probe_private)
{
struct marker_entry *entry;
- int ret = 0;
+ int ret = 0, ret_err;
struct marker_probe_closure *old;
+ int first_probe = 0;

mutex_lock(&markers_mutex);
entry = get_marker(name);
if (!entry) {
- entry = add_marker(name, format);
+ first_probe = 1;
+ entry = add_marker(channel, name, format);
if (IS_ERR(entry))
ret = PTR_ERR(entry);
+ if (ret)
+ goto end;
+ ret = ltt_channels_register(channel);
+ if (ret)
+ goto error_remove_marker;
+ ret = ltt_channels_get_index_from_name(channel);
+ if (ret < 0)
+ goto error_unregister_channel;
+ entry->channel_id = ret;
+ ret = ltt_channels_get_event_id(channel);
+ if (ret < 0)
+ goto error_unregister_channel;
+ entry->event_id = ret;
} else if (format) {
if (!entry->format)
ret = marker_set_format(entry, format);
else if (strcmp(entry->format, format))
ret = -EPERM;
+ if (ret)
+ goto end;
}
- if (ret)
- goto end;

/*
* If we detect that a call_rcu is pending for this marker,
@@ -705,12 +753,17 @@ int marker_probe_register(const char *na
old = marker_entry_add_probe(entry, probe, probe_private);
if (IS_ERR(old)) {
ret = PTR_ERR(old);
- goto end;
+ if (first_probe)
+ goto error_unregister_channel;
+ else
+ goto end;
}
mutex_unlock(&markers_mutex);
+
marker_update_probes();
+
mutex_lock(&markers_mutex);
- entry = get_marker(name);
+ entry = get_marker(channel, name);
if (!entry)
goto end;
if (entry->rcu_pending)
@@ -720,6 +773,13 @@ int marker_probe_register(const char *na
/* write rcu_pending before calling the RCU callback */
smp_wmb();
call_rcu_sched(&entry->rcu, free_old_closure);
+
+error_unregister_channel:
+ ret_err = ltt_channels_unregister(channel);
+ WARN_ON(ret_err);
+error_remove_marker:
+ ret_err = remove_marker(channel, name);
+ WARN_ON(ret_err);
end:
mutex_unlock(&markers_mutex);
return ret;
@@ -728,6 +788,7 @@ EXPORT_SYMBOL_GPL(marker_probe_register)

/**
* marker_probe_unregister - Disconnect a probe from a marker
+ * @channel: marker channel
* @name: marker name
* @probe: probe function pointer
* @probe_private: probe private data
@@ -738,24 +799,26 @@ EXPORT_SYMBOL_GPL(marker_probe_register)
* itself uses stop_machine(), which insures that every preempt disabled section
* have finished.
*/
-int marker_probe_unregister(const char *name,
- marker_probe_func *probe, void *probe_private)
+int marker_probe_unregister(const char *channel, const char *name,
+ marker_probe_func *probe, void *probe_private)
{
struct marker_entry *entry;
struct marker_probe_closure *old;
int ret = -ENOENT;

mutex_lock(&markers_mutex);
- entry = get_marker(name);
+ entry = get_marker(channel, name);
if (!entry)
goto end;
if (entry->rcu_pending)
rcu_barrier_sched();
old = marker_entry_remove_probe(entry, probe, probe_private);
mutex_unlock(&markers_mutex);
+
marker_update_probes();
+
mutex_lock(&markers_mutex);
- entry = get_marker(name);
+ entry = get_marker(channel, name);
if (!entry)
goto end;
if (entry->rcu_pending)
@@ -765,7 +828,7 @@ int marker_probe_unregister(const char *
/* write rcu_pending before calling the RCU callback */
smp_wmb();
call_rcu_sched(&entry->rcu, free_old_closure);
- remove_marker(name); /* Ignore busy error message */
+ remove_marker(channel, name); /* Ignore busy error message */
ret = 0;
end:
mutex_unlock(&markers_mutex);
@@ -823,6 +886,7 @@ int marker_probe_unregister_private_data
struct marker_entry *entry;
int ret = 0;
struct marker_probe_closure *old;
+ const char *channel = NULL, *name = NULL;

mutex_lock(&markers_mutex);
entry = get_marker_from_private_data(probe, probe_private);
@@ -833,10 +897,14 @@ int marker_probe_unregister_private_data
if (entry->rcu_pending)
rcu_barrier_sched();
old = marker_entry_remove_probe(entry, NULL, probe_private);
+ channel = kstrdup(entry->channel, GFP_KERNEL);
+ name = kstrdup(entry->name, GFP_KERNEL);
mutex_unlock(&markers_mutex);
+
marker_update_probes();
+
mutex_lock(&markers_mutex);
- entry = get_marker_from_private_data(probe, probe_private);
+ entry = get_marker(channel, name);
if (!entry)
goto end;
if (entry->rcu_pending)
@@ -846,15 +914,19 @@ int marker_probe_unregister_private_data
/* write rcu_pending before calling the RCU callback */
smp_wmb();
call_rcu_sched(&entry->rcu, free_old_closure);
- remove_marker(entry->name); /* Ignore busy error message */
+ /* Ignore busy error message */
+ remove_marker(channel, name);
end:
mutex_unlock(&markers_mutex);
+ kfree(channel);
+ kfree(name);
return ret;
}
EXPORT_SYMBOL_GPL(marker_probe_unregister_private_data);

/**
* marker_get_private_data - Get a marker's probe private data
+ * @channel: marker channel
* @name: marker name
* @probe: probe to match
* @num: get the nth matching probe's private data
@@ -866,19 +938,21 @@ EXPORT_SYMBOL_GPL(marker_probe_unregiste
* owner of the data, or its content could vanish. This is mostly used to
* confirm that a caller is the owner of a registered probe.
*/
-void *marker_get_private_data(const char *name, marker_probe_func *probe,
- int num)
+void *marker_get_private_data(const char *channel, const char *name,
+ marker_probe_func *probe, int num)
{
struct hlist_head *head;
struct hlist_node *node;
struct marker_entry *e;
+ size_t channel_len = strlen(channel) + 1;
size_t name_len = strlen(name) + 1;
- u32 hash = jhash(name, name_len-1, 0);
int i;
+ u32 hash;

+ hash = jhash(channel, channel_len-1, 0) ^ jhash(name, name_len-1, 0);
head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
hlist_for_each_entry(e, node, head, hlist) {
- if (!strcmp(name, e->name)) {
+ if (!strcmp(channel, e->channel) && !strcmp(name, e->name)) {
if (!e->ptype) {
if (num == 0 && e->single.func == probe)
return e->single.probe_private;
@@ -900,6 +974,32 @@ void *marker_get_private_data(const char
}
EXPORT_SYMBOL_GPL(marker_get_private_data);

+/**
+ * markers_compact_event_ids - Compact markers event IDs and reassign channels
+ *
+ * Called when no channel users are active by the channel infrastructure.
+ * Called with lock_markers() held.
+ */
+void markers_compact_event_ids(void)
+{
+ struct marker_entry *entry;
+ unsigned int i;
+ struct hlist_head *head;
+ struct hlist_node *node;
+
+ for (i = 0; i < MARKER_TABLE_SIZE; i++) {
+ head = &marker_table[i];
+ hlist_for_each_entry(entry, node, head, hlist) {
+ ret = ltt_channels_get_index_from_name(entry->channel);
+ WARN_ON(ret < 0);
+ entry->channel_id = ret;
+ ret = ltt_channels_get_event_id(entry->channel);
+ WARN_ON(ret < 0);
+ entry->event_id = ret;
+ }
+ }
+}
+
#ifdef CONFIG_MODULES

/**