Re: [PATCH] tracing: Kernel access to Ftrace instances
On 05/20/2019 08:23 AM, Steven Rostedt wrote: On Wed, 15 May 2019 20:24:43 -0700 Steven Rostedt wrote: It seems your's already in Steve's ftrace/core branch, so I think you can make additional patch to fix it. Steve, is that OK? Yes. In fact I already sent a pull request to Linus. Please send a patch on top of my ftrace/core branch. And now it is in mainline (v5.2-rc1). Please send a patch with a sample module (as Masami requested). Also, that function not only needs to be changed to not being static, you need to add it to a header (include/linux/trace_events.h?) Thanks! Working on it. Will send it out soon. Thanks, Divya -- Steve
Re: [PATCH] tracing: Kernel access to Ftrace instances
On Wed, 15 May 2019 20:24:43 -0700 Steven Rostedt wrote: > > >It seems your's already in Steve's ftrace/core branch, so I think you > >can make > >additional patch to fix it. Steve, is that OK? > > > > Yes. In fact I already sent a pull request to Linus. Please send a patch on > top of my ftrace/core branch. > And now it is in mainline (v5.2-rc1). Please send a patch with a sample module (as Masami requested). Also, that function not only needs to be changed to not being static, you need to add it to a header (include/linux/trace_events.h?) Thanks! -- Steve
Re: [PATCH] tracing: Kernel access to Ftrace instances
On May 15, 2019 8:05:29 PM PDT, Masami Hiramatsu wrote: > >> >> diff --git a/kernel/trace/trace_events.c >b/kernel/trace/trace_events.c >> >> index 5b3b0c3..81c038e 100644 >> >> --- a/kernel/trace/trace_events.c >> >> +++ b/kernel/trace/trace_events.c >> >> @@ -832,6 +832,7 @@ static int ftrace_set_clr_event(struct >trace_array *tr, char *buf, int set) >> >> >> >> return ret; >> >> } >> >> +EXPORT_SYMBOL_GPL(ftrace_set_clr_event); >> > I found this exports a static function to module. Did it work? >> >> I had tested the changes with my module. This change to static was >added >> in the test patch, but somehow missed it in the final patch that was >> sent out. > >If you can send some example module patch under samples/, that is more >helpful for us to check it. And it is possible to use in kselftest too. > >> >> Will send a new patch along with a few additional ones to add some >NULL >> checks to ensure safe usage by modules and add the APIs to a header >file >> that can be used by the modules. > >It seems your's already in Steve's ftrace/core branch, so I think you >can make >additional patch to fix it. Steve, is that OK? > Yes. In fact I already sent a pull request to Linus. Please send a patch on top of my ftrace/core branch. Thanks, -- Steve >Thank you, > >> >> Thanks, >> >> Divya >> >> > >> > Thank you, >> > -- Sent from my Android device with K-9 Mail. Please excuse my brevity and top posting.
Re: [PATCH] tracing: Kernel access to Ftrace instances
Hi, On Wed, 15 May 2019 18:51:23 -0700 Divya Indi wrote: > Hi Masami, > > Thanks for pointing it out. > > Yes, it should not be static. > > On 5/15/19 5:09 PM, Masami Hiramatsu wrote: > > HI Divya, > > > > On Wed, 20 Mar 2019 11:28:51 -0700 > > Divya Indi wrote: > > > >> Ftrace provides the feature “instances” that provides the capability to > >> create multiple Ftrace ring buffers. However, currently these buffers > >> are created/accessed via userspace only. The kernel APIs providing these > >> features are not exported, hence cannot be used by other kernel > >> components. > >> > >> This patch aims to extend this infrastructure to provide the > >> flexibility to create/log/remove/ enable-disable existing trace events > >> to these buffers from within the kernel. > >> > >> Signed-off-by: Divya Indi > >> Reviewed-by: Joe Jin > > Would you tested these APIs with your module? Since, > > [...] > >> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > >> index 5b3b0c3..81c038e 100644 > >> --- a/kernel/trace/trace_events.c > >> +++ b/kernel/trace/trace_events.c > >> @@ -832,6 +832,7 @@ static int ftrace_set_clr_event(struct trace_array > >> *tr, char *buf, int set) > >> > >>return ret; > >> } > >> +EXPORT_SYMBOL_GPL(ftrace_set_clr_event); > > I found this exports a static function to module. Did it work? > > I had tested the changes with my module. This change to static was added > in the test patch, but somehow missed it in the final patch that was > sent out. If you can send some example module patch under samples/, that is more helpful for us to check it. And it is possible to use in kselftest too. > > Will send a new patch along with a few additional ones to add some NULL > checks to ensure safe usage by modules and add the APIs to a header file > that can be used by the modules. It seems your's already in Steve's ftrace/core branch, so I think you can make additional patch to fix it. Steve, is that OK? Thank you, > > Thanks, > > Divya > > > > > Thank you, > > -- Masami Hiramatsu
Re: [PATCH] tracing: Kernel access to Ftrace instances
Hi Masami, Thanks for pointing it out. Yes, it should not be static. On 5/15/19 5:09 PM, Masami Hiramatsu wrote: HI Divya, On Wed, 20 Mar 2019 11:28:51 -0700 Divya Indi wrote: Ftrace provides the feature “instances” that provides the capability to create multiple Ftrace ring buffers. However, currently these buffers are created/accessed via userspace only. The kernel APIs providing these features are not exported, hence cannot be used by other kernel components. This patch aims to extend this infrastructure to provide the flexibility to create/log/remove/ enable-disable existing trace events to these buffers from within the kernel. Signed-off-by: Divya Indi Reviewed-by: Joe Jin Would you tested these APIs with your module? Since, [...] diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index 5b3b0c3..81c038e 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -832,6 +832,7 @@ static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set) return ret; } +EXPORT_SYMBOL_GPL(ftrace_set_clr_event); I found this exports a static function to module. Did it work? I had tested the changes with my module. This change to static was added in the test patch, but somehow missed it in the final patch that was sent out. Will send a new patch along with a few additional ones to add some NULL checks to ensure safe usage by modules and add the APIs to a header file that can be used by the modules. Thanks, Divya Thank you,
Re: [PATCH] tracing: Kernel access to Ftrace instances
HI Divya, On Wed, 20 Mar 2019 11:28:51 -0700 Divya Indi wrote: > Ftrace provides the feature “instances” that provides the capability to > create multiple Ftrace ring buffers. However, currently these buffers > are created/accessed via userspace only. The kernel APIs providing these > features are not exported, hence cannot be used by other kernel > components. > > This patch aims to extend this infrastructure to provide the > flexibility to create/log/remove/ enable-disable existing trace events > to these buffers from within the kernel. > > Signed-off-by: Divya Indi > Reviewed-by: Joe Jin Would you tested these APIs with your module? Since, [...] > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > index 5b3b0c3..81c038e 100644 > --- a/kernel/trace/trace_events.c > +++ b/kernel/trace/trace_events.c > @@ -832,6 +832,7 @@ static int ftrace_set_clr_event(struct trace_array *tr, > char *buf, int set) > > return ret; > } > +EXPORT_SYMBOL_GPL(ftrace_set_clr_event); I found this exports a static function to module. Did it work? Thank you, -- Masami Hiramatsu
Re: [PATCH] tracing: Kernel access to Ftrace instances
On Wed, 20 Mar 2019 11:28:51 -0700 Divya Indi wrote: > Ftrace provides the feature “instances” that provides the capability to > create multiple Ftrace ring buffers. However, currently these buffers > are created/accessed via userspace only. The kernel APIs providing these > features are not exported, hence cannot be used by other kernel > components. > > This patch aims to extend this infrastructure to provide the > flexibility to create/log/remove/ enable-disable existing trace events > to these buffers from within the kernel. > > Signed-off-by: Divya Indi > Reviewed-by: Joe Jin > --- I added this to my queue, for 5.2 and then we'll see what it will do. -- Steve
[PATCH] tracing: Kernel access to Ftrace instances
Ftrace provides the feature “instances” that provides the capability to
create multiple Ftrace ring buffers. However, currently these buffers
are created/accessed via userspace only. The kernel APIs providing these
features are not exported, hence cannot be used by other kernel
components.
This patch aims to extend this infrastructure to provide the
flexibility to create/log/remove/ enable-disable existing trace events
to these buffers from within the kernel.
Signed-off-by: Divya Indi
Reviewed-by: Joe Jin
---
kernel/trace/trace.c| 74 ++---
kernel/trace/trace_events.c | 1 +
2 files changed, 51 insertions(+), 24 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c4238b4..eaf163a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2878,6 +2878,7 @@ void trace_printk_init_buffers(void)
if (global_trace.trace_buffer.buffer)
tracing_start_cmdline_record();
}
+EXPORT_SYMBOL_GPL(trace_printk_init_buffers);
void trace_printk_start_comm(void)
{
@@ -3038,6 +3039,7 @@ int trace_array_printk(struct trace_array *tr,
va_end(ap);
return ret;
}
+EXPORT_SYMBOL_GPL(trace_array_printk);
__printf(3, 4)
int trace_array_printk_buf(struct ring_buffer *buffer,
@@ -7832,7 +7834,7 @@ static void update_tracer_options(struct trace_array *tr)
mutex_unlock(_types_lock);
}
-static int instance_mkdir(const char *name)
+struct trace_array *trace_array_create(const char *name)
{
struct trace_array *tr;
int ret;
@@ -7896,7 +7898,7 @@ static int instance_mkdir(const char *name)
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
- return 0;
+ return tr;
out_free_tr:
free_trace_buffers(tr);
@@ -7908,33 +7910,21 @@ static int instance_mkdir(const char *name)
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
- return ret;
+ 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));
}
-static int instance_rmdir(const char *name)
+static int __remove_instance(struct trace_array *tr)
{
- struct trace_array *tr;
- int found = 0;
- int ret;
int i;
- mutex_lock(_mutex);
- mutex_lock(_types_lock);
-
- ret = -ENODEV;
- list_for_each_entry(tr, _trace_arrays, list) {
- if (tr->name && strcmp(tr->name, name) == 0) {
- found = 1;
- break;
- }
- }
- if (!found)
- goto out_unlock;
-
- ret = -EBUSY;
if (tr->ref || (tr->current_trace && tr->current_trace->ref))
- goto out_unlock;
+ return -EBUSY;
list_del(>list);
@@ -7960,10 +7950,46 @@ static int instance_rmdir(const char *name)
free_cpumask_var(tr->tracing_cpumask);
kfree(tr->name);
kfree(tr);
+ tr = NULL;
- ret = 0;
+ return 0;
+}
+
+int trace_array_destroy(struct trace_array *tr)
+{
+ int ret;
+
+ if (!tr)
+ return -EINVAL;
+
+ mutex_lock(_mutex);
+ mutex_lock(_types_lock);
+
+ ret = __remove_instance(tr);
+
+ mutex_unlock(_types_lock);
+ mutex_unlock(_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(trace_array_destroy);
+
+static int instance_rmdir(const char *name)
+{
+ struct trace_array *tr;
+ int ret;
+
+ mutex_lock(_mutex);
+ mutex_lock(_types_lock);
+
+ ret = -ENODEV;
+ list_for_each_entry(tr, _trace_arrays, list) {
+ if (tr->name && strcmp(tr->name, name) == 0) {
+ ret = __remove_instance(tr);
+ break;
+ }
+ }
- out_unlock:
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 5b3b0c3..81c038e 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -832,6 +832,7 @@ 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
--
1.8.3.1
Re: [PATCH] tracing: Kernel access to ftrace instances
Sure, thanks for taking the time to review. There are a few issues with the patch styling and some minor modifications. I will shortly send the v2 for the same. Regards, Divya On 03/19/2019 01:16 PM, Steven Rostedt wrote: On Fri, 15 Mar 2019 16:35:25 -0700 Divya Indi wrote: Ftrace provides the feature “instances” that provides the capability to create multiple Ftrace ring buffers. However, currently these buffers are created/accessed via userspace only. The kernel APIs providing these features are not exported, hence cannot be used by other kernel components. This patch aims to extend this infrastructure to provide the flexibility to create/log/remove/ enable-disable existing trace events to these buffers from within the kernel. Thanks for sending this. I'm currently working on some other changes, but will look in this when I'm finished with the other work. Should be within this week. -- Steve
Re: [PATCH] tracing: Kernel access to ftrace instances
On Fri, 15 Mar 2019 16:35:25 -0700 Divya Indi wrote: > Ftrace provides the feature “instances” that provides the capability to > create multiple Ftrace ring buffers. However, currently these buffers > are created/accessed via userspace only. The kernel APIs providing these > features are not exported, hence cannot be used by other kernel > components. > > This patch aims to extend this infrastructure to provide the > flexibility to create/log/remove/ enable-disable existing trace events > to these buffers from within the kernel. > > Thanks for sending this. I'm currently working on some other changes, but will look in this when I'm finished with the other work. Should be within this week. -- Steve
[PATCH] tracing: Kernel access to ftrace instances
Ftrace provides the feature “instances” that provides the capability to
create multiple Ftrace ring buffers. However, currently these buffers
are created/accessed via userspace only. The kernel APIs providing these
features are not exported, hence cannot be used by other kernel
components.
This patch aims to extend this infrastructure to provide the
flexibility to create/log/remove/ enable-disable existing trace events
to these buffers from within the kernel.
Signed-off-by: Divya Indi
---
kernel/trace/trace.c| 72 +
kernel/trace/trace_events.c | 1 +
2 files changed, 48 insertions(+), 25 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c4238b4..a5e7e51 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2878,6 +2878,7 @@ void trace_printk_init_buffers(void)
if (global_trace.trace_buffer.buffer)
tracing_start_cmdline_record();
}
+EXPORT_SYMBOL_GPL(trace_printk_init_buffers);
void trace_printk_start_comm(void)
{
@@ -3038,6 +3039,7 @@ int trace_array_printk(struct trace_array *tr,
va_end(ap);
return ret;
}
+EXPORT_SYMBOL_GPL(trace_array_printk);
__printf(3, 4)
int trace_array_printk_buf(struct ring_buffer *buffer,
@@ -7832,7 +7834,7 @@ static void update_tracer_options(struct trace_array *tr)
mutex_unlock(_types_lock);
}
-static int instance_mkdir(const char *name)
+struct trace_array *trace_array_create(const char *name)
{
struct trace_array *tr;
int ret;
@@ -7896,7 +7898,7 @@ static int instance_mkdir(const char *name)
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
- return 0;
+ return tr;
out_free_tr:
free_trace_buffers(tr);
@@ -7908,33 +7910,21 @@ static int instance_mkdir(const char *name)
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
- return ret;
+ 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));
}
-static int instance_rmdir(const char *name)
+static int __remove_instance(struct trace_array *tr)
{
- struct trace_array *tr;
- int found = 0;
- int ret;
int i;
-
- mutex_lock(_mutex);
- mutex_lock(_types_lock);
-
- ret = -ENODEV;
- list_for_each_entry(tr, _trace_arrays, list) {
- if (tr->name && strcmp(tr->name, name) == 0) {
- found = 1;
- break;
- }
- }
- if (!found)
- goto out_unlock;
-
- ret = -EBUSY;
+
if (tr->ref || (tr->current_trace && tr->current_trace->ref))
- goto out_unlock;
+ return -EBUSY;
list_del(>list);
@@ -7961,9 +7951,41 @@ static int instance_rmdir(const char *name)
kfree(tr->name);
kfree(tr);
- ret = 0;
+ return 0;
+}
- out_unlock:
+int trace_array_destroy(struct trace_array *tr)
+{
+ int ret;
+
+ mutex_lock(_mutex);
+ mutex_lock(_types_lock);
+
+ ret = __remove_instance(tr);
+
+ mutex_unlock(_types_lock);
+ mutex_unlock(_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(trace_array_destroy);
+
+static int instance_rmdir(const char *name)
+{
+ struct trace_array *tr;
+ int ret;
+
+ mutex_lock(_mutex);
+ mutex_lock(_types_lock);
+
+ ret = -ENODEV;
+ list_for_each_entry(tr, _trace_arrays, list) {
+ if (tr->name && strcmp(tr->name, name) == 0) {
+ ret = __remove_instance(tr);
+ break;
+ }
+ }
+
mutex_unlock(_types_lock);
mutex_unlock(_mutex);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 5b3b0c3..81c038e 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -832,6 +832,7 @@ 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
--
1.8.3.1
