Ability for classifying of measurements

Ability for classifying of measurements

[Leonardo Boquillon]

Hi,

When profiling an application, grouping measurements according to specific
criteria can provide valuable information for the developer. For instance, 
suppose there is a variable in the application whose value represents the
current state of an internal state machine. We might want to classify the
sampled data depending on the current state. The decision of what constitutes an
adequate classification criteria depends on the application developer. 

The idea proposed here is to have a mechanism to let perf_events know the data
used to annotate the samples, either via a callback implemented by the 
application developer or by the application itself telling perf_events the data
used to classify the measurements.

The sample annotation would be a struct, whose size is specified before samples 
are recorded.

As an experiment, this patch implements the second approach, where the
annotation is a 64 bit integer that is located at a fixed memory address. The
samples are annotated using the value at that address. The value is assumed to
be in a fixed location, specified by the application being profiled through a 
new perf_ioctl subcommand.

In no way we intend this patch to be committed; we just want to show the idea 
and start a discussion instead, focusing on the idea first rather than on the 
design & code.

Thanks

---
 include/uapi/linux/perf_event.h |  1 +
 kernel/events/core.c            | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1afe962..f4f2a32 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -401,6 +401,7 @@ struct perf_event_attr {
 #define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_SET_WATCHED  _IOW('$', 9, __u64 *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 52bedc5..cf148b8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -49,6 +49,8 @@
 
 #include <asm/irq_regs.h>
 
+static __u64 *watched_data;
+
 typedef int (*remote_function_f)(void *);
 
 struct remote_function_call {
@@ -4079,10 +4081,12 @@ static int perf_read_one(struct perf_event *event,
 				 u64 read_format, char __user *buf)
 {
 	u64 enabled, running;
-	u64 values[4];
+	u64 values[5];
 	int n = 0;
 
 	values[n++] = perf_event_read_value(event, &enabled, &running);
+	if (watched_data != NULL)
+		values[n++] = *((__u64 *)watched_data);
 	if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
 		values[n++] = enabled;
 	if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
@@ -4295,6 +4299,7 @@ static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_event_set_watched(__u64* watched);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -4351,6 +4356,9 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 	case PERF_EVENT_IOC_SET_BPF:
 		return perf_event_set_bpf_prog(event, arg);
 
+	case PERF_EVENT_IOC_SET_WATCHED:
+		return perf_event_set_watched((void __user *)arg);
+
 	default:
 		return -ENOTTY;
 	}
@@ -4628,6 +4636,12 @@ static void ring_buffer_wakeup(struct perf_event *event)
 	rcu_read_unlock();
 }
 
+static int perf_event_set_watched(__u64 *watched)
+{
+	watched_data = watched;
+	return 0;
+}
+
 struct ring_buffer *ring_buffer_get(struct perf_event *event)
 {
 	struct ring_buffer *rb;
-- 
1.9.1

Re: Ability for classifying of measurements

[Andi Kleen]

Leonardo Boquillon <leonardo.boquillon@tallertechnologies.com> writes:
>
> In no way we intend this patch to be committed; we just want to show the idea 
> and start a discussion instead, focusing on the idea first rather than on the 
> design & code.

That's good because the patch as written is a big security hole (and in
fact will not work on any systems with SMAP enabled)

The basic concept seems reasonable, although a single address may be
too inflexible.

Often the main request I hear is for a simple way for an application
to turn on/off profiling, and perhaps add markers to distinguish
phases. It many cases that may be good enough.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: Ability for classifying of measurements

[Daniel Gutson]

(2nd resend due to Majordomo rejection, this time as plain text..sorry
for inconveniences)


On Wed, Apr 20, 2016 at 10:26 AM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
>
> El 20/4/2016 10:08, "Andi Kleen" <andi@firstfloor.org> escribió:
>
 Hi Andi.
>
>>
>> Leonardo Boquillon <leonardo.boquillon@tallertechnologies.com> writes:
>> >
>> > In no way we intend this patch to be committed; we just want to show the
>> > idea
>> > and start a discussion instead, focusing on the idea first rather than
>> > on the
>> > design & code.
>>
>> That's good because the patch as written is a big security hole (and in
>> fact will not work on any systems with SMAP enabled)
>>
>> The basic concept seems reasonable, although a single address may be
>> too inflexible.
>
 Agree, this was the first step. What we would like to see implemented from
 this feature is, 1) some dwarf-aided ability to specify watched variables in
 the perf record command (e.g. perf record --groupby="::var1,
class1::staticvar2" ),
and 2) provide an API to the applications so they can
 communicate with perf_events telling what's going on, such as the example we
 are showing here (which is *just* an example to fuel this discussion). This
 API would lead to intrusively mess the code of the app but not more than
 people already do with debugging printfs; this would be very important for
 performance-centric applications.

 So, is there some consensus to start defining the API?
