Re: [PATCH] perf: Differentiate exec() and non-exec() comm events

[Ingo Molnar <mingo@kernel.org>]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, May 28, 2014 at 12:08:57PM +0300, Adrian Hunter wrote:
> > On 05/28/2014 11:55 AM, Peter Zijlstra wrote:
> > > On Wed, May 28, 2014 at 11:45:04AM +0300, Adrian Hunter wrote:
> > >> 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.
> > >>
> > > 
> > >> --- a/include/uapi/linux/perf_event.h
> > >> +++ b/include/uapi/linux/perf_event.h
> > >> @@ -302,8 +302,8 @@ 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     */
> > >> -
> > >> -				__reserved_1   : 40;
> > >> +				exec           :  1, /* flag comm events that are due to an exec */
> > >> +				__reserved_1   : 39;
> > >>  
> > > 
> > > Yah.. that's just sad :-(
> > > 
> > > the only capabilities mask we have is in the mmap() page, so without
> > > mmap()ing we have no way to test that.
> > > 
> > > Would it make sense to call it comm_exec?
> > 
> > Yes, that is better.  Do you want me to resend the patch?
> 
> Nah, I'll frob it. Thanks!

FYI, this patch breaks pretty much every non-x86 architecture:

/home/mingo/tip/fs/exec.c: In function 'setup_new_exec':
/home/mingo/tip/fs/exec.c:1113: error: implicit declaration of function 'perf_event_exec'
make[2]: *** [fs/exec.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [fs] Error 2
make[1]: *** Waiting for unfinished jobs....

Thanks,

	Ingo