* [RFC v4] Kernel access to ftrace instances. @ 2019-11-13 21:15 Divya Indi 2019-11-13 21:15 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi 0 siblings, 1 reply; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:15 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil Hi, Please review the patches that follow - [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h [PATCH 2/5] tracing: Verify if trace array exists before destroying it. [PATCH 3/5] tracing: Adding NULL checks [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances. [PATCH 5/5] tracing: Sample module to demonstrate kernel access to Ftrace instances. This patchset addresses the feedback recieved for v3. Changes from v3 include - 1) trace_array_get_by_name() replaces trace_array_lookup and with its new implementation we no longer need to export trace_array_create(). If a trace array with given name exists, this func returns a pointer to this trace array (Previously, trace_array_lookup()). If does not exist, create a new trace array (Previously done by trace_array_create()). 2) A new trace array will always have ref ctr = 1 on creation. Destroying a trace array will require its ref ctr to be 1. 3) trace_array_set_clr_event(): Uses boolean instead of 0/1 to enable/disable events to a trace array. 4) Sample module reflects the above changes. It is now part of the same patch-set. Thanks, Divya ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h 2019-11-13 21:15 [RFC v4] Kernel access to ftrace instances Divya Indi @ 2019-11-13 21:15 ` Divya Indi 2019-11-13 21:15 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi 0 siblings, 1 reply; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:15 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil Declare the newly introduced and exported APIs in the header file - include/linux/trace.h. Moving previous declarations from kernel/trace/trace.h to include/linux/trace.h. Signed-off-by: Divya Indi <divya.indi@oracle.com> Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com> Reviewed-by: Manjunath Patil <manjunath.b.patil@oracle.com> --- include/linux/trace.h | 7 +++++++ kernel/trace/trace.h | 4 +--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/include/linux/trace.h b/include/linux/trace.h index b95ffb2..24fcf07 100644 --- a/include/linux/trace.h +++ b/include/linux/trace.h @@ -24,6 +24,13 @@ struct trace_export { int register_ftrace_export(struct trace_export *export); int unregister_ftrace_export(struct trace_export *export); +struct trace_array; + +void trace_printk_init_buffers(void); +int trace_array_printk(struct trace_array *tr, unsigned long ip, + const char *fmt, ...); +struct trace_array *trace_array_create(const char *name); +int trace_array_destroy(struct trace_array *tr); #endif /* CONFIG_TRACING */ #endif /* _LINUX_TRACE_H */ diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index 005f086..66ff63e 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -11,6 +11,7 @@ #include <linux/mmiotrace.h> #include <linux/tracepoint.h> #include <linux/ftrace.h> +#include <linux/trace.h> #include <linux/hw_breakpoint.h> #include <linux/trace_seq.h> #include <linux/trace_events.h> @@ -852,8 +853,6 @@ extern int trace_selftest_startup_branch(struct tracer *trace, extern int trace_array_vprintk(struct trace_array *tr, unsigned long ip, const char *fmt, va_list args); -int trace_array_printk(struct trace_array *tr, - unsigned long ip, const char *fmt, ...); int trace_array_printk_buf(struct ring_buffer *buffer, unsigned long ip, const char *fmt, ...); void trace_printk_seq(struct trace_seq *s); @@ -1869,7 +1868,6 @@ extern int trace_event_enable_disable(struct trace_event_file *file, extern const char *__stop___tracepoint_str[]; void trace_printk_control(bool enabled); -void trace_printk_init_buffers(void); void trace_printk_start_comm(void); int trace_keep_overwrite(struct tracer *tracer, u32 mask, int set); int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] tracing: Verify if trace array exists before destroying it. 2019-11-13 21:15 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi @ 2019-11-13 21:15 ` Divya Indi 2019-11-13 21:16 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi 0 siblings, 1 reply; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:15 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil A trace array can be destroyed from userspace or kernel. Verify if the trace exists before proceeding to destroy/remove it. Signed-off-by: Divya Indi <divya.indi@oracle.com> Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com> Reviewed-by: Manjunath Patil <manjunath.b.patil@oracle.com> --- kernel/trace/trace.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 1c80521..bff967f 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -8421,17 +8421,26 @@ static int __remove_instance(struct trace_array *tr) return 0; } -int trace_array_destroy(struct trace_array *tr) +int trace_array_destroy(struct trace_array *this_tr) { + struct trace_array *tr; int ret; - if (!tr) + if (!this_tr) return -EINVAL; mutex_lock(&event_mutex); mutex_lock(&trace_types_lock); - ret = __remove_instance(tr); + ret = -ENODEV; + + /* Making sure trace array exists before destroying it. */ + list_for_each_entry(tr, &ftrace_trace_arrays, list) { + if (tr == this_tr) { + ret = __remove_instance(tr); + break; + } + } mutex_unlock(&trace_types_lock); mutex_unlock(&event_mutex); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] tracing: Adding NULL checks 2019-11-13 21:15 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi @ 2019-11-13 21:16 ` Divya Indi 2019-11-13 21:16 ` [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances Divya Indi 0 siblings, 1 reply; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:16 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil As part of commit f45d1225adb0 ("tracing: Kernel access to Ftrace instances") we exported certain functions. Here, we are adding some additional NULL checks to ensure safe usage by users of these APIs. Signed-off-by: Divya Indi <divya.indi@oracle.com> Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com> Reviewed-by: Manjunath Patil <manjunath.b.patil@oracle.com> --- kernel/trace/trace.c | 3 +++ kernel/trace/trace_events.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index bff967f..e0faf81 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3205,6 +3205,9 @@ int trace_array_printk(struct trace_array *tr, if (!(global_trace.trace_flags & TRACE_ITER_PRINTK)) return 0; + if (!tr) + return -ENOENT; + va_start(ap, fmt); ret = trace_array_vprintk(tr, ip, fmt, ap); va_end(ap); diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index 0ce3db6..2621995 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -800,6 +800,8 @@ static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set) char *event = NULL, *sub = NULL, *match; int ret; + if (!tr) + return -ENOENT; /* * The buf format can be <subsystem>:<event-name> * *:<event-name> means any event by that name. -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances 2019-11-13 21:16 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi @ 2019-11-13 21:16 ` Divya Indi 2019-11-13 21:16 ` [PATCH 5/5] tracing: Sample module to demonstrate " Divya Indi 2019-11-14 17:17 ` [PATCH 4/5] tracing: Adding new functions for " Steven Rostedt 0 siblings, 2 replies; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:16 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil Adding 2 new functions - 1) struct trace_array *trace_array_get_by_name(const char *name); Return pointer to a trace array with given name. If it does not exist, create and return pointer to the new trace array. 2) int trace_array_set_clr_event(struct trace_array *tr, const char *system ,const char *event, bool enable); Enable/Disable events to this trace array. Additionally, - To handle reference counters, export trace_array_put() - Due to introduction of the above 2 new functions, we no longer need to export - ftrace_set_clr_event & trace_array_create APIs. Signed-off-by: Divya Indi <divya.indi@oracle.com> Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com> Reviewed-by: Manjunath Patil <manjunath.b.patil@oracle.com> --- include/linux/trace.h | 3 +- include/linux/trace_events.h | 3 +- kernel/trace/trace.c | 90 +++++++++++++++++++++++++++++++++++--------- kernel/trace/trace.h | 1 - kernel/trace/trace_events.c | 27 ++++++++++++- 5 files changed, 103 insertions(+), 21 deletions(-) diff --git a/include/linux/trace.h b/include/linux/trace.h index 24fcf07..7fd86d3 100644 --- a/include/linux/trace.h +++ b/include/linux/trace.h @@ -29,7 +29,8 @@ struct trace_export { void trace_printk_init_buffers(void); int trace_array_printk(struct trace_array *tr, unsigned long ip, const char *fmt, ...); -struct trace_array *trace_array_create(const char *name); +void trace_array_put(struct trace_array *tr); +struct trace_array *trace_array_get_by_name(const char *name); int trace_array_destroy(struct trace_array *tr); #endif /* CONFIG_TRACING */ diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 8a62731..3898299 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -540,7 +540,8 @@ extern int trace_define_field(struct trace_event_call *call, const char *type, #define is_signed_type(type) (((type)(-1)) < (type)1) int trace_set_clr_event(const char *system, const char *event, int set); - +int trace_array_set_clr_event(struct trace_array *tr, const char *system, + const char *event, bool enable); /* * The double __builtin_constant_p is because gcc will give us an error * if we try to allocate the static variable to fmt if it is not a diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index e0faf81..58be07b 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -297,12 +297,24 @@ static void __trace_array_put(struct trace_array *this_tr) this_tr->ref--; } +/** + * trace_array_put - Decrement the reference counter for this trace array. + * + * NOTE: Use this when we no longer need the trace array returned by + * trace_array_get_by_name(). This ensures the trace array can be later + * destroyed. + * + */ void trace_array_put(struct trace_array *this_tr) { + if (!this_tr) + return; + mutex_lock(&trace_types_lock); __trace_array_put(this_tr); mutex_unlock(&trace_types_lock); } +EXPORT_SYMBOL_GPL(trace_array_put); int call_filter_check_discard(struct trace_event_call *call, void *rec, struct ring_buffer *buffer, @@ -8302,24 +8314,17 @@ static void update_tracer_options(struct trace_array *tr) mutex_unlock(&trace_types_lock); } -struct trace_array *trace_array_create(const char *name) +static struct trace_array *trace_array_create(const char *name) { struct trace_array *tr; int ret; - mutex_lock(&event_mutex); - mutex_lock(&trace_types_lock); - - ret = -EEXIST; - list_for_each_entry(tr, &ftrace_trace_arrays, list) { - if (tr->name && strcmp(tr->name, name) == 0) - goto out_unlock; - } - ret = -ENOMEM; tr = kzalloc(sizeof(*tr), GFP_KERNEL); if (!tr) - goto out_unlock; + return ERR_PTR(ret); + + mutex_lock(&event_mutex); tr->name = kstrdup(name, GFP_KERNEL); if (!tr->name) @@ -8364,7 +8369,8 @@ struct trace_array *trace_array_create(const char *name) list_add(&tr->list, &ftrace_trace_arrays); - mutex_unlock(&trace_types_lock); + tr->ref++; + mutex_unlock(&event_mutex); return tr; @@ -8375,24 +8381,74 @@ struct trace_array *trace_array_create(const char *name) kfree(tr->name); kfree(tr); - out_unlock: - mutex_unlock(&trace_types_lock); mutex_unlock(&event_mutex); return ERR_PTR(ret); } -EXPORT_SYMBOL_GPL(trace_array_create); static int instance_mkdir(const char *name) { - return PTR_ERR_OR_ZERO(trace_array_create(name)); + struct trace_array *tr; + int ret; + + mutex_lock(&trace_types_lock); + + ret = -EEXIST; + list_for_each_entry(tr, &ftrace_trace_arrays, list) { + if (tr->name && strcmp(tr->name, name) == 0) + goto out_unlock; + } + + tr = trace_array_create(name); + + ret = PTR_ERR_OR_ZERO(tr); + +out_unlock: + mutex_unlock(&trace_types_lock); + return ret; +} + +/** + * trace_array_get_by_name - Create/Lookup a trace array, given its name. + * @name: The name of the trace array to be looked up/created. + * + * Returns pointer to trace array with given name. + * NULL, if it cannot be created. + * + * NOTE: This function increments the reference counter associated with the + * trace array returned. This makes sure it cannot be freed while in use. + * Use trace_array_put() once the trace array is no longer needed. + * + */ +struct trace_array *trace_array_get_by_name(const char *name) +{ + struct trace_array *tr; + + mutex_lock(&trace_types_lock); + + list_for_each_entry(tr, &ftrace_trace_arrays, list) { + if (tr->name && strcmp(tr->name, name) == 0) + goto out_unlock; + } + + tr = trace_array_create(name); + + if (IS_ERR(tr)) + tr = NULL; +out_unlock: + if (tr) + tr->ref++; + mutex_unlock(&trace_types_lock); + return tr; } +EXPORT_SYMBOL_GPL(trace_array_get_by_name); static int __remove_instance(struct trace_array *tr) { int i; - if (tr->ref || (tr->current_trace && tr->current_trace->ref)) + /* Reference counter for a newly created trace array = 1. */ + if (tr->ref > 1 || (tr->current_trace && tr->current_trace->ref)) return -EBUSY; list_del(&tr->list); diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index 66ff63e..643faaa 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -338,7 +338,6 @@ enum { extern struct mutex trace_types_lock; extern int trace_array_get(struct trace_array *tr); -extern void trace_array_put(struct trace_array *tr); extern int tracing_set_time_stamp_abs(struct trace_array *tr, bool abs); extern int tracing_set_clock(struct trace_array *tr, const char *clockstr); diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index 2621995..c58ef22 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -834,7 +834,6 @@ static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set) return ret; } -EXPORT_SYMBOL_GPL(ftrace_set_clr_event); /** * trace_set_clr_event - enable or disable an event @@ -859,6 +858,32 @@ int trace_set_clr_event(const char *system, const char *event, int set) } EXPORT_SYMBOL_GPL(trace_set_clr_event); +/** + * trace_array_set_clr_event - enable or disable an event for a trace array. + * @tr: concerned trace array. + * @system: system name to match (NULL for any system) + * @event: event name to match (NULL for all events, within system) + * @enable: true to enable, false to disable + * + * This is a way for other parts of the kernel to enable or disable + * event recording. + * + * Returns 0 on success, -EINVAL if the parameters do not match any + * registered events. + */ +int trace_array_set_clr_event(struct trace_array *tr, const char *system, + const char *event, bool enable) +{ + int set; + + if (!tr) + return -ENOENT; + + set = (enable == true) ? 1 : 0; + return __ftrace_set_clr_event(tr, NULL, system, event, set); +} +EXPORT_SYMBOL_GPL(trace_array_set_clr_event); + /* 128 should be much more than enough */ #define EVENT_BUF_SIZE 127 -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] tracing: Sample module to demonstrate kernel access to Ftrace instances. 2019-11-13 21:16 ` [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances Divya Indi @ 2019-11-13 21:16 ` Divya Indi 2019-11-14 17:17 ` [PATCH 4/5] tracing: Adding new functions for " Steven Rostedt 1 sibling, 0 replies; 7+ messages in thread From: Divya Indi @ 2019-11-13 21:16 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Divya Indi, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil This is a sample module to demonstrate the use of the newly introduced and exported APIs to access Ftrace instances from within the kernel. Newly introduced APIs used here - 1. Create/Lookup a trace array with the given name. struct trace_array *trace_array_get_by_name(const char *name) 2. Destroy/Remove a trace array. int trace_array_destroy(struct trace_array *tr) 4. Enable/Disable trace events: int trace_array_set_clr_event(struct trace_array *tr, const char *system, const char *event, bool enable); Exported APIs - 1. trace_printk equivalent for instances. int trace_array_printk(struct trace_array *tr, unsigned long ip, const char *fmt, ...); 2. Helper function. void trace_printk_init_buffers(void); 3. To decrement the reference counter. void trace_array_put(struct trace_array *tr) Sample output(contents of /sys/kernel/tracing/instances/sample-instance) NOTE: Tracing disabled after ~5 sec) _-----=> irqs-off / _----=> need-resched | / _---=> hardirq/softirq || / _--=> preempt-depth ||| / delay TASK-PID CPU# |||| TIMESTAMP FUNCTION | | | |||| | | sample-instance-1452 [002] .... 49.430948: simple_thread: trace_array_printk: count=0 sample-instance-1452 [002] .... 49.430951: sample_event: count value=0 at jiffies=4294716608 sample-instance-1452 [002] .... 50.454847: simple_thread: trace_array_printk: count=1 sample-instance-1452 [002] .... 50.454849: sample_event: count value=1 at jiffies=4294717632 sample-instance-1452 [002] .... 51.478748: simple_thread: trace_array_printk: count=2 sample-instance-1452 [002] .... 51.478750: sample_event: count value=2 at jiffies=4294718656 sample-instance-1452 [002] .... 52.502652: simple_thread: trace_array_printk: count=3 sample-instance-1452 [002] .... 52.502655: sample_event: count value=3 at jiffies=4294719680 sample-instance-1452 [002] .... 53.526533: simple_thread: trace_array_printk: count=4 sample-instance-1452 [002] .... 53.526535: sample_event: count value=4 at jiffies=4294720704 sample-instance-1452 [002] .... 54.550438: simple_thread: trace_array_printk: count=5 sample-instance-1452 [002] .... 55.574336: simple_thread: trace_array_printk: count=6 Signed-off-by: Divya Indi <divya.indi@oracle.com> Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com> Reviewed-by: Manjunath Patil <manjunath.b.patil@oracle.com> --- samples/Kconfig | 7 ++ samples/Makefile | 1 + samples/ftrace_instance/Makefile | 6 ++ samples/ftrace_instance/sample-trace-array.c | 131 +++++++++++++++++++++++++++ samples/ftrace_instance/sample-trace-array.h | 84 +++++++++++++++++ 5 files changed, 229 insertions(+) create mode 100644 samples/ftrace_instance/Makefile create mode 100644 samples/ftrace_instance/sample-trace-array.c create mode 100644 samples/ftrace_instance/sample-trace-array.h diff --git a/samples/Kconfig b/samples/Kconfig index d63cc8a..1c7864b 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -20,6 +20,13 @@ config SAMPLE_TRACE_PRINTK This builds a module that calls trace_printk() and can be used to test various trace_printk() calls from a module. +config SAMPLE_TRACE_ARRAY + tristate "Build sample module for kernel access to Ftrace instancess" + depends on EVENT_TRACING && m + help + This builds a module that demonstrates the use of various APIs to + access Ftrace instances from within the kernel. + config SAMPLE_KOBJECT tristate "Build kobject examples" help diff --git a/samples/Makefile b/samples/Makefile index debf892..02c444e 100644 --- a/samples/Makefile +++ b/samples/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_SAMPLE_RPMSG_CLIENT) += rpmsg/ subdir-$(CONFIG_SAMPLE_SECCOMP) += seccomp obj-$(CONFIG_SAMPLE_TRACE_EVENTS) += trace_events/ obj-$(CONFIG_SAMPLE_TRACE_PRINTK) += trace_printk/ +obj-$(CONFIG_SAMPLE_TRACE_ARRAY) += ftrace_instance/ obj-$(CONFIG_VIDEO_PCI_SKELETON) += v4l/ obj-y += vfio-mdev/ subdir-$(CONFIG_SAMPLE_VFS) += vfs diff --git a/samples/ftrace_instance/Makefile b/samples/ftrace_instance/Makefile new file mode 100644 index 0000000..3603b13 --- /dev/null +++ b/samples/ftrace_instance/Makefile @@ -0,0 +1,6 @@ +# Builds a module that calls various routines to access Ftrace instances. +# To use(as root): insmod sample-trace-array.ko + +CFLAGS_sample-trace-array.o := -I$(src) + +obj-$(CONFIG_SAMPLE_TRACE_ARRAY) += sample-trace-array.o diff --git a/samples/ftrace_instance/sample-trace-array.c b/samples/ftrace_instance/sample-trace-array.c new file mode 100644 index 0000000..d523450 --- /dev/null +++ b/samples/ftrace_instance/sample-trace-array.c @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <linux/module.h> +#include <linux/kthread.h> +#include <linux/trace.h> +#include <linux/trace_events.h> +#include <linux/timer.h> +#include <linux/err.h> +#include <linux/jiffies.h> + +/* + * Any file that uses trace points, must include the header. + * But only one file, must include the header by defining + * CREATE_TRACE_POINTS first. This will make the C code that + * creates the handles for the trace points. + */ +#define CREATE_TRACE_POINTS +#include "sample-trace-array.h" + +struct trace_array *tr; +static void mytimer_handler(struct timer_list *unused); +static struct task_struct *simple_tsk; + +/* + * mytimer: Timer setup to disable tracing for event "sample_event". This + * timer is only for the purposes of the sample module to demonstrate access of + * Ftrace instances from within kernel. + */ +static DEFINE_TIMER(mytimer, mytimer_handler); + +static void mytimer_handler(struct timer_list *unused) +{ + /* + * Disable tracing for event "sample_event". + */ + trace_array_set_clr_event(tr, "sample-subsystem", "sample_event", + false); +} + +static void simple_thread_func(int count) +{ + set_current_state(TASK_INTERRUPTIBLE); + schedule_timeout(HZ); + + /* + * Printing count value using trace_array_printk() - trace_printk() + * equivalent for the instance buffers. + */ + trace_array_printk(tr, _THIS_IP_, "trace_array_printk: count=%d\n", + count); + /* + * Tracepoint for event "sample_event". This will print the + * current value of count and current jiffies. + */ + trace_sample_event(count, jiffies); +} + +static int simple_thread(void *arg) +{ + int count = 0; + unsigned long delay = msecs_to_jiffies(5000); + + /* + * Enable tracing for "sample_event". + */ + trace_array_set_clr_event(tr, "sample-subsystem", "sample_event", true); + + /* + * Adding timer - mytimer. This timer will disable tracing after + * delay seconds. + * + */ + add_timer(&mytimer); + mod_timer(&mytimer, jiffies+delay); + + while (!kthread_should_stop()) + simple_thread_func(count++); + + del_timer(&mytimer); + + /* + * trace_array_put() decrements the reference counter associated with + * the trace array - "tr". We are done using the trace array, hence + * decrement the reference counter so that it can be destroyed using + * trace_array_destroy(). + */ + trace_array_put(tr); + + return 0; +} + +static int __init sample_trace_array_init(void) +{ + /* + * Return a pointer to the trace array with name "sample-instance" if it + * exists, else create a new trace array. + * + * NOTE: This function increments the reference counter + * associated with the trace array - "tr". + */ + tr = trace_array_get_by_name("sample-instance"); + + if (!tr) + return -1; + /* + * If context specific per-cpu buffers havent already been allocated. + */ + trace_printk_init_buffers(); + + simple_tsk = kthread_run(simple_thread, NULL, "sample-instance"); + if (IS_ERR(simple_tsk)) + return -1; + return 0; +} + +static void __exit sample_trace_array_exit(void) +{ + kthread_stop(simple_tsk); + + /* + * We are unloading our module and no longer require the trace array. + * Remove/destroy "tr" using trace_array_destroy() + */ + trace_array_destroy(tr); +} + +module_init(sample_trace_array_init); +module_exit(sample_trace_array_exit); + +MODULE_AUTHOR("Divya Indi"); +MODULE_DESCRIPTION("Sample module for kernel access to Ftrace instances"); +MODULE_LICENSE("GPL"); diff --git a/samples/ftrace_instance/sample-trace-array.h b/samples/ftrace_instance/sample-trace-array.h new file mode 100644 index 0000000..6f89624 --- /dev/null +++ b/samples/ftrace_instance/sample-trace-array.h @@ -0,0 +1,84 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * If TRACE_SYSTEM is defined, that will be the directory created + * in the ftrace directory under /sys/kernel/tracing/events/<system> + * + * The define_trace.h below will also look for a file name of + * TRACE_SYSTEM.h where TRACE_SYSTEM is what is defined here. + * In this case, it would look for sample-trace.h + * + * If the header name will be different than the system name + * (as in this case), then you can override the header name that + * define_trace.h will look up by defining TRACE_INCLUDE_FILE + * + * This file is called sample-trace-array.h but we want the system + * to be called "sample-subsystem". Therefore we must define the name of this + * file: + * + * #define TRACE_INCLUDE_FILE sample-trace-array + * + * As we do in the bottom of this file. + * + * Notice that TRACE_SYSTEM should be defined outside of #if + * protection, just like TRACE_INCLUDE_FILE. + */ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM sample-subsystem + +/* + * TRACE_SYSTEM is expected to be a C valid variable (alpha-numeric + * and underscore), although it may start with numbers. If for some + * reason it is not, you need to add the following lines: + */ +#undef TRACE_SYSTEM_VAR +#define TRACE_SYSTEM_VAR sample_subsystem + +/* + * But the above is only needed if TRACE_SYSTEM is not alpha-numeric + * and underscored. By default, TRACE_SYSTEM_VAR will be equal to + * TRACE_SYSTEM. As TRACE_SYSTEM_VAR must be alpha-numeric, if + * TRACE_SYSTEM is not, then TRACE_SYSTEM_VAR must be defined with + * only alpha-numeric and underscores. + * + * The TRACE_SYSTEM_VAR is only used internally and not visible to + * user space. + */ + +/* + * Notice that this file is not protected like a normal header. + * We also must allow for rereading of this file. The + * + * || defined(TRACE_HEADER_MULTI_READ) + * + * serves this purpose. + */ +#if !defined(_SAMPLE_TRACE_ARRAY_H) || defined(TRACE_HEADER_MULTI_READ) +#define _SAMPLE_TRACE_ARRAY_H + +#include <linux/tracepoint.h> +TRACE_EVENT(sample_event, + + TP_PROTO(int count, unsigned long time), + + TP_ARGS(count, time), + + TP_STRUCT__entry( + __field(int, count) + __field(unsigned long, time) + ), + + TP_fast_assign( + __entry->count = count; + __entry->time = time; + ), + + TP_printk("count value=%d at jiffies=%lu", __entry->count, + __entry->time) + ); +#endif + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH . +#define TRACE_INCLUDE_FILE sample-trace-array +#include <trace/define_trace.h> -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances 2019-11-13 21:16 ` [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances Divya Indi 2019-11-13 21:16 ` [PATCH 5/5] tracing: Sample module to demonstrate " Divya Indi @ 2019-11-14 17:17 ` Steven Rostedt 1 sibling, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2019-11-14 17:17 UTC (permalink / raw) To: Divya Indi Cc: linux-kernel, Aruna Ramakrishna, Srinivas Eeda, Joe Jin, Manjunath Patil On Wed, 13 Nov 2019 13:16:01 -0800 Divya Indi <divya.indi@oracle.com> wrote: > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -297,12 +297,24 @@ static void __trace_array_put(struct trace_array *this_tr) > this_tr->ref--; > } > > +/** > + * trace_array_put - Decrement the reference counter for this trace array. > + * > + * NOTE: Use this when we no longer need the trace array returned by > + * trace_array_get_by_name(). This ensures the trace array can be later > + * destroyed. > + * > + */ > void trace_array_put(struct trace_array *this_tr) > { > + if (!this_tr) > + return; > + > mutex_lock(&trace_types_lock); > __trace_array_put(this_tr); > mutex_unlock(&trace_types_lock); > } > +EXPORT_SYMBOL_GPL(trace_array_put); > > int call_filter_check_discard(struct trace_event_call *call, void *rec, > struct ring_buffer *buffer, > @@ -8302,24 +8314,17 @@ static void update_tracer_options(struct trace_array *tr) > mutex_unlock(&trace_types_lock); > } > > -struct trace_array *trace_array_create(const char *name) > +static struct trace_array *trace_array_create(const char *name) > { > struct trace_array *tr; > int ret; > > - mutex_lock(&event_mutex); > - mutex_lock(&trace_types_lock); > - > - ret = -EEXIST; > - list_for_each_entry(tr, &ftrace_trace_arrays, list) { > - if (tr->name && strcmp(tr->name, name) == 0) > - goto out_unlock; > - } > - > ret = -ENOMEM; > tr = kzalloc(sizeof(*tr), GFP_KERNEL); > if (!tr) > - goto out_unlock; > + return ERR_PTR(ret); > + > + mutex_lock(&event_mutex); > > tr->name = kstrdup(name, GFP_KERNEL); > if (!tr->name) > @@ -8364,7 +8369,8 @@ struct trace_array *trace_array_create(const char *name) > > list_add(&tr->list, &ftrace_trace_arrays); > > - mutex_unlock(&trace_types_lock); > + tr->ref++; > + > mutex_unlock(&event_mutex); > > return tr; > @@ -8375,24 +8381,74 @@ struct trace_array *trace_array_create(const char *name) > kfree(tr->name); > kfree(tr); > > - out_unlock: > - mutex_unlock(&trace_types_lock); > mutex_unlock(&event_mutex); > > return ERR_PTR(ret); > } > -EXPORT_SYMBOL_GPL(trace_array_create); > > static int instance_mkdir(const char *name) > { > - return PTR_ERR_OR_ZERO(trace_array_create(name)); > + struct trace_array *tr; > + int ret; > + > + mutex_lock(&trace_types_lock); > + > + ret = -EEXIST; > + list_for_each_entry(tr, &ftrace_trace_arrays, list) { > + if (tr->name && strcmp(tr->name, name) == 0) > + goto out_unlock; > + } > + > + tr = trace_array_create(name); You just changed the locking order here, which can cause a deadlock. You can't take event_mutex after taking trace_types_lock. I applied this, booted with lockdep enabled, loaded your sample module and triggered this: e1000e: em1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx IPv6: ADDRCONF(NETDEV_CHANGE): em1: link becomes ready L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details. ====================================================== WARNING: possible circular locking dependency detected 5.4.0-rc6-test+ #23 Not tainted ------------------------------------------------------ modprobe/1569 is trying to acquire lock: ffffffff90677a80 (event_mutex){+.+.}, at: trace_array_create+0x47/0x230 but task is already holding lock: ffffffff90674660 (trace_types_lock){+.+.}, at: trace_array_get_by_name+0x13/0x80 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (trace_types_lock){+.+.}: __mutex_lock+0x95/0x920 trace_add_event_call+0x23/0xd0 trace_probe_register_event_call+0x22/0x50 trace_kprobe_create+0x681/0xa90 create_or_delete_trace_kprobe+0xd/0x30 trace_run_command+0x72/0x90 kprobe_trace_self_tests_init+0x53/0x41c do_one_initcall+0x5d/0x314 kernel_init_freeable+0x218/0x2dd kernel_init+0xa/0x100 ret_from_fork+0x3a/0x50 -> #0 (event_mutex){+.+.}: __lock_acquire+0xd17/0x14c0 lock_acquire+0x9e/0x190 __mutex_lock+0x95/0x920 trace_array_create+0x47/0x230 trace_array_get_by_name+0x4c/0x80 sample_trace_array_init+0x12/0xfa8 [sample_trace_array] do_one_initcall+0x5d/0x314 do_init_module+0x5a/0x220 load_module+0x2172/0x2480 __do_sys_finit_module+0xa8/0x110 do_syscall_64+0x60/0x210 entry_SYSCALL_64_after_hwframe+0x49/0xbe other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(trace_types_lock); lock(event_mutex); lock(trace_types_lock); lock(event_mutex); *** DEADLOCK *** 1 lock held by modprobe/1569: #0: ffffffff90674660 (trace_types_lock){+.+.}, at: trace_array_get_by_name+0x13/0x80 stack backtrace: CPU: 2 PID: 1569 Comm: modprobe Not tainted 5.4.0-rc6-test+ #23 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016 Call Trace: dump_stack+0x85/0xc0 check_noncircular+0x172/0x190 ? find_held_lock+0x2d/0x90 __lock_acquire+0xd17/0x14c0 lock_acquire+0x9e/0x190 ? trace_array_create+0x47/0x230 __mutex_lock+0x95/0x920 ? trace_array_create+0x47/0x230 ? fs_reclaim_release.part.99+0x5/0x20 ? trace_array_create+0x47/0x230 ? trace_array_create+0x2d/0x230 ? rcu_read_lock_sched_held+0x52/0x80 ? trace_array_create+0x47/0x230 trace_array_create+0x47/0x230 trace_array_get_by_name+0x4c/0x80 ? trace_event_define_fields_sample_event+0x58/0x58 [sample_trace_array] sample_trace_array_init+0x12/0xfa8 [sample_trace_array] do_one_initcall+0x5d/0x314 ? rcu_read_lock_sched_held+0x52/0x80 ? kmem_cache_alloc_trace+0x278/0x2b0 do_init_module+0x5a/0x220 load_module+0x2172/0x2480 ? vfs_read+0x11d/0x140 ? __do_sys_finit_module+0xa8/0x110 __do_sys_finit_module+0xa8/0x110 do_syscall_64+0x60/0x210 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x7f6ba89b0efd Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5b 7f 0c 00 f7 d8 64 89 01 48 -- Steve > + > + ret = PTR_ERR_OR_ZERO(tr); > + > +out_unlock: > + mutex_unlock(&trace_types_lock); > + return ret; > +} > + > +/** > + * trace_array_get_by_name - Create/Lookup a trace array, given its name. > + * @name: The name of the trace array to be looked up/created. > + * > + * Returns pointer to trace array with given name. > + * NULL, if it cannot be created. > + * > + * NOTE: This function increments the reference counter associated with the > + * trace array returned. This makes sure it cannot be freed while in use. > + * Use trace_array_put() once the trace array is no longer needed. > + * > + */ > +struct trace_array *trace_array_get_by_name(const char *name) > +{ > + struct trace_array *tr; > + > + mutex_lock(&trace_types_lock); > + > + list_for_each_entry(tr, &ftrace_trace_arrays, list) { > + if (tr->name && strcmp(tr->name, name) == 0) > + goto out_unlock; > + } > + > + tr = trace_array_create(name); > + > + if (IS_ERR(tr)) > + tr = NULL; > +out_unlock: > + if (tr) > + tr->ref++; > + mutex_unlock(&trace_types_lock); > + return tr; > } > +EXPORT_SYMBOL_GPL(trace_array_get_by_name); > > static int __remove_instance(struct trace_array *tr) > { > int i; > > - if (tr->ref || (tr->current_trace && tr->current_trace->ref)) > + /* Reference counter for a newly created trace array = 1. */ > + if (tr->ref > 1 || (tr->current_trace && tr->current_trace->ref)) > return -EBUSY; > > list_del(&tr->list); > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h > index 66ff63e..643faaa 100644 > --- a/kernel/trace/trace.h > +++ b/kernel/trace/trace.h > @@ -338,7 +338,6 @@ enum { > extern struct mutex trace_types_lock; > > extern int trace_array_get(struct trace_array *tr); > -extern void trace_array_put(struct trace_array *tr); > > extern int tracing_set_time_stamp_abs(struct trace_array *tr, bool abs); > extern int tracing_set_clock(struct trace_array *tr, const char *clockstr); > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > index 2621995..c58ef22 100644 > --- a/kernel/trace/trace_events.c > +++ b/kernel/trace/trace_events.c > @@ -834,7 +834,6 @@ static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set) > > return ret; > } > -EXPORT_SYMBOL_GPL(ftrace_set_clr_event); > > /** > * trace_set_clr_event - enable or disable an event > @@ -859,6 +858,32 @@ int trace_set_clr_event(const char *system, const char *event, int set) > } > EXPORT_SYMBOL_GPL(trace_set_clr_event); > > +/** > + * trace_array_set_clr_event - enable or disable an event for a trace array. > + * @tr: concerned trace array. > + * @system: system name to match (NULL for any system) > + * @event: event name to match (NULL for all events, within system) > + * @enable: true to enable, false to disable > + * > + * This is a way for other parts of the kernel to enable or disable > + * event recording. > + * > + * Returns 0 on success, -EINVAL if the parameters do not match any > + * registered events. > + */ > +int trace_array_set_clr_event(struct trace_array *tr, const char *system, > + const char *event, bool enable) > +{ > + int set; > + > + if (!tr) > + return -ENOENT; > + > + set = (enable == true) ? 1 : 0; > + return __ftrace_set_clr_event(tr, NULL, system, event, set); > +} > +EXPORT_SYMBOL_GPL(trace_array_set_clr_event); > + > /* 128 should be much more than enough */ > #define EVENT_BUF_SIZE 127 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-14 17:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-13 21:15 [RFC v4] Kernel access to ftrace instances Divya Indi 2019-11-13 21:15 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi 2019-11-13 21:15 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi 2019-11-13 21:16 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi 2019-11-13 21:16 ` [PATCH 4/5] tracing: Adding new functions for kernel access to Ftrace instances Divya Indi 2019-11-13 21:16 ` [PATCH 5/5] tracing: Sample module to demonstrate " Divya Indi 2019-11-14 17:17 ` [PATCH 4/5] tracing: Adding new functions for " Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox