LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/5 v4]Kernel Access to Ftrace instances.
@ 2019-08-14 17:55 Divya Indi
  2019-08-14 17:55 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ 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

In addition to patches introduced by commit f45d1225adb0 "tracing: Kernel
access to Ftrace instances") we also need the following patches to reliably
access ftrace instances from other kernel modules or components.

This version addresses the review comments/suggestions received for v3.

Please review the patches that follow.

Divya Indi (5):
  tracing: Declare newly exported APIs in include/linux/trace.h
  tracing: Verify if trace array exists before destroying it.
  tracing: Adding NULL checks
  tracing: Handle the trace array ref counter in new functions
  tracing: New functions for kernel access to Ftrace instances

 include/linux/trace.h        | 10 +++++
 include/linux/trace_events.h |  3 +-
 kernel/trace/trace.c         | 88 ++++++++++++++++++++++++++++++++++++++++++--
 kernel/trace/trace.h         |  4 +-
 kernel/trace/trace_events.c  | 25 ++++++++++++-
 5 files changed, 121 insertions(+), 9 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h
  2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
  2019-08-14 17:55 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ 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

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>
---
 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	[flat|nested] 7+ messages in thread

* [PATCH 2/5] tracing: Verify if trace array exists before destroying it.
  2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
  2019-08-14 17:55 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
  2019-08-14 18:42   ` Aruna Ramakrishna
  2019-08-14 17:55 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 7+ 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

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>
---
 kernel/trace/trace.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1c80521..468ecc5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8421,17 +8421,27 @@ 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	[flat|nested] 7+ messages in thread

* [PATCH 3/5] tracing: Adding NULL checks
  2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
  2019-08-14 17:55 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi
  2019-08-14 17:55 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
  2019-08-14 17:55 ` [PATCH 4/5] tracing: Handle the trace array ref counter in new functions Divya Indi
  2019-08-14 17:55 ` [PATCH 5/5] tracing: New functions for kernel access to Ftrace instances Divya Indi
  4 siblings, 0 replies; 7+ 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	[flat|nested] 7+ messages in thread

* [PATCH 4/5] tracing: Handle the trace array ref counter in new functions
  2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
                   ` (2 preceding siblings ...)
  2019-08-14 17:55 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
  2019-08-14 17:55 ` [PATCH 5/5] tracing: New functions for kernel access to Ftrace instances Divya Indi
  4 siblings, 0 replies; 7+ 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

For functions returning a trace array Eg: trace_array_create(), we need to
increment the reference counter associated with the trace array to ensure it
does not get freed when in use.

Once we are done using the trace array, we need to call
trace_array_put() to make sure we are not holding a reference to it
anymore and the instance/trace array can be removed when required.

Hence, additionally exporting trace_array_put().

Example use:

tr = trace_array_create("foo-bar");
// Use this trace array
// Log to this trace array or enable/disable events to this trace array.
....
....
// tr no longer required
trace_array_put();

Signed-off-by: Divya Indi <divya.indi@oracle.com>
---
 include/linux/trace.h |  1 +
 kernel/trace/trace.c  | 41 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index 24fcf07..2c782d5 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -31,6 +31,7 @@ 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);
+void trace_array_put(struct trace_array *tr);
 #endif	/* CONFIG_TRACING */
 
 #endif	/* _LINUX_TRACE_H */
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 22bf166..7b6a37a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -297,12 +297,22 @@ static void __trace_array_put(struct trace_array *this_tr)
 	this_tr->ref--;
 }
 
+/**
+ * trace_array_put - Decrement reference counter for the given trace array.
+ * @tr: Trace array for which reference counter needs to decrement.
+ *
+ * NOTE: Functions like trace_array_create increment the reference counter for
+ * the trace array to ensure they do not get freed while in use. Make sure to
+ * call trace_array_put() when the use is done. This will ensure that the
+ * instance can be later removed.
+ */
 void trace_array_put(struct trace_array *this_tr)
 {
 	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,6 +8312,16 @@ static void update_tracer_options(struct trace_array *tr)
 	mutex_unlock(&trace_types_lock);
 }
 
+/**
+ * trace_array_create - Create a new trace array with the given name.
+ * @name: The name of the trace array to be created.
+ *
+ * Create and return a trace array with given name if it does not exist.
+ *
+ * NOTE: The reference counter associated with the returned trace array is
+ * incremented to ensure it is not freed when in use. Make sure to call
+ * "trace_array_put" for this trace array when its use is done.
+ */
 struct trace_array *trace_array_create(const char *name)
 {
 	struct trace_array *tr;
@@ -8364,6 +8384,8 @@ struct trace_array *trace_array_create(const char *name)
 
 	list_add(&tr->list, &ftrace_trace_arrays);
 
+	tr->ref++;
+
 	mutex_unlock(&trace_types_lock);
 	mutex_unlock(&event_mutex);
 
@@ -8385,7 +8407,19 @@ struct trace_array *trace_array_create(const char *name)
 
 static int instance_mkdir(const char *name)
 {
-	return PTR_ERR_OR_ZERO(trace_array_create(name));
+	struct trace_array *tr;
+
+	tr = trace_array_create(name);
+	if (IS_ERR(tr))
+		return PTR_ERR(tr);
+
+	/* This function does not return a reference to the created trace array,
+	 * so the reference counter is to be 0 when it returns.
+	 * trace_array_create increments the ref counter, decrement it here
+	 * by calling trace_array_put() */
+	trace_array_put(tr);
+
+	return 0;
 }
 
 static int __remove_instance(struct trace_array *tr)
@@ -8424,6 +8458,11 @@ static int __remove_instance(struct trace_array *tr)
 	return 0;
 }
 
+/*
+ * NOTE: An instance cannot be removed if there is still a reference to it.
+ * Make sure to call "trace_array_put" for a trace array returned by functions
+ * like trace_array_create(), otherwise trace_array_destroy will not succeed.
+ */
 int trace_array_destroy(struct trace_array *this_tr)
 {
 	struct trace_array *tr;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 5/5] tracing: New functions for kernel access to Ftrace instances
  2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
                   ` (3 preceding siblings ...)
  2019-08-14 17:55 ` [PATCH 4/5] tracing: Handle the trace array ref counter in new functions Divya Indi
@ 2019-08-14 17:55 ` Divya Indi
  4 siblings, 0 replies; 7+ 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