>
>>
>> Often the main request I hear is for a simple way for an application
>> to turn on/off profiling, and perhaps add markers to distinguish
>> phases. It many cases that may be good enough.
>>
>> -Andi
>>
>> --
>> ak@linux.intel.com -- Speaking for myself only



-- 

Daniel F. Gutson
Engineering Manager

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: Ability for classifying of measurements\

[Andi Kleen]

>  Agree, this was the first step. What we would like to see implemented from
>  this feature is, 1) some dwarf-aided ability to specify watched variables in
>  the perf record command (e.g. perf record --groupby="::var1,
> class1::staticvar2" ),
> and 2) provide an API to the applications so they can
>  communicate with perf_events telling what's going on, such as the example we
>  are showing here (which is *just* an example to fuel this discussion). This
>  API would lead to intrusively mess the code of the app but not more than
>  people already do with debugging printfs; this would be very important for
>  performance-centric applications.

perf probe has an ability to reference variables relative
to a register or in a register, and parse dwarf for it. 

I'm not sure it's a good idea to have a lot of arbitrary memory
accesses in the PMI handler (although there is already some for stack
walking)

One problem with global variables is that they can move
around with ASLR, so it can be difficult to find them.

If the variable is in a register you can already get it from the PEBS
record on Intel CPUs.  There is no good tooling for it currently,
but the information is in perf script output, and can be enabled
for perf record.

An easy way to implement it would be to use a
global register variable (register asm("...") ...).
However you would need to rebuild all your libraries to preserve
the global register.

Best would be to use an unused register. One that comes to
mind on x86-64bit Linux is GS, which is normally only used by the kernel.

With this pending patchkit[1] the application can write to it
using WRGSBASE, previously it needed a syscall.

Currently perf cannot log GS I believe, but that would be fairly
easy to add. Would also need some tooling to make the value
an extra sort key in perf report.

-Andi

Re: Ability for classifying of measurements\

[Daniel Gutson]

On Wed, Apr 20, 2016 at 2:47 PM, Andi Kleen <andi@firstfloor.org> wrote:
>>  Agree, this was the first step. What we would like to see implemented from
>>  this feature is, 1) some dwarf-aided ability to specify watched variables in
>>  the perf record command (e.g. perf record --groupby="::var1,
>> class1::staticvar2" ),
>> and 2) provide an API to the applications so they can
>>  communicate with perf_events telling what's going on, such as the example we
>>  are showing here (which is *just* an example to fuel this discussion). This
>>  API would lead to intrusively mess the code of the app but not more than
>>  people already do with debugging printfs; this would be very important for
>>  performance-centric applications.
>
> perf probe has an ability to reference variables relative
> to a register or in a register, and parse dwarf for it.
>
> I'm not sure it's a good idea to have a lot of arbitrary memory
> accesses in the PMI handler (although there is already some for stack
> walking)
>
> One problem with global variables is that they can move
> around with ASLR, so it can be difficult to find them.
>
> If the variable is in a register you can already get it from the PEBS
> record on Intel CPUs.  There is no good tooling for it currently,
> but the information is in perf script output, and can be enabled
> for perf record.
>
> An easy way to implement it would be to use a
> global register variable (register asm("...") ...).
> However you would need to rebuild all your libraries to preserve
> the global register.
>
> Best would be to use an unused register. One that comes to
> mind on x86-64bit Linux is GS, which is normally only used by the kernel.

I kept thinking about all this, and I changed my mind towards a
simpler (a first step) yet useful approach.
In the same way people spread some printfs for old school debugging, I
keep the idea of some perf API made available
for the application (which calls depend on some flag, say -DNPERF
similar to -DNDEBUG, so if the macro is
not defined, calls have no effects).
The first relatively easy (again, but yet useful) feature that comes
to my mind, is the ability (from the app) to
label phases of the perf recording, including enabling, disabling, and
labelling.
For example, let's suppose that an application has an initialization
phase, then a communication with a server,
then data gathering, then processing, then storing the processed
results. Currently, all the recording and reporting would mix all that
measured information, whereas we could be interested in some phases
only (or disabling the rest).

Thus, the API could have:

perf_enable();
perf_disable();
perf_label(const char* label);

So, the application would appropriately call those functions; at
reporting, both a summary and a breakdown of analysis would be
performed.
A quick hack that comes to my mind is that perf_label just changes the
destination file of perf record, closing the previous file and opening
(potentially appending)
the specified in the argument. This way I think perf report could be
left unmodified, so the user would just do perf report
[labelled_file].

The communication from the application to perf_events (when enabled
with e.g. compiled wih NPERF) could be through some /proc/perf or any
other mechanism.

>
> With this pending patchkit[1] the application can write to it
> using WRGSBASE, previously it needed a syscall.

-ENOLINK

>
> Currently perf cannot log GS I believe, but that would be fairly
> easy to add. Would also need some tooling to make the value
> an extra sort key in perf report.
>
> -Andi
>

Thanks Andi.

   Daniel.


-- 

Daniel F. Gutson
Engineering Manager

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: Ability for classifying of measurements\

[Daniel Gutson]

On Wed, May 4, 2016 at 2:20 PM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> On Wed, Apr 20, 2016 at 2:47 PM, Andi Kleen <andi@firstfloor.org> wrote:
>>>  Agree, this was the first step. What we would like to see implemented from
>>>  this feature is, 1) some dwarf-aided ability to specify watched variables in
>>>  the perf record command (e.g. perf record --groupby="::var1,
>>> class1::staticvar2" ),
>>> and 2) provide an API to the applications so they can
>>>  communicate with perf_events telling what's going on, such as the example we
>>>  are showing here (which is *just* an example to fuel this discussion). This
>>>  API would lead to intrusively mess the code of the app but not more than
>>>  people already do with debugging printfs; this would be very important for
>>>  performance-centric applications.
>>
>> perf probe has an ability to reference variables relative
>> to a register or in a register, and parse dwarf for it.
>>
>> I'm not sure it's a good idea to have a lot of arbitrary memory
>> accesses in the PMI handler (although there is already some for stack
>> walking)
>>
>> One problem with global variables is that they can move
>> around with ASLR, so it can be difficult to find them.
>>
>> If the variable is in a register you can already get it from the PEBS
>> record on Intel CPUs.  There is no good tooling for it currently,
>> but the information is in perf script output, and can be enabled
>> for perf record.
>>
>> An easy way to implement it would be to use a
>> global register variable (register asm("...") ...).
>> However you would need to rebuild all your libraries to preserve
>> the global register.
>>
>> Best would be to use an unused register. One that comes to
>> mind on x86-64bit Linux is GS, which is normally only used by the kernel.
>
> I kept thinking about all this, and I changed my mind towards a
> simpler (a first step) yet useful approach.
> In the same way people spread some printfs for old school debugging, I
> keep the idea of some perf API made available
> for the application (which calls depend on some flag, say -DNPERF
> similar to -DNDEBUG, so if the macro is
> not defined, calls have no effects).
> The first relatively easy (again, but yet useful) feature that comes
> to my mind, is the ability (from the app) to
> label phases of the perf recording, including enabling, disabling, and
> labelling.
> For example, let's suppose that an application has an initialization
> phase, then a communication with a server,
> then data gathering, then processing, then storing the processed
> results. Currently, all the recording and reporting would mix all that
> measured information, whereas we could be interested in some phases
> only (or disabling the rest).
>
> Thus, the API could have:
>
> perf_enable();
> perf_disable();
> perf_label(const char* label);
>
> So, the application would appropriately call those functions; at
> reporting, both a summary and a breakdown of analysis would be
> performed.
> A quick hack that comes to my mind is that perf_label just changes the
> destination file of perf record, closing the previous file and opening
> (potentially appending)
> the specified in the argument. This way I think perf report could be
> left unmodified, so the user would just do perf report
> [labelled_file].
>
> The communication from the application to perf_events (when enabled
> with e.g. compiled wih NPERF) could be through some /proc/perf or any
> other mechanism.

One more thing to add: I think this API would actually communicate
with the perf user space tool rather than with the kernel perf_events.


>
>>
>> With this pending patchkit[1] the application can write to it
>> using WRGSBASE, previously it needed a syscall.
>
> -ENOLINK
>
>>
>> Currently perf cannot log GS I believe, but that would be fairly
>> easy to add. Would also need some tooling to make the value
>> an extra sort key in perf report.
>>
>> -Andi
>>
>
> Thanks Andi.
>
>    Daniel.
>
>
> --
>
> Daniel F. Gutson
> Engineering Manager
>
> San Lorenzo 47, 3rd Floor, Office 5
> Córdoba, Argentina
>
> Phone:   +54 351 4217888 / +54 351 4218211
> Skype:    dgutson
> LinkedIn: http://ar.linkedin.com/in/danielgutson



-- 

Daniel F. Gutson
Engineering Manager

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: Ability for classifying of measurements\

[Andi Kleen]

Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:

> I kept thinking about all this, and I changed my mind towards a
> simpler (a first step) yet useful approach.
> In the same way people spread some printfs for old school debugging, I
> keep the idea of some perf API made available
> for the application (which calls depend on some flag, say -DNPERF
> similar to -DNDEBUG, so if the macro is
> not defined, calls have no effects).

> The first relatively easy (again, but yet useful) feature that comes
> to my mind, is the ability (from the app) to
> label phases of the perf recording, including enabling, disabling, and
> labelling.

ftrace has a labelling feature like this (/sys/kernel/debug/tracing_marker)

But you can just do it on your own by defining some uprobes with
perf probe for unique points and tracing them.

The main challenge would be good user tooling support.

For enabling/disabling:
it works for processes that do self monitoring today.

For non self monitoring there were perf patches at some point, but they
were rejected because it wasn't clear if a process should be able
to opt out of monitoring on its own.

I suppose it may be ok with an explicit opt-in on the perf side.

BTW I suspect the main motivation why you need this is because
perf doesn't have a time line to select what time region to browse.
The information is actually all in the perf.data, the UI is just
not good. I sometimes now use vtune (which reads perf files)
and has a time line to work around this.

At some point it would be nice to add a time line to perf. Unfortunately
I don't think it would work well with the terminal UI, so would
need to be done in the gui mode.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: Ability for classifying of measurements\

[Daniel Gutson]

On Wed, May 4, 2016 at 6:41 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:
>
>> I kept thinking about all this, and I changed my mind towards a
>> simpler (a first step) yet useful approach.
>> In the same way people spread some printfs for old school debugging, I
>> keep the idea of some perf API made available
>> for the application (which calls depend on some flag, say -DNPERF
>> similar to -DNDEBUG, so if the macro is
>> not defined, calls have no effects).
>
>> The first relatively easy (again, but yet useful) feature that comes
>> to my mind, is the ability (from the app) to
>> label phases of the perf recording, including enabling, disabling, and
>> labelling.
>
> ftrace has a labelling feature like this (/sys/kernel/debug/tracing_marker)
>
> But you can just do it on your own by defining some uprobes with
> perf probe for unique points and tracing them.
>
> The main challenge would be good user tooling support.
>
> For enabling/disabling:
> it works for processes that do self monitoring today.

What is self monitoring?

>
> For non self monitoring there were perf patches at some point, but they
> were rejected because it wasn't clear if a process should be able
> to opt out of monitoring on its own.

Any link?

>
> I suppose it may be ok with an explicit opt-in on the perf side.
>
> BTW I suspect the main motivation why you need this is because
> perf doesn't have a time line to select what time region to browse.
> The information is actually all in the perf.data, the UI is just
> not good. I sometimes now use vtune (which reads perf files)
> and has a time line to work around this.

But a untagged timeline is not extremely useful, moreover, please note
that in my proposal,
perf_label can append (if the file exists) the future data to the
existing data, so that would be a sort
of back to the past in time travelling parlance; what I want instead,
is the notion of logic states rather
than time points: suppose we are talking about a state machine (where
the machine bounces between
some states), and I want to record the metrics for each state
(separately, of course): timing is of no help there.
What I would like to know, is what happened (in terms of performance
metrics) on each state. I would simply
call perf_label("state name") on each transition.

>
> At some point it would be nice to add a time line to perf. Unfortunately
> I don't think it would work well with the terminal UI, so would
> need to be done in the gui mode.
>
> -Andi
>
> --
> ak@linux.intel.com -- Speaking for myself only



-- 

Daniel F. Gutson
Engineering Manager

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson

Re: Ability for classifying of measurements\

[Vince Weaver]

On Wed, 4 May 2016, Daniel Gutson wrote:

> > For enabling/disabling:
> > it works for processes that do self monitoring today.
>
> What is self monitoring?

it's when a program turns on/off the performance events manually from
within the program itself.

There are libraries that abstract this out a bit, for example PAPI
        http://icl.cs.utk.edu/papi/


Vince

Re: Ability for classifying of measurements\

[Andi Kleen]

Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:
>>
>> For non self monitoring there were perf patches at some point, but they
>> were rejected because it wasn't clear if a process should be able
>> to opt out of monitoring on its own.
>
> Any link?

Here's an older variant from Jiri https://lwn.net/Articles/568602/

I think there was another newer from Stephane to enable/disable perf
through an prctl, but I can't find it right now.

> (separately, of course): timing is of no help there.
> What I would like to know, is what happened (in terms of performance
> metrics) on each state. I would simply
> call perf_label("state name") on each transition.

Ok. It could be done using the uprobes. But would still need to write
the tools to filter by it.

-Andi