| Message ID | 1464779939-24986-3-git-send-email-zhang.chunyan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chunyan Zhang <zhang.chunyan@linaro.org> writes: > This patch is introducing a new function to print Ftrace messages > to STM buffer when the traces happen. In order to reduce the > effect on timing overhead as much as possible, only the current > function and its parent ip address will be recorded into STM in > this patch. This idea was first introduced by Philippe Langlais > at ST-Microelectronics a long time ago[1]. So why is this useful? The value of trace points is in their payload. > +#define STM_FTRACE_CHAN 0 This is why we have the stm master/channel allocation policy, which should already assign a channel to your stm_source device when it is linked to an stm device. Also, why is this a separate compilation unit from the stm_ftrace.o? > + > +void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) > +{ > + unsigned long ip_array[2] = {ip, parent_ip}; > + > + stm_ftrace_write((char *)ip_array, sizeof(unsigned long) * 2, > + STM_FTRACE_CHAN); > +} > +EXPORT_SYMBOL_GPL(ftrace_stm_func); > diff --git a/kernel/trace/trace_output_stm.h b/kernel/trace/trace_output_stm.h > new file mode 100644 > index 0000000..fc3f989 > --- /dev/null > +++ b/kernel/trace/trace_output_stm.h > @@ -0,0 +1,14 @@ > +#ifndef __TRACE_OUTPUT_STM_H > +#define __TRACE_OUTPUT_STM_H > + > +#include <linux/module.h> > + > +#ifdef CONFIG_STM_FTRACE > +extern void stm_ftrace_write(const char *buf, unsigned int len, > + unsigned int chan); > +extern void ftrace_stm_func(unsigned long ip, unsigned long parent_ip); > +#else > +static inline void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) {} > +#endif > + > +#endif /* __TRACE_OUTPUT_STM_H */ > -- > 1.9.1
On Tue, Jun 7, 2016 at 6:04 PM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> This patch is introducing a new function to print Ftrace messages >> to STM buffer when the traces happen. In order to reduce the >> effect on timing overhead as much as possible, only the current >> function and its parent ip address will be recorded into STM in >> this patch. This idea was first introduced by Philippe Langlais >> at ST-Microelectronics a long time ago[1]. > > So why is this useful? The value of trace points is in their payload. Sorry, didn't get you, what did you mean by "The value of trace points" here? > >> +#define STM_FTRACE_CHAN 0 > > This is why we have the stm master/channel allocation policy, which > should already assign a channel to your stm_source device when it is > linked to an stm device. If I understand correctly, STM core can assign many channels for one stm_source, the number we want to be allocated is specified by 'stm_source_data.nr_chans', for this patchset it's 1 for the time being, STM_FTRACE_CHAN is the index in the range of channels allocated to this stm_source by STM core. > > Also, why is this a separate compilation unit from the stm_ftrace.o? My idea was stm_ftrace.c is part of STM driver, this patch is a process of traces from Ftrace subsystem. These two parts are small so far, since this serial only added function trace exporting to STM, but I want to add more functionalities to export more kinds of traces which Ftrace subsystem supports to STM in the feature, my thought is dividing into two parts like this serial did is convenient to add other feature to each part. > >> + >> +void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) >> +{ >> + unsigned long ip_array[2] = {ip, parent_ip}; >> + >> + stm_ftrace_write((char *)ip_array, sizeof(unsigned long) * 2, >> + STM_FTRACE_CHAN); >> +} >> +EXPORT_SYMBOL_GPL(ftrace_stm_func); >> diff --git a/kernel/trace/trace_output_stm.h b/kernel/trace/trace_output_stm.h >> new file mode 100644 >> index 0000000..fc3f989 >> --- /dev/null >> +++ b/kernel/trace/trace_output_stm.h >> @@ -0,0 +1,14 @@ >> +#ifndef __TRACE_OUTPUT_STM_H >> +#define __TRACE_OUTPUT_STM_H >> + >> +#include <linux/module.h> >> + >> +#ifdef CONFIG_STM_FTRACE >> +extern void stm_ftrace_write(const char *buf, unsigned int len, >> + unsigned int chan); >> +extern void ftrace_stm_func(unsigned long ip, unsigned long parent_ip); >> +#else >> +static inline void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) {} >> +#endif >> + >> +#endif /* __TRACE_OUTPUT_STM_H */ >> -- >> 1.9.1
Chunyan Zhang <zhang.chunyan@linaro.org> writes: > On Tue, Jun 7, 2016 at 6:04 PM, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >> >>> This patch is introducing a new function to print Ftrace messages >>> to STM buffer when the traces happen. In order to reduce the >>> effect on timing overhead as much as possible, only the current >>> function and its parent ip address will be recorded into STM in >>> this patch. This idea was first introduced by Philippe Langlais >>> at ST-Microelectronics a long time ago[1]. >> >> So why is this useful? The value of trace points is in their payload. > > Sorry, didn't get you, what did you mean by "The value of trace points" here? I suggest that you read about ftrace usage and in particular the types of records that end up in its ring buffer. > >> >>> +#define STM_FTRACE_CHAN 0 >> >> This is why we have the stm master/channel allocation policy, which >> should already assign a channel to your stm_source device when it is >> linked to an stm device. > > If I understand correctly, STM core can assign many channels for one > stm_source, the number we want to be allocated is specified by > 'stm_source_data.nr_chans', for this patchset it's 1 for the time > being, STM_FTRACE_CHAN is the index in the range of channels allocated > to this stm_source by STM core. Yes, I misread the code, apologies. >> Also, why is this a separate compilation unit from the stm_ftrace.o? > > My idea was stm_ftrace.c is part of STM driver, this patch is a > process of traces from Ftrace subsystem. These two parts are small so > far, since this serial only added function trace exporting to STM, but > I want to add more functionalities to export more kinds of traces > which Ftrace subsystem supports to STM in the feature, my thought is > dividing into two parts like this serial did is convenient to add > other feature to each part. Leaving aside the question of different ftrace functionalities for a moment, what is the benefit of splitting stm_ftrace like this? The only purpose of stm_ftrace is to take data from ftrace and send it down to an stm device, using necessary intefaces on both ends and performing additional framing if required. If would make sense if the ftrace part gets a notion of "output" or "sink" interface, which stm_ftrace can declare itself to be an implementation of. Regards, -- Alex
On Tue, Jun 7, 2016 at 6:04 PM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> This patch is introducing a new function to print Ftrace messages >> to STM buffer when the traces happen. In order to reduce the >> effect on timing overhead as much as possible, only the current >> function and its parent ip address will be recorded into STM in >> this patch. This idea was first introduced by Philippe Langlais >> at ST-Microelectronics a long time ago[1]. > > So why is this useful? The value of trace points is in their payload. From what I understand, we have no way to log as much information as Ftrace has done if we want to control the overhead of writing STM in a limited/acceptable range. Exporting information to STM is much more time-consuming than writing ring buffer, the thought was simply that export as much useful information as possible while taking a limited bytes. Thanks, Chunyan > >> +#define STM_FTRACE_CHAN 0 > > This is why we have the stm master/channel allocation policy, which > should already assign a channel to your stm_source device when it is > linked to an stm device. > > Also, why is this a separate compilation unit from the stm_ftrace.o? > >> + >> +void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) >> +{ >> + unsigned long ip_array[2] = {ip, parent_ip}; >> + >> + stm_ftrace_write((char *)ip_array, sizeof(unsigned long) * 2, >> + STM_FTRACE_CHAN); >> +} >> +EXPORT_SYMBOL_GPL(ftrace_stm_func); >> diff --git a/kernel/trace/trace_output_stm.h b/kernel/trace/trace_output_stm.h >> new file mode 100644 >> index 0000000..fc3f989 >> --- /dev/null >> +++ b/kernel/trace/trace_output_stm.h >> @@ -0,0 +1,14 @@ >> +#ifndef __TRACE_OUTPUT_STM_H >> +#define __TRACE_OUTPUT_STM_H >> + >> +#include <linux/module.h> >> + >> +#ifdef CONFIG_STM_FTRACE >> +extern void stm_ftrace_write(const char *buf, unsigned int len, >> + unsigned int chan); >> +extern void ftrace_stm_func(unsigned long ip, unsigned long parent_ip); >> +#else >> +static inline void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) {} >> +#endif >> + >> +#endif /* __TRACE_OUTPUT_STM_H */ >> -- >> 1.9.1
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile index 979e7bf..445afa8 100644 --- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -69,4 +69,5 @@ obj-$(CONFIG_UPROBE_EVENT) += trace_uprobe.o obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o +obj-$(CONFIG_STM_FTRACE) += trace_output_stm.o libftrace-y := ftrace.o diff --git a/kernel/trace/trace_output_stm.c b/kernel/trace/trace_output_stm.c new file mode 100644 index 0000000..f6a3c01 --- /dev/null +++ b/kernel/trace/trace_output_stm.c @@ -0,0 +1,27 @@ +/* + * Output interface from Ftrace to STM buffer + * Copyright (c) 2016, Linaro Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include "trace_output_stm.h" + +/* Offset above the start channel number */ +#define STM_FTRACE_CHAN 0 + +void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) +{ + unsigned long ip_array[2] = {ip, parent_ip}; + + stm_ftrace_write((char *)ip_array, sizeof(unsigned long) * 2, + STM_FTRACE_CHAN); +} +EXPORT_SYMBOL_GPL(ftrace_stm_func); diff --git a/kernel/trace/trace_output_stm.h b/kernel/trace/trace_output_stm.h new file mode 100644 index 0000000..fc3f989 --- /dev/null +++ b/kernel/trace/trace_output_stm.h @@ -0,0 +1,14 @@ +#ifndef __TRACE_OUTPUT_STM_H +#define __TRACE_OUTPUT_STM_H + +#include <linux/module.h> + +#ifdef CONFIG_STM_FTRACE +extern void stm_ftrace_write(const char *buf, unsigned int len, + unsigned int chan); +extern void ftrace_stm_func(unsigned long ip, unsigned long parent_ip); +#else +static inline void ftrace_stm_func(unsigned long ip, unsigned long parent_ip) {} +#endif + +#endif /* __TRACE_OUTPUT_STM_H */
This patch is introducing a new function to print Ftrace messages to STM buffer when the traces happen. In order to reduce the effect on timing overhead as much as possible, only the current function and its parent ip address will be recorded into STM in this patch. This idea was first introduced by Philippe Langlais at ST-Microelectronics a long time ago[1]. [1] http://thread.gmane.org/gmane.linux.kernel/1114274/focus=1114275 Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- kernel/trace/Makefile | 1 + kernel/trace/trace_output_stm.c | 27 +++++++++++++++++++++++++++ kernel/trace/trace_output_stm.h | 14 ++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 kernel/trace/trace_output_stm.c create mode 100644 kernel/trace/trace_output_stm.h