* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* [PATCH 3/5] tracing: Adding NULL checks
2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access " Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
0 siblings, 0 replies; 8+ messages in thread
From: Divya Indi @ 2019-08-14 17:55 UTC (permalink / raw)
To: Steven Rostedt, linux-kernel
Cc: Divya Indi, Joe Jin, Srinivas Eeda, Aruna Ramakrishna
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>
---
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 468ecc5..22bf166 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] 8+ messages in thread
end of thread, other threads:[~2019-11-14 17:17 UTC | newest]
Thread overview: 8+ 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
-- strict thread matches above, loose matches on Subject: below --
2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access " Divya Indi
2019-08-14 17:55 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).