From 853aee818b0f82ff37a7af159b6fa657ec3bcd29 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 2 Mar 2020 11:26:39 -0500 Subject: [PATCH] Remove lttng-ftrace integration The lttng-ftrace integration (LTTNG_KERNEL_FUNCTION instrumentation type) was unused for a while now. The "function" probing is actually done with kprobes and kretprobes (LTTNG_KERNEL_KPROBE and LTTNG_KERNEL_KRETPROBE). Remove it so a use of kallsyms_lookup_name() can be removed as well. Note that in the future we could add back this support by using register_ftrace_function() which is exported to kernel modules, but considering that we have not been using this code for a while, just remove the implementation for now. Signed-off-by: Mathieu Desnoyers --- Kbuild.common | 21 ---- lttng-abi.c | 8 +- lttng-events.c | 40 ++----- lttng-events.h | 29 ------ probes/Kbuild | 8 -- probes/lttng-ftrace.c | 236 ------------------------------------------ wrapper/ftrace.h | 75 -------------- 7 files changed, 11 insertions(+), 406 deletions(-) delete mode 100644 probes/lttng-ftrace.c delete mode 100644 wrapper/ftrace.h diff --git a/Kbuild.common b/Kbuild.common index c7cbd0b2..dde47a95 100644 --- a/Kbuild.common +++ b/Kbuild.common @@ -50,25 +50,4 @@ endif EXTRA_VERSION_PATCHES:=$(shell $(TOP_LTTNG_MODULES_DIR)/scripts/extra-version-patches.sh $(TOP_LTTNG_MODULES_DIR)) -# Starting with kernel 4.12, the ftrace header was moved to private headers -# and as such is not available when building against distro headers instead -# of the full kernel sources. In the situation, define LTTNG_FTRACE_MISSING_HEADER -# so we can enable the compat code even if CONFIG_DYNAMIC_FTRACE is enabled. -ifneq ($(CONFIG_DYNAMIC_FTRACE),) - ftrace_dep = $(srctree)/kernel/trace/trace.h - ftrace_dep_check = $(wildcard $(ftrace_dep)) - have_ftrace_header = $(shell \ - if [ $(VERSION) -ge 5 -o \( $(VERSION) -eq 4 -a $(PATCHLEVEL) -ge 12 \) ] ; then \ - if [ -z "$(ftrace_dep_check)" ] ; then \ - echo "no" ; \ - exit ; \ - fi; \ - fi; \ - echo "yes" ; \ - ) - ifeq ($(have_ftrace_header), no) - ccflags-y += -DLTTNG_FTRACE_MISSING_HEADER - endif -endif - # vim:syntax=make diff --git a/lttng-abi.c b/lttng-abi.c index 058c022e..2c6d6623 100644 --- a/lttng-abi.c +++ b/lttng-abi.c @@ -1115,7 +1115,8 @@ int lttng_abi_create_event(struct file *channel_file, event_param->u.kprobe.symbol_name[LTTNG_KERNEL_SYM_NAME_LEN - 1] = '\0'; break; case LTTNG_KERNEL_FUNCTION: - event_param->u.ftrace.symbol_name[LTTNG_KERNEL_SYM_NAME_LEN - 1] = '\0'; + WARN_ON_ONCE(1); + /* Not implemented. */ break; default: break; @@ -1266,9 +1267,8 @@ long lttng_channel_ioctl(struct file *file, unsigned int cmd, unsigned long arg) sizeof(uevent_param->u.kretprobe.symbol_name)); break; case LTTNG_KERNEL_FUNCTION: - memcpy(uevent_param->u.ftrace.symbol_name, - old_uevent_param->u.ftrace.symbol_name, - sizeof(uevent_param->u.ftrace.symbol_name)); + WARN_ON_ONCE(1); + /* Not implemented. */ break; default: break; diff --git a/lttng-events.c b/lttng-events.c index 1308d949..831f0aef 100644 --- a/lttng-events.c +++ b/lttng-events.c @@ -400,7 +400,6 @@ int lttng_event_enable(struct lttng_event *event) ret = -EINVAL; break; case LTTNG_KERNEL_KPROBE: - case LTTNG_KERNEL_FUNCTION: case LTTNG_KERNEL_UPROBE: case LTTNG_KERNEL_NOOP: WRITE_ONCE(event->enabled, 1); @@ -408,6 +407,7 @@ int lttng_event_enable(struct lttng_event *event) case LTTNG_KERNEL_KRETPROBE: ret = lttng_kretprobes_event_enable_state(event, 1); break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -436,7 +436,6 @@ int lttng_event_disable(struct lttng_event *event) ret = -EINVAL; break; case LTTNG_KERNEL_KPROBE: - case LTTNG_KERNEL_FUNCTION: case LTTNG_KERNEL_UPROBE: case LTTNG_KERNEL_NOOP: WRITE_ONCE(event->enabled, 0); @@ -444,6 +443,7 @@ int lttng_event_disable(struct lttng_event *event) case LTTNG_KERNEL_KRETPROBE: ret = lttng_kretprobes_event_enable_state(event, 0); break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -586,11 +586,11 @@ struct lttng_event *_lttng_event_create(struct lttng_channel *chan, case LTTNG_KERNEL_KPROBE: case LTTNG_KERNEL_UPROBE: case LTTNG_KERNEL_KRETPROBE: - case LTTNG_KERNEL_FUNCTION: case LTTNG_KERNEL_NOOP: case LTTNG_KERNEL_SYSCALL: event_name = event_param->name; break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -713,27 +713,6 @@ struct lttng_event *_lttng_event_create(struct lttng_channel *chan, list_add(&event_return->list, &chan->session->events); break; } - case LTTNG_KERNEL_FUNCTION: - /* - * Needs to be explicitly enabled after creation, since - * we may want to apply filters. - */ - event->enabled = 0; - event->registered = 1; - /* - * Populate lttng_event structure before event - * registration. - */ - smp_wmb(); - ret = lttng_ftrace_register(event_name, - event_param->u.ftrace.symbol_name, - event); - if (ret) { - goto register_error; - } - ret = try_module_get(event->desc->owner); - WARN_ON_ONCE(!ret); - break; case LTTNG_KERNEL_NOOP: case LTTNG_KERNEL_SYSCALL: /* @@ -770,6 +749,7 @@ struct lttng_event *_lttng_event_create(struct lttng_channel *chan, ret = try_module_get(event->desc->owner); WARN_ON_ONCE(!ret); break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through */ default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -834,10 +814,10 @@ void register_event(struct lttng_event *event) case LTTNG_KERNEL_KPROBE: case LTTNG_KERNEL_UPROBE: case LTTNG_KERNEL_KRETPROBE: - case LTTNG_KERNEL_FUNCTION: case LTTNG_KERNEL_NOOP: ret = 0; break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through */ default: WARN_ON_ONCE(1); } @@ -871,10 +851,6 @@ int _lttng_event_unregister(struct lttng_event *event) lttng_kretprobes_unregister(event); ret = 0; break; - case LTTNG_KERNEL_FUNCTION: - lttng_ftrace_unregister(event); - ret = 0; - break; case LTTNG_KERNEL_SYSCALL: ret = lttng_syscall_filter_disable(event->chan, desc->name); @@ -886,6 +862,7 @@ int _lttng_event_unregister(struct lttng_event *event) lttng_uprobes_unregister(event); ret = 0; break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through */ default: WARN_ON_ONCE(1); } @@ -912,10 +889,6 @@ void _lttng_event_destroy(struct lttng_event *event) module_put(event->desc->owner); lttng_kretprobes_destroy_private(event); break; - case LTTNG_KERNEL_FUNCTION: - module_put(event->desc->owner); - lttng_ftrace_destroy_private(event); - break; case LTTNG_KERNEL_NOOP: case LTTNG_KERNEL_SYSCALL: break; @@ -923,6 +896,7 @@ void _lttng_event_destroy(struct lttng_event *event) module_put(event->desc->owner); lttng_uprobes_destroy_private(event); break; + case LTTNG_KERNEL_FUNCTION: /* Fall-through */ default: WARN_ON_ONCE(1); } diff --git a/lttng-events.h b/lttng-events.h index 93a10fed..470df254 100644 --- a/lttng-events.h +++ b/lttng-events.h @@ -314,9 +314,6 @@ struct lttng_event { struct lttng_krp *lttng_krp; char *symbol_name; } kretprobe; - struct { - char *symbol_name; - } ftrace; struct { struct inode *inode; struct list_head head; @@ -862,32 +859,6 @@ int lttng_kretprobes_event_enable_state(struct lttng_event *event, } #endif -#if defined(CONFIG_DYNAMIC_FTRACE) && !defined(LTTNG_FTRACE_MISSING_HEADER) -int lttng_ftrace_register(const char *name, - const char *symbol_name, - struct lttng_event *event); -void lttng_ftrace_unregister(struct lttng_event *event); -void lttng_ftrace_destroy_private(struct lttng_event *event); -#else -static inline -int lttng_ftrace_register(const char *name, - const char *symbol_name, - struct lttng_event *event) -{ - return -ENOSYS; -} - -static inline -void lttng_ftrace_unregister(struct lttng_event *event) -{ -} - -static inline -void lttng_ftrace_destroy_private(struct lttng_event *event) -{ -} -#endif - int lttng_calibrate(struct lttng_kernel_calibrate *calibrate); extern const struct file_operations lttng_tracepoint_list_fops; diff --git a/probes/Kbuild b/probes/Kbuild index 0b1e016f..ceef8995 100644 --- a/probes/Kbuild +++ b/probes/Kbuild @@ -271,14 +271,6 @@ ifneq ($(CONFIG_KRETPROBES),) obj-$(CONFIG_LTTNG) += lttng-kretprobes.o endif # CONFIG_KRETPROBES -ifneq ($(CONFIG_DYNAMIC_FTRACE),) - ifeq ($(have_ftrace_header),yes) - obj-$(CONFIG_LTTNG) += lttng-ftrace.o - else - $(warning Files $(ftrace_dep) not found. Probe "ftrace" is disabled. Use full kernel source tree to enable it.) - endif -endif # CONFIG_DYNAMIC_FTRACE - ifneq ($(CONFIG_PREEMPTIRQ_EVENTS),) obj-$(CONFIG_LTTNG) += lttng-probe-preemptirq.o endif # CONFIG_PREEMPTIRQ_EVENTS diff --git a/probes/lttng-ftrace.c b/probes/lttng-ftrace.c deleted file mode 100644 index fe1bfd66..00000000 --- a/probes/lttng-ftrace.c +++ /dev/null @@ -1,236 +0,0 @@ -/* SPDX-License-Identifier: (GPL-2.0 or LGPL-2.1) - * - * probes/lttng-ftrace.c - * - * LTTng function tracer integration module. - * - * Copyright (C) 2009-2012 Mathieu Desnoyers - */ - -/* - * Ftrace function tracer does not seem to provide synchronization between probe - * teardown and callback execution. Therefore, we make this module permanently - * loaded (unloadable). - * - * TODO: Move to register_ftrace_function() (which is exported for - * modules) for Linux >= 3.0. It is faster (only enables the selected - * functions), and will stay there. - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,12,0)) -static -void lttng_ftrace_handler(unsigned long ip, unsigned long parent_ip, - struct trace_array *tr, struct ftrace_probe_ops *ops, - void *data) -{ - struct lttng_event *event = data; - struct lttng_probe_ctx lttng_probe_ctx = { - .event = event, - .interruptible = !irqs_disabled(), - }; - struct lttng_channel *chan = event->chan; - struct lib_ring_buffer_ctx ctx; - struct { - unsigned long ip; - unsigned long parent_ip; - } payload; - int ret; - - if (unlikely(!READ_ONCE(chan->session->active))) - return; - if (unlikely(!READ_ONCE(chan->enabled))) - return; - if (unlikely(!READ_ONCE(event->enabled))) - return; - - lib_ring_buffer_ctx_init(&ctx, chan->chan, <tng_probe_ctx, - sizeof(payload), lttng_alignof(payload), -1); - ret = chan->ops->event_reserve(&ctx, event->id); - if (ret < 0) - return; - payload.ip = ip; - payload.parent_ip = parent_ip; - lib_ring_buffer_align_ctx(&ctx, lttng_alignof(payload)); - chan->ops->event_write(&ctx, &payload, sizeof(payload)); - chan->ops->event_commit(&ctx); - return; -} -#else -static -void lttng_ftrace_handler(unsigned long ip, unsigned long parent_ip, void **data) -{ - struct lttng_event *event = *data; - struct lttng_probe_ctx lttng_probe_ctx = { - .event = event, - .interruptible = !irqs_disabled(), - }; - struct lttng_channel *chan = event->chan; - struct lib_ring_buffer_ctx ctx; - struct { - unsigned long ip; - unsigned long parent_ip; - } payload; - int ret; - - if (unlikely(!READ_ONCE(chan->session->active))) - return; - if (unlikely(!READ_ONCE(chan->enabled))) - return; - if (unlikely(!READ_ONCE(event->enabled))) - return; - - lib_ring_buffer_ctx_init(&ctx, chan->chan, <tng_probe_ctx, - sizeof(payload), lttng_alignof(payload), -1); - ret = chan->ops->event_reserve(&ctx, event->id); - if (ret < 0) - return; - payload.ip = ip; - payload.parent_ip = parent_ip; - lib_ring_buffer_align_ctx(&ctx, lttng_alignof(payload)); - chan->ops->event_write(&ctx, &payload, sizeof(payload)); - chan->ops->event_commit(&ctx); - return; -} -#endif - -/* - * Create event description - */ -static -int lttng_create_ftrace_event(const char *name, struct lttng_event *event) -{ - struct lttng_event_field *fields; - struct lttng_event_desc *desc; - int ret; - - desc = kzalloc(sizeof(*event->desc), GFP_KERNEL); - if (!desc) - return -ENOMEM; - desc->name = kstrdup(name, GFP_KERNEL); - if (!desc->name) { - ret = -ENOMEM; - goto error_str; - } - desc->nr_fields = 2; - desc->fields = fields = - kzalloc(2 * sizeof(struct lttng_event_field), GFP_KERNEL); - if (!desc->fields) { - ret = -ENOMEM; - goto error_fields; - } - fields[0].name = "ip"; - fields[0].type.atype = atype_integer; - fields[0].type.u.basic.integer.size = sizeof(unsigned long) * CHAR_BIT; - fields[0].type.u.basic.integer.alignment = lttng_alignof(unsigned long) * CHAR_BIT; - fields[0].type.u.basic.integer.signedness = lttng_is_signed_type(unsigned long); - fields[0].type.u.basic.integer.reverse_byte_order = 0; - fields[0].type.u.basic.integer.base = 16; - fields[0].type.u.basic.integer.encoding = lttng_encode_none; - - fields[1].name = "parent_ip"; - fields[1].type.atype = atype_integer; - fields[1].type.u.basic.integer.size = sizeof(unsigned long) * CHAR_BIT; - fields[1].type.u.basic.integer.alignment = lttng_alignof(unsigned long) * CHAR_BIT; - fields[1].type.u.basic.integer.signedness = lttng_is_signed_type(unsigned long); - fields[1].type.u.basic.integer.reverse_byte_order = 0; - fields[1].type.u.basic.integer.base = 16; - fields[1].type.u.basic.integer.encoding = lttng_encode_none; - - desc->owner = THIS_MODULE; - event->desc = desc; - - return 0; - -error_fields: - kfree(desc->name); -error_str: - kfree(desc); - return ret; -} - -static -struct ftrace_probe_ops lttng_ftrace_ops = { - .func = lttng_ftrace_handler, -}; - -int lttng_ftrace_register(const char *name, - const char *symbol_name, - struct lttng_event *event) -{ - int ret; - - ret = lttng_create_ftrace_event(name, event); - if (ret) - goto error; - - event->u.ftrace.symbol_name = kstrdup(symbol_name, GFP_KERNEL); - if (!event->u.ftrace.symbol_name) - goto name_error; - - /* Ensure the memory we just allocated don't trigger page faults */ - wrapper_vmalloc_sync_all(); - - ret = wrapper_register_ftrace_function_probe(event->u.ftrace.symbol_name, - <tng_ftrace_ops, event); - if (ret < 0) - goto register_error; - return 0; - -register_error: - kfree(event->u.ftrace.symbol_name); -name_error: - kfree(event->desc->name); - kfree(event->desc); -error: - return ret; -} -EXPORT_SYMBOL_GPL(lttng_ftrace_register); - -void lttng_ftrace_unregister(struct lttng_event *event) -{ - wrapper_unregister_ftrace_function_probe(event->u.ftrace.symbol_name, - <tng_ftrace_ops, event); -} -EXPORT_SYMBOL_GPL(lttng_ftrace_unregister); - -void lttng_ftrace_destroy_private(struct lttng_event *event) -{ - kfree(event->u.ftrace.symbol_name); - kfree(event->desc->fields); - kfree(event->desc->name); - kfree(event->desc); -} -EXPORT_SYMBOL_GPL(lttng_ftrace_destroy_private); - -int lttng_ftrace_init(void) -{ - wrapper_vmalloc_sync_all(); - return 0; -} -module_init(lttng_ftrace_init) - -/* - * Ftrace takes care of waiting for a grace period (RCU sched) at probe - * unregistration, and disables preemption around probe call. - */ -void lttng_ftrace_exit(void) -{ -} -module_exit(lttng_ftrace_exit) - -MODULE_LICENSE("GPL and additional rights"); -MODULE_AUTHOR("Mathieu Desnoyers "); -MODULE_DESCRIPTION("LTTng ftrace probes"); -MODULE_VERSION(__stringify(LTTNG_MODULES_MAJOR_VERSION) "." - __stringify(LTTNG_MODULES_MINOR_VERSION) "." - __stringify(LTTNG_MODULES_PATCHLEVEL_VERSION) - LTTNG_MODULES_EXTRAVERSION); diff --git a/wrapper/ftrace.h b/wrapper/ftrace.h deleted file mode 100644 index eb4f788a..00000000 --- a/wrapper/ftrace.h +++ /dev/null @@ -1,75 +0,0 @@ -/* SPDX-License-Identifier: (GPL-2.0 or LGPL-2.1) - * - * wrapper/ftrace.h - * - * wrapper around vmalloc_sync_all. Using KALLSYMS to get its address when - * available, else we need to have a kernel that exports this function to GPL - * modules. - * - * Copyright (C) 2011-2012 Mathieu Desnoyers - */ - -#ifndef _LTTNG_WRAPPER_FTRACE_H -#define _LTTNG_WRAPPER_FTRACE_H - -#include -#include -#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,12,0)) -#include <../kernel/trace/trace.h> -#endif - -#ifdef CONFIG_KALLSYMS - -#include -#include - -static inline -int wrapper_register_ftrace_function_probe(char *glob, - struct ftrace_probe_ops *ops, void *data) -{ - int (*register_ftrace_function_probe_sym)(char *glob, - struct ftrace_probe_ops *ops, void *data); - - register_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("register_ftrace_function_probe"); - if (register_ftrace_function_probe_sym) { - return register_ftrace_function_probe_sym(glob, ops, data); - } else { - printk_once(KERN_WARNING "LTTng: register_ftrace_function_probe symbol lookup failed.\n"); - return -EINVAL; - } -} - -static inline -void wrapper_unregister_ftrace_function_probe(char *glob, - struct ftrace_probe_ops *ops, void *data) -{ - void (*unregister_ftrace_function_probe_sym)(char *glob, - struct ftrace_probe_ops *ops, void *data); - - unregister_ftrace_function_probe_sym = (void *) kallsyms_lookup_funcptr("unregister_ftrace_function_probe"); - if (unregister_ftrace_function_probe_sym) { - unregister_ftrace_function_probe_sym(glob, ops, data); - } else { - printk_once(KERN_WARNING "LTTng: unregister_ftrace_function_probe symbol lookup failed.\n"); - WARN_ON(1); - } -} - -#else - -static inline -int wrapper_register_ftrace_function_probe(char *glob, - struct ftrace_probe_ops *ops, void *data) -{ - return register_ftrace_function_probe(glob, ops, data); -} - -static inline -void wrapper_unregister_ftrace_function_probe(char *glob, - struct ftrace_probe_ops *ops, void *data) -{ - return unregister_ftrace_function_probe(glob, ops, data); -} -#endif - -#endif /* _LTTNG_WRAPPER_FTRACE_H */ -- 2.34.1