Adding 2 new functions -
1) trace_array_lookup : Look up and return a trace array, given its
name.
2) trace_array_set_clr_event : Enable/disable event recording to the
given trace array.

NOTE: trace_array_lookup returns a trace array and also increments the
reference counter associated with the returned trace array. Make sure to
call trace_array_put() once the use is done so that the instance can be
removed at a later time.

Example use:

tr = trace_array_lookup("foo-bar");
if (!tr)
	tr = trace_array_create("foo-bar");
// Log to tr
// Enable/disable events to tr
trace_array_set_clr_event(tr, _THIS_IP,"system","event",1);

// Done using tr
trace_array_put(tr);
..

Also, use trace_array_set_clr_event to enable/disable events to a trace array.
So now we no longer need to have ftrace_set_clr_event as an exported
API.

Signed-off-by: Divya Indi <divya.indi@oracle.com>
---
 include/linux/trace.h        |  2 ++
 include/linux/trace_events.h |  3 ++-
 kernel/trace/trace.c         | 28 ++++++++++++++++++++++++++++
 kernel/trace/trace_events.c  | 23 ++++++++++++++++++++++-
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index 2c782d5..05164bb 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -32,6 +32,8 @@ int trace_array_printk(struct trace_array *tr, unsigned long ip,
 struct trace_array *trace_array_create(const char *name);
 int trace_array_destroy(struct trace_array *tr);
 void trace_array_put(struct trace_array *tr);
+struct trace_array *trace_array_lookup(const char *name);
+
 #endif	/* CONFIG_TRACING */
 
 #endif	/* _LINUX_TRACE_H */
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 8a62731..05a7514 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, int set);
 /*
  * 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 7b6a37a..e394d55 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8514,6 +8514,34 @@ static int instance_rmdir(const char *name)
 	return ret;
 }
 
+/**
+ * trace_array_lookup - Lookup the trace array, given its name.
+ * @name: The name of the trace array to be looked up.
+ *
+ * Lookup and return the trace array associated with @name.
+ *
+ * NOTE: The reference counter associated with the returned trace array is
+ * incremented to ensure it is not freed when in use. Make sure to call
+ * "trace_array_put" for this trace array when its use is done.
+ */
+struct trace_array *trace_array_lookup(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) {
+			tr->ref++;
+			mutex_unlock(&trace_types_lock);
+			return tr;
+		}
+	}
+	mutex_unlock(&trace_types_lock);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(trace_array_lookup);
+
 static __init void create_trace_instances(struct dentry *d_tracer)
 {
 	trace_instance_dir = tracefs_create_instance_dir("instances", d_tracer,
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2621995..96dd997 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,28 @@ 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
+ * @system: system name to match (NULL for any system)
+ * @event: event name to match (NULL for all events, within system)
+ * @set: 1 to enable, 0 to disable
+ *
+ * This is a way for other parts of the kernel to enable or disable
+ * event recording to instances.
+ *
+ * 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, int set)
+{
+	if (!tr)
+		return -ENOENT;
+
+	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	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] tracing: Verify if trace array exists before destroying it.
  2019-08-14 17:55 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi
@ 2019-08-14 18:42   ` Aruna Ramakrishna
  0 siblings, 0 replies; 7+ messages in thread
From: Aruna Ramakrishna @ 2019-08-14 18:42 UTC (permalink / raw)
  To: Divya Indi, Steven Rostedt, linux-kernel; +Cc: Joe Jin, Srinivas Eeda



On 08/14/2019 10:55 AM, Divya Indi wrote:
> A trace array can be destroyed from userspace or kernel. Verify if the
> trace exists before proceeding to destroy/remove it.
>

^^ s/trace/trace array/

As you pointed out yourself. :)

All the patches look good to me. For this patchset:

Reviewed-by: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>

Thanks,
Aruna

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 17:55 [PATCH 0/5 v4]Kernel Access to Ftrace instances Divya Indi
2019-08-14 17:55 ` [PATCH 1/5] tracing: Declare newly exported APIs in include/linux/trace.h Divya Indi
2019-08-14 17:55 ` [PATCH 2/5] tracing: Verify if trace array exists before destroying it Divya Indi
2019-08-14 18:42   ` Aruna Ramakrishna
2019-08-14 17:55 ` [PATCH 3/5] tracing: Adding NULL checks Divya Indi
2019-08-14 17:55 ` [PATCH 4/5] tracing: Handle the trace array ref counter in new functions Divya Indi
2019-08-14 17:55 ` [PATCH 5/5] tracing: New functions for kernel access to Ftrace instances Divya Indi

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox