All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: ye olde task_ctx_sched_out trace.
Date: Wed, 28 May 2014 11:02:59 +0300	[thread overview]
Message-ID: <53859833.6050905@intel.com> (raw)
In-Reply-To: <20140522072222.GR30445@twins.programming.kicks-ass.net>

On 05/22/2014 10:22 AM, Peter Zijlstra wrote:
> On Thu, May 22, 2014 at 09:52:46AM +0300, Adrian Hunter wrote:
>> +/*
>> + * PERF_RECORD_MISC_MMAP_DATA and PERF_RECORD_MISC_COMM_EXEC are used on
>> + * different events so can reuse the same bit position.
>> + */
>>  #define PERF_RECORD_MISC_MMAP_DATA		(1 << 13)
>> +#define PERF_RECORD_MISC_COMM_EXEC		(1 << 13)
>>  /*
>>   * Indicates that the content of PERF_SAMPLE_IP points to
>>   * the actual instruction that triggered the event. See also
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index ed50b09..760abd0 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -5067,7 +5067,7 @@ static void perf_event_comm_event(struct perf_comm_event *comm_event)
>>  		       NULL);
>>  }
>>  
>> -void perf_event_comm(struct task_struct *task)
>> +void perf_event_comm(struct task_struct *task, bool exec)
>>  {
>>  	struct perf_comm_event comm_event;
>>  	struct perf_event_context *ctx;
>> @@ -5093,7 +5093,7 @@ void perf_event_comm(struct task_struct *task)
>>  		.event_id  = {
>>  			.header = {
>>  				.type = PERF_RECORD_COMM,
>> -				.misc = 0,
>> +				.misc = exec ? PERF_RECORD_MISC_COMM_EXEC : 0,
>>  				/* .size */
>>  			},
>>  			/* .pid */
> 
> OK, now that you pointed out the obvious, yeah, I suppose we can do that :-)
> 

One problem is how user space can figure out if the kernel supports it.
For my purposes, I don't need to know in advance, but I can imagine some
users might want to.  So I suggest adding:

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index e3fc8f0..67cec3e 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -301,8 +301,9 @@ struct perf_event_attr {
                                exclude_callchain_kernel : 1, /* exclude kernel callchains */
                                exclude_callchain_user   : 1, /* exclude user callchains */
                                mmap2          :  1, /* include mmap with inode data     */
+                               exec           :  1, /* flag comm events that are due to an exec */
 
-                               __reserved_1   : 40;
+                               __reserved_1   : 39;
 
        union {
                __u32           wakeup_events;    /* wakeup every n events */


Commit message:

perf tools like 'perf report' can aggregate samples by comm
strings, which generally works.  However, there are other
potential use-cases.  For example, to pair up 'calls'
with 'returns' accurately (from branch events like Intel BTS)
it is necessary to identify whether the process has exec'd.
Although a comm event is generated when an 'exec' happens
it is also generated whenever the comm string is changed
on a whim (e.g. by prctl PR_SET_NAME).  This patch adds a
flag to the comm event to differentiate one case from the
other.

In order to determine whether the kernel supports the new
flag, a selection bit named 'exec' is added to struct
perf_event_attr.  The bit does nothing but will cause
perf_event_open() to fail if the bit is set on kernels
that do not have it defined.






  reply	other threads:[~2014-05-28  8:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-21 15:06 ye olde task_ctx_sched_out trace Dave Jones
2014-05-21 15:16 ` Peter Zijlstra
2014-05-21 15:30   ` Peter Zijlstra
2014-05-21 15:32     ` Peter Zijlstra
2014-05-22  6:52       ` Adrian Hunter
2014-05-22  7:04         ` Peter Zijlstra
2014-05-22  7:10           ` Adrian Hunter
2014-05-22  7:20             ` Peter Zijlstra
2014-05-22  7:22         ` Peter Zijlstra
2014-05-28  8:02           ` Adrian Hunter [this message]
2014-06-06 12:19         ` [tip:perf/core] perf: Differentiate exec() and non-exec() comm events tip-bot for Adrian Hunter
2014-06-06 12:19       ` [tip:perf/core] perf: Fix perf_event_comm() vs. exec() assumption tip-bot for Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53859833.6050905@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.