From e69abdfd4d92f4a360bf31bdd9d1951e8cb540a6 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 6 May 2021 11:48:42 -0400 Subject: [PATCH] Introduce struct lttng_kernel_tracepoint_class, enum probe_desc field Introduce struct lttng_kernel_tracepoint_class to clearly split the event instance from the class, thus allowing the event instance to refer to the class through a single symbol. This removes the need to rely on ARRAY_SIZE() to calculate the size of the event field array (part of the class) from within the event instance. This refactoring opens the door to have event class and instance not only in different providers, but also in providers emitted within different compile units and shared objects in the future. While refactoring kprobes, uprobes and kretprobes code, there is a lot of dynamically allocated memory which can be turned into static data structures for tp_class, fields and types. This simplification ends up fixing a few memory leaks as well. Introduce a probe_desc field in the enumeration descriptor to keep the same pattern for both tp_class and enumerations. Signed-off-by: Mathieu Desnoyers Change-Id: I221ad33383de48144d0d14b2b461ecb38dfb54ab --- include/lttng/events.h | 12 +++- include/lttng/tracepoint-event-impl.h | 95 +++++++++++++++------------ src/lttng-bytecode-specialize.c | 4 +- src/lttng-bytecode.c | 4 +- src/lttng-events.c | 12 ++-- src/probes/lttng-kprobes.c | 40 ++++------- src/probes/lttng-kretprobes.c | 59 ++++------------- src/probes/lttng-uprobes.c | 40 ++++------- 8 files changed, 110 insertions(+), 156 deletions(-) diff --git a/include/lttng/events.h b/include/lttng/events.h index a04d0f2a..09b4c672 100644 --- a/include/lttng/events.h +++ b/include/lttng/events.h @@ -130,6 +130,7 @@ struct lttng_kernel_enum_desc { const char *name; const struct lttng_kernel_enum_entry **entries; unsigned int nr_entries; + const struct lttng_kernel_probe_desc *probe_desc; }; /* Event field description */ @@ -302,13 +303,18 @@ struct lttng_kernel_probe_ctx { uint8_t interruptible; }; +struct lttng_kernel_tracepoint_class { + void (*probe_callback)(void); + const struct lttng_kernel_event_field **fields; /* event payload */ + unsigned int nr_fields; + const struct lttng_kernel_probe_desc *probe_desc; +}; + struct lttng_kernel_event_desc { const char *event_name; /* lttng-modules name */ const char *event_kname; /* Linux kernel name (tracepoints) */ const struct lttng_kernel_probe_desc *probe_desc; - void (*probe_callback)(void); - const struct lttng_kernel_event_field **fields; /* event payload */ - unsigned int nr_fields; + const struct lttng_kernel_tracepoint_class *tp_class; struct module *owner; }; diff --git a/include/lttng/tracepoint-event-impl.h b/include/lttng/tracepoint-event-impl.h index a1eff6c2..35b0059b 100644 --- a/include/lttng/tracepoint-event-impl.h +++ b/include/lttng/tracepoint-event-impl.h @@ -208,6 +208,42 @@ void __event_template_proto___##_name(void); #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) +/* + * Stage 1.3 of the trace events. + * + * Create probe callback prototypes. + */ + +/* Reset all macros within TRACEPOINT_EVENT */ +#include + +#undef TP_PROTO +#define TP_PROTO(...) __VA_ARGS__ + +#undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE +#define LTTNG_TRACEPOINT_EVENT_CLASS_CODE(_name, _proto, _args, _locvar, _code_pre, _fields, _code_post) \ +static void __event_probe__##_name(void *__data, _proto); + +#undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS +#define LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS(_name, _locvar, _code_pre, _fields, _code_post) \ +static void __event_probe__##_name(void *__data); + +#include TRACE_INCLUDE(TRACE_INCLUDE_FILE) + +/* + * Stage 1.4 of tracepoint event generation. + * + * Declare toplevel descriptor for the whole probe. + */ + +#define TP_ID1(_token, _system) _token##_system +#define TP_ID(_token, _system) TP_ID1(_token, _system) + +static __used struct lttng_kernel_probe_desc TP_ID(__probe_desc___, TRACE_SYSTEM); + +#undef TP_ID1 +#undef TP_ID + /* * Stage 2 of the trace events. * @@ -299,10 +335,20 @@ void __event_template_proto___##_name(void); #undef TP_FIELDS #define TP_FIELDS(...) __VA_ARGS__ /* Only one used in this phase */ +#ifndef TP_PROBE_CB +#define TP_PROBE_CB(_template) &__event_probe__##_template +#endif + #undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS #define LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS(_name, _locvar, _code_pre, _fields, _code_post) \ static const struct lttng_kernel_event_field *__event_fields___##_name[] = { \ _fields \ + }; \ + static const struct lttng_kernel_tracepoint_class lttng_kernel__event_class___##_name = { \ + .fields = __event_fields___##_name, \ + .nr_fields = ARRAY_SIZE(__event_fields___##_name), \ + .probe_callback = (void (*)(void)) TP_PROBE_CB(_name), \ + .probe_desc = &TP_ID(__probe_desc___, TRACE_SYSTEM), \ }; #undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE @@ -332,35 +378,20 @@ void __event_template_proto___##_name(void); .name = #_name, \ .entries = __enum_values__##_name, \ .nr_entries = ARRAY_SIZE(__enum_values__##_name), \ + .probe_desc = &TP_ID(__probe_desc___, TRACE_SYSTEM), \ })) +#define TP_ID1(_token, _system) _token##_system +#define TP_ID(_token, _system) TP_ID1(_token, _system) + #define LTTNG_CREATE_FIELD_METADATA #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) #undef LTTNG_CREATE_FIELD_METADATA #undef LTTNG_TRACEPOINT_TYPE_VISIBILITY -/* - * Stage 3 of the trace events. - * - * Create probe callback prototypes. - */ - -/* Reset all macros within TRACEPOINT_EVENT */ -#include - -#undef TP_PROTO -#define TP_PROTO(...) __VA_ARGS__ - -#undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE -#define LTTNG_TRACEPOINT_EVENT_CLASS_CODE(_name, _proto, _args, _locvar, _code_pre, _fields, _code_post) \ -static void __event_probe__##_name(void *__data, _proto); - -#undef LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS -#define LTTNG_TRACEPOINT_EVENT_CLASS_CODE_NOARGS(_name, _locvar, _code_pre, _fields, _code_post) \ -static void __event_probe__##_name(void *__data); - -#include TRACE_INCLUDE(TRACE_INCLUDE_FILE) +#undef TP_ID1 +#undef TP_ID /* * Stage 4 of the trace events. @@ -1163,20 +1194,6 @@ __post: \ #undef __get_dynamic_len -/* - * Stage 7.0 of tracepoint event generation. - * - * Declare toplevel descriptor for the whole probe. - */ - -#define TP_ID1(_token, _system) _token##_system -#define TP_ID(_token, _system) TP_ID1(_token, _system) - -static __used struct lttng_kernel_probe_desc TP_ID(__probe_desc___, TRACE_SYSTEM); - -#undef TP_ID1 -#undef TP_ID - /* * Stage 7.1 of the trace events. * @@ -1187,19 +1204,13 @@ static __used struct lttng_kernel_probe_desc TP_ID(__probe_desc___, TRACE_SYSTEM #include /* Reset all macros within LTTNG_TRACEPOINT_EVENT */ -#ifndef TP_PROBE_CB -#define TP_PROBE_CB(_template) &__event_probe__##_template -#endif - #undef LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP_NOARGS #define LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP_NOARGS(_template, _name, _map) \ static const struct lttng_kernel_event_desc __event_desc___##_map = { \ .event_name = #_map, \ .event_kname = #_name, \ + .tp_class = <tng_kernel__event_class___##_template, \ .probe_desc = &TP_ID(__probe_desc___, TRACE_SYSTEM), \ - .probe_callback = (void (*)(void)) TP_PROBE_CB(_template), \ - .fields = __event_fields___##_template, \ - .nr_fields = ARRAY_SIZE(__event_fields___##_template), \ .owner = THIS_MODULE, \ }; diff --git a/src/lttng-bytecode-specialize.c b/src/lttng-bytecode-specialize.c index 5c577548..c4b9d04b 100644 --- a/src/lttng-bytecode-specialize.c +++ b/src/lttng-bytecode-specialize.c @@ -454,11 +454,11 @@ static int specialize_payload_lookup(const struct lttng_kernel_event_desc *event struct bytecode_get_index_data gid; ssize_t data_offset; - nr_fields = event_desc->nr_fields; + nr_fields = event_desc->tp_class->nr_fields; offset = ((struct get_symbol *) insn->data)->offset; name = runtime->p.bc->bc.data + runtime->p.bc->bc.reloc_offset + offset; for (i = 0; i < nr_fields; i++) { - field = event_desc->fields[i]; + field = event_desc->tp_class->fields[i]; if (field->nofilter) { continue; } diff --git a/src/lttng-bytecode.c b/src/lttng-bytecode.c index fbd9147f..f094b009 100644 --- a/src/lttng-bytecode.c +++ b/src/lttng-bytecode.c @@ -185,10 +185,10 @@ int apply_field_reloc(const struct lttng_kernel_event_desc *event_desc, /* Lookup event by name */ if (!event_desc) return -EINVAL; - fields = event_desc->fields; + fields = event_desc->tp_class->fields; if (!fields) return -EINVAL; - nr_fields = event_desc->nr_fields; + nr_fields = event_desc->tp_class->nr_fields; for (i = 0; i < nr_fields; i++) { if (fields[i]->nofilter) continue; diff --git a/src/lttng-events.c b/src/lttng-events.c index 2c007ad4..78e50c87 100644 --- a/src/lttng-events.c +++ b/src/lttng-events.c @@ -1404,7 +1404,7 @@ void register_event(struct lttng_kernel_event_recorder *event_recorder) switch (event_recorder->priv->parent.instrumentation) { case LTTNG_KERNEL_ABI_TRACEPOINT: ret = lttng_wrapper_tracepoint_probe_register(desc->event_kname, - desc->probe_callback, + desc->tp_class->probe_callback, event_recorder); break; @@ -1443,7 +1443,7 @@ int _lttng_event_unregister(struct lttng_kernel_event_recorder *event_recorder) switch (event_priv->instrumentation) { case LTTNG_KERNEL_ABI_TRACEPOINT: ret = lttng_wrapper_tracepoint_probe_unregister(event_priv->desc->event_kname, - event_priv->desc->probe_callback, + event_priv->desc->tp_class->probe_callback, event_recorder); break; @@ -1493,7 +1493,7 @@ void register_event_notifier(struct lttng_kernel_event_notifier *event_notifier) switch (event_notifier->priv->parent.instrumentation) { case LTTNG_KERNEL_ABI_TRACEPOINT: ret = lttng_wrapper_tracepoint_probe_register(desc->event_kname, - desc->probe_callback, + desc->tp_class->probe_callback, event_notifier); break; @@ -1530,7 +1530,7 @@ int _lttng_event_notifier_unregister( switch (event_notifier->priv->parent.instrumentation) { case LTTNG_KERNEL_ABI_TRACEPOINT: ret = lttng_wrapper_tracepoint_probe_unregister(event_notifier->priv->parent.desc->event_kname, - event_notifier->priv->parent.desc->probe_callback, + event_notifier->priv->parent.desc->tp_class->probe_callback, event_notifier); break; @@ -3610,8 +3610,8 @@ int _lttng_fields_metadata_statedump(struct lttng_kernel_session *session, int ret = 0; int i; - for (i = 0; i < desc->nr_fields; i++) { - const struct lttng_kernel_event_field *field = desc->fields[i]; + for (i = 0; i < desc->tp_class->nr_fields; i++) { + const struct lttng_kernel_event_field *field = desc->tp_class->fields[i]; ret = _lttng_field_statedump(session, field, 2, &prev_field_name); if (ret) diff --git a/src/probes/lttng-kprobes.c b/src/probes/lttng-kprobes.c index 7c77c566..95f48540 100644 --- a/src/probes/lttng-kprobes.c +++ b/src/probes/lttng-kprobes.c @@ -89,8 +89,16 @@ int lttng_kprobes_event_handler_pre(struct kprobe *p, struct pt_regs *regs) return 0; } -static const struct lttng_kernel_type_common *event_type = - lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16); +static const struct lttng_kernel_event_field *event_fields[] = { + lttng_kernel_static_event_field("ip", + lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16), + false, false, false), +}; + +static const struct lttng_kernel_tracepoint_class tp_class = { + .nr_fields = ARRAY_SIZE(event_fields), + .fields = event_fields, +}; /* * Create event description @@ -98,43 +106,23 @@ static const struct lttng_kernel_type_common *event_type = static int lttng_create_kprobe_event(const char *name, struct lttng_kernel_event_recorder *event_recorder) { - const struct lttng_kernel_event_field **fieldp_array; - struct lttng_kernel_event_field *field; struct lttng_kernel_event_desc *desc; int ret; desc = kzalloc(sizeof(*desc), GFP_KERNEL); if (!desc) return -ENOMEM; + desc->tp_class = &tp_class; desc->event_name = kstrdup(name, GFP_KERNEL); if (!desc->event_name) { ret = -ENOMEM; goto error_str; } - desc->nr_fields = 1; - fieldp_array = kzalloc(1 * sizeof(struct lttng_kernel_event_field *), GFP_KERNEL); - if (!fieldp_array) { - ret = -ENOMEM; - goto error_fieldp_array; - } - desc->fields = fieldp_array; - desc->fields[0] = field = - kzalloc(sizeof(struct lttng_kernel_event_field), GFP_KERNEL); - if (!field) { - ret = -ENOMEM; - goto error_field; - } - field->name = "ip"; - field->type = event_type; desc->owner = THIS_MODULE; event_recorder->priv->parent.desc = desc; return 0; -error_field: - kfree(fieldp_array); -error_fieldp_array: - kfree(desc->event_name); error_str: kfree(desc); return ret; @@ -152,13 +140,12 @@ int lttng_create_kprobe_event_notifier(const char *name, struct lttng_kernel_eve desc = kzalloc(sizeof(*desc), GFP_KERNEL); if (!desc) return -ENOMEM; + desc->tp_class = &tp_class; desc->event_name = kstrdup(name, GFP_KERNEL); if (!desc->event_name) { ret = -ENOMEM; goto error_str; } - desc->nr_fields = 0; - desc->owner = THIS_MODULE; event_notifier->priv->parent.desc = desc; @@ -240,7 +227,6 @@ int lttng_kprobes_register_event(const char *name, return 0; register_error: - kfree(event_recorder->priv->parent.desc->fields); kfree(event_recorder->priv->parent.desc->event_name); kfree(event_recorder->priv->parent.desc); error: @@ -288,8 +274,6 @@ EXPORT_SYMBOL_GPL(lttng_kprobes_unregister_event_notifier); void lttng_kprobes_destroy_event_private(struct lttng_kernel_event_recorder *event_recorder) { kfree(event_recorder->priv->parent.u.kprobe.symbol_name); - kfree(event_recorder->priv->parent.desc->fields[0]); - kfree(event_recorder->priv->parent.desc->fields); kfree(event_recorder->priv->parent.desc->event_name); kfree(event_recorder->priv->parent.desc); } diff --git a/src/probes/lttng-kretprobes.c b/src/probes/lttng-kretprobes.c index 2677f8ab..7d0db416 100644 --- a/src/probes/lttng-kretprobes.c +++ b/src/probes/lttng-kretprobes.c @@ -112,8 +112,19 @@ int lttng_kretprobes_handler_exit(struct kretprobe_instance *krpi, return _lttng_kretprobes_handler(krpi, regs, EVENT_EXIT); } -static const struct lttng_kernel_type_common *event_type = - lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16); +static const struct lttng_kernel_event_field *event_fields[] = { + lttng_kernel_static_event_field("ip", + lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16), + false, false, false), + lttng_kernel_static_event_field("parent_ip", + lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16), + false, false, false), +}; + +static const struct lttng_kernel_tracepoint_class tp_class = { + .nr_fields = ARRAY_SIZE(event_fields), + .fields = event_fields, +}; /* * Create event description @@ -122,8 +133,6 @@ static int lttng_create_kprobe_event(const char *name, struct lttng_kernel_event_recorder *event_recorder, enum lttng_kretprobe_type type) { - const struct lttng_kernel_event_field **fieldp_array; - struct lttng_kernel_event_field *field; struct lttng_kernel_event_desc *desc; char *alloc_name; size_t name_len; @@ -151,43 +160,12 @@ int lttng_create_kprobe_event(const char *name, struct lttng_kernel_event_record strcpy(alloc_name, name); strcat(alloc_name, suffix); desc->event_name = alloc_name; - desc->nr_fields = 2; - fieldp_array = kzalloc(desc->nr_fields * sizeof(struct lttng_kernel_event_field *), GFP_KERNEL); - if (!fieldp_array) { - ret = -ENOMEM; - goto error_fieldp_array; - } - desc->fields = fieldp_array; - - field = kzalloc(sizeof(struct lttng_kernel_event_field), GFP_KERNEL); - if (!field) { - ret = -ENOMEM; - goto error_field0; - } - field->name = "ip"; - field->type = event_type; - desc->fields[0] = field; - - field = kzalloc(sizeof(struct lttng_kernel_event_field), GFP_KERNEL); - if (!field) { - ret = -ENOMEM; - goto error_field1; - } - field->name = "parent_ip"; - field->type = event_type; - desc->fields[1] = field; - + desc->tp_class = &tp_class; desc->owner = THIS_MODULE; event_recorder->priv->parent.desc = desc; return 0; -error_field1: - kfree(desc->fields[0]); -error_field0: - kfree(fieldp_array); -error_fieldp_array: - kfree(desc->event_name); error_str: kfree(desc); return ret; @@ -268,15 +246,9 @@ register_error: name_error: kfree(lttng_krp); krp_error: - kfree(event_recorder_exit->priv->parent.desc->fields[0]); - kfree(event_recorder_exit->priv->parent.desc->fields[1]); - kfree(event_recorder_exit->priv->parent.desc->fields); kfree(event_recorder_exit->priv->parent.desc->event_name); kfree(event_recorder_exit->priv->parent.desc); event_exit_error: - kfree(event_recorder_entry->priv->parent.desc->fields[0]); - kfree(event_recorder_entry->priv->parent.desc->fields[1]); - kfree(event_recorder_entry->priv->parent.desc->fields); kfree(event_recorder_entry->priv->parent.desc->event_name); kfree(event_recorder_entry->priv->parent.desc); error: @@ -309,9 +281,6 @@ void _lttng_kretprobes_release(struct kref *kref) void lttng_kretprobes_destroy_private(struct lttng_kernel_event_recorder *event_recorder) { - kfree(event_recorder->priv->parent.desc->fields[0]); - kfree(event_recorder->priv->parent.desc->fields[1]); - kfree(event_recorder->priv->parent.desc->fields); kfree(event_recorder->priv->parent.desc->event_name); kfree(event_recorder->priv->parent.desc); kref_put(&event_recorder->priv->parent.u.kretprobe.lttng_krp->kref_alloc, diff --git a/src/probes/lttng-uprobes.c b/src/probes/lttng-uprobes.c index 0240aaca..b88aee13 100644 --- a/src/probes/lttng-uprobes.c +++ b/src/probes/lttng-uprobes.c @@ -99,8 +99,16 @@ int lttng_uprobes_event_handler_pre(struct uprobe_consumer *uc, struct pt_regs * return 0; } -static const struct lttng_kernel_type_common *event_type = - lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16); +static const struct lttng_kernel_event_field *event_fields[] = { + lttng_kernel_static_event_field("ip", + lttng_kernel_static_type_integer_from_type(unsigned long, __BYTE_ORDER, 16), + false, false, false), +}; + +static const struct lttng_kernel_tracepoint_class tp_class = { + .nr_fields = ARRAY_SIZE(event_fields), + .fields = event_fields, +}; /* * Create event description. @@ -108,43 +116,23 @@ static const struct lttng_kernel_type_common *event_type = static int lttng_create_uprobe_event(const char *name, struct lttng_kernel_event_recorder *event_recorder) { - const struct lttng_kernel_event_field **fieldp_array; - struct lttng_kernel_event_field *field; struct lttng_kernel_event_desc *desc; int ret; desc = kzalloc(sizeof(*desc), GFP_KERNEL); if (!desc) return -ENOMEM; + desc->tp_class = &tp_class; desc->event_name = kstrdup(name, GFP_KERNEL); if (!desc->event_name) { ret = -ENOMEM; goto error_str; } - desc->nr_fields = 1; - fieldp_array = kzalloc(1 * sizeof(struct lttng_kernel_event_field *), GFP_KERNEL); - if (!fieldp_array) { - ret = -ENOMEM; - goto error_fieldp_array; - } - desc->fields = fieldp_array; - desc->fields[0] = field = - kzalloc(sizeof(struct lttng_kernel_event_field), GFP_KERNEL); - if (!field) { - ret = -ENOMEM; - goto error_field; - } - field->name = "ip"; - field->type = event_type; desc->owner = THIS_MODULE; event_recorder->priv->parent.desc = desc; return 0; -error_field: - kfree(fieldp_array); -error_fieldp_array: - kfree(desc->event_name); error_str: kfree(desc); return ret; @@ -162,14 +150,12 @@ int lttng_create_uprobe_event_notifier(const char *name, struct lttng_kernel_eve desc = kzalloc(sizeof(*desc), GFP_KERNEL); if (!desc) return -ENOMEM; + desc->tp_class = &tp_class; desc->event_name = kstrdup(name, GFP_KERNEL); if (!desc->event_name) { ret = -ENOMEM; goto error_str; } - - desc->nr_fields = 0; - desc->owner = THIS_MODULE; event_notifier->priv->parent.desc = desc; @@ -367,8 +353,6 @@ EXPORT_SYMBOL_GPL(lttng_uprobes_unregister_event_notifier); void lttng_uprobes_destroy_event_private(struct lttng_kernel_event_recorder *event_recorder) { iput(event_recorder->priv->parent.u.uprobe.inode); - kfree(event_recorder->priv->parent.desc->fields[0]); - kfree(event_recorder->priv->parent.desc->fields); kfree(event_recorder->priv->parent.desc->event_name); kfree(event_recorder->priv->parent.desc); } -- 2.34.1