[PATCH 0/7[v3]] Kernel access to Ftrace instances.

[PATCH 0/7[v3]] Kernel access to Ftrace instances.

[Divya Indi]

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.

Please review the patches that follow.

Divya Indi (7):
  tracing: Required changes to use ftrace_set_clr_event.
  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
  tracing: Un-export ftrace_set_clr_event

 include/linux/trace.h        | 10 +++++
 include/linux/trace_events.h |  2 +
 kernel/trace/trace.c         | 90 ++++++++++++++++++++++++++++++++++++++++++--
 kernel/trace/trace.h         |  4 +-
 kernel/trace/trace_events.c  | 25 +++++++++++-
 5 files changed, 123 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[PATCH 1/7] tracing: Required changes to use ftrace_set_clr_event.

[Divya Indi]

As part of commit f45d1225adb0 ("tracing: Kernel access to Ftrace
instances") ftrace_set_clr_event was exported, but for other kernel
modules to use the function, we require the following additional changes

1. Removing the static keyword for this newly exported function.
2. Declaring it in the header file - include/linux/trace_events.h

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

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 8a62731..0f874fb 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -540,6 +540,7 @@ 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 ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
 
 /*
  * The double __builtin_constant_p is because gcc will give us an error
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0ce3db6..b6b4618 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -795,7 +795,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
 	return ret;
 }
 
-static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
 {
 	char *event = NULL, *sub = NULL, *match;
 	int ret;
-- 
1.8.3.1

[PATCH 2/7] tracing: Declare newly exported APIs in include/linux/trace.h

[Divya Indi]

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>
---
 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

[PATCH 3/7] tracing: Verify if trace array exists before destroying it.

[Divya Indi]

A trace array can be destroyed from userspace or kernel. Verify if the
trace array exists before proceeding to destroy/remove it.

Signed-off-by: Divya Indi <divya.indi@oracle.com>
Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@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

[PATCH 4/7] tracing: Adding NULL checks

[Divya Indi]

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>
---
 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 b6b4618..99eb5f8 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -800,6 +800,8 @@ 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

[PATCH 5/7] tracing: Handle the trace array ref counter in new functions

[Divya Indi]

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>
Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
---
 include/linux/trace.h |  1 +
 kernel/trace/trace.c  | 43 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 43 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..130579b 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,21 @@ 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 +8460,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

[PATCH 6/7] tracing: New functions for kernel access to Ftrace instances

[Divya Indi]

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);
..

Signed-off-by: Divya Indi <divya.indi@oracle.com>
Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
---
 include/linux/trace.h        |  2 ++
 include/linux/trace_events.h |  2 ++
 kernel/trace/trace.c         | 28 ++++++++++++++++++++++++++++
 kernel/trace/trace_events.c  | 22 ++++++++++++++++++++++
 4 files changed, 54 insertions(+)

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 0f874fb..6da3600 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -540,6 +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);
 int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
 
 /*
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 130579b..fcf42bb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8516,6 +8516,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 99eb5f8..9ee6b52 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -859,6 +859,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

[PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Divya Indi]

Use "trace_array_set_clr_event" to enable/disable events to a trace
array from other kernel modules/components.
Hence, we no longer need to have ftrace_set_clr_event as an exported API.

Signed-off-by: Divya Indi <divya.indi@oracle.com>
Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
---
 include/linux/trace_events.h | 1 -
 kernel/trace/trace_events.c  | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 6da3600..025ae2b 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -542,7 +542,6 @@ extern int trace_define_field(struct trace_event_call *call, const char *type,
 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);
-int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
 
 /*
  * The double __builtin_constant_p is because gcc will give us an error
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 9ee6b52..96dd997 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -795,7 +795,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
 	return ret;
 }
 
-int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
 {
 	char *event = NULL, *sub = NULL, *match;
 	int ret;
@@ -834,7 +834,6 @@ 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
-- 
1.8.3.1

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Steven Rostedt]

On Mon, 29 Jul 2019 17:02:34 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> Use "trace_array_set_clr_event" to enable/disable events to a trace
> array from other kernel modules/components.
> Hence, we no longer need to have ftrace_set_clr_event as an exported API.

Hmm, this simply reverts patch 1. Why have patch 1 and this patch in
the first place?

-- Steve

> 
> Signed-off-by: Divya Indi <divya.indi@oracle.com>
> Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Divya Indi]

Hi Steven,

On 7/29/19 5:51 PM, Steven Rostedt wrote:
> On Mon, 29 Jul 2019 17:02:34 -0700
> Divya Indi <divya.indi@oracle.com> wrote:
>
>> Use "trace_array_set_clr_event" to enable/disable events to a trace
>> array from other kernel modules/components.
>> Hence, we no longer need to have ftrace_set_clr_event as an exported API.
> Hmm, this simply reverts patch 1. Why have patch 1 and this patch in
> the first place?

Right! First patch fixes issues in a previous commit and then this patch 
reverts the part of previous commit that required the fix.

We can eliminate the first patch if it seems counter intuitive.


Thanks,

Divya

>
> -- Steve
>
>> Signed-off-by: Divya Indi <divya.indi@oracle.com>
>> Reviewed-By: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Steven Rostedt]

On Tue, 30 Jul 2019 15:29:41 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> Hi Steven,
> 
> On 7/29/19 5:51 PM, Steven Rostedt wrote:
> > On Mon, 29 Jul 2019 17:02:34 -0700
> > Divya Indi <divya.indi@oracle.com> wrote:
> >  
> >> Use "trace_array_set_clr_event" to enable/disable events to a trace
> >> array from other kernel modules/components.
> >> Hence, we no longer need to have ftrace_set_clr_event as an exported API.  
> > Hmm, this simply reverts patch 1. Why have patch 1 and this patch in
> > the first place?  
> 
> Right! First patch fixes issues in a previous commit and then this patch 
> reverts the part of previous commit that required the fix.
> 
> We can eliminate the first patch if it seems counter intuitive.
> 
> 

As a stand alone patch, the first one may be fine. But as part of a
series, it doesn't make sense to add it.

-- Steve

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Steven Rostedt]

On Fri, 2 Aug 2019 13:41:20 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> > As a stand alone patch, the first one may be fine. But as part of a
> > series, it doesn't make sense to add it.  
> 
> I see. Will separate this out from the series.

Is that really needed? Do you need to have that patch in the kernel?

Do you plan on marking it for stable?

-- Steve

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Divya Indi]

Hi Steve,

The first patch would be like a temporary fix in case we need more 
changes to the patches that add the new function - trace_array_set_clr() 
and unexport ftrace_set_clr_event() and might take some time. In which 
case I think it would be good to have this in place (But, not part of 
this series).

If they all are to go in together as part of the same release ie if all 
is good with the concerned patches (Patch 6 & Patch 7), then I think 
having this patch would be meaningless.


Thanks,

Divya

On 8/2/19 1:46 PM, Steven Rostedt wrote:
> On Fri, 2 Aug 2019 13:41:20 -0700
> Divya Indi <divya.indi@oracle.com> wrote:
>
>>> As a stand alone patch, the first one may be fine. But as part of a
>>> series, it doesn't make sense to add it.
>> I see. Will separate this out from the series.
> Is that really needed? Do you need to have that patch in the kernel?
>
> Do you plan on marking it for stable?
>
> -- Steve

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Steven Rostedt]

On Fri, 2 Aug 2019 14:12:54 -0700
Divya Indi <divya.indi@oracle.com> wrote:

> Hi Steve,
> 
> The first patch would be like a temporary fix in case we need more 
> changes to the patches that add the new function - trace_array_set_clr() 
> and unexport ftrace_set_clr_event() and might take some time. In which 
> case I think it would be good to have this in place (But, not part of 
> this series).
> 
> If they all are to go in together as part of the same release ie if all 
> is good with the concerned patches (Patch 6 & Patch 7), then I think 
> having this patch would be meaningless.

Can you just do this part of patch 6 instead?

+/**
+ * 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);
+

-- Steve

Re: [PATCH 7/7] tracing: Un-export ftrace_set_clr_event

[Divya Indi]

Hi Steven,

On 8/2/19 2:25 PM, Steven Rostedt wrote:
> On Fri, 2 Aug 2019 14:12:54 -0700
> Divya Indi <divya.indi@oracle.com> wrote:
>
>> Hi Steve,
>>
>> The first patch would be like a temporary fix in case we need more
>> changes to the patches that add the new function - trace_array_set_clr()
>> and unexport ftrace_set_clr_event() and might take some time. In which
>> case I think it would be good to have this in place (But, not part of
>> this series).
>>
>> If they all are to go in together as part of the same release ie if all
>> is good with the concerned patches (Patch 6 & Patch 7), then I think
>> having this patch would be meaningless.
> Can you just do this part of patch 6 instead?

Yes, will merge the two ie -

1)  Add 2 new functions - trace_array_set_clr_event(), trace_array_lookup()

2)  Unexport ftrace_set_clr_event.

into a single patch.


Thanks,

Divya

>
> +/**
> + * 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);
> +
>
> -- Steve