From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
Stephane Eranian <eranian@google.com>,
Namhyung Kim <namhyung@gmail.com>,
LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
"ak@linux.intel.com" <ak@linux.intel.com>
Subject: Re: [RFC] perf record: missing buildid for callstack modules
Date: Tue, 12 Jan 2016 14:15:17 -0300 [thread overview]
Message-ID: <20160112171517.GI18367@kernel.org> (raw)
In-Reply-To: <20160112162719.GX6373@twins.programming.kicks-ass.net>
Em Tue, Jan 12, 2016 at 05:27:19PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 12, 2016 at 05:10:27PM +0100, Peter Zijlstra wrote:
> > > How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
> > > if we don't look at those records?
> > Kernel side, the vma has a vm_file member. So
Gotcha, via PERF_RECORD_MMAP3, using something we have (mtime) that
while not good as a content-based cookie (that we don't easily have at
PERF_RECORD_MMAP3 generation time) will allow us to do minimal detection
of mismatched DSOs at analysis time.
> > vma->vm_file->f_inode->i_mtime will get us that for all file based
> > mmap()s.
> > (or maybe ctime, not sure how popular it is these days to switch off
> > mtime accounting).
> And this would (obviously) be a good time to see what else we'd like to
> stuff in there.
Perhaps a version number? Else we'll be using those reserved bits again
when we decide to introduce MMAP4 ;-\
But I still think we should have a content-based cookie to be delivered
via that record, so at a minimum leave a cookie there starting with a
size, that, for now, would be zero. If present, that would get into the
current build-id infrastructure existing in the tooling side, something
like:
+ /*
+ * The MMAP3 records are an augmented version of MMAP2, they add
+ * mtime and an optional content-based cookie, if available.
+ * struct perf_event_header header;
+ *
+ * u32 pid, tid;
+ * u64 addr;
+ * u64 len;
+ * u64 pgoff;
+ * u32 maj;
+ * u32 min;
+ * u64 ino;
+ * u64 ino_generation;
+ * u32 prot, flags;
+ * u64 mtime;
+ * u32 cookie_len
+ * char cookie[2+];
+ * char filename[];
+ * struct sample_id sample_id;
Then someone (I want to do this, after processing more stuff from a
neverending backlog) should try to experiment with the varios ELF
loaders, etc to, in the process of loading, set that content-based
cookie somehow (ioctl? prctl? whatever).
- Arnaldo
> ---
> include/uapi/linux/perf_event.h | 26 +++++++++++++++++++++++++-
> kernel/events/core.c | 19 +++++++++++++++----
> 2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 1afe9623c1a7..a0f43140a0e2 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -340,7 +340,8 @@ struct perf_event_attr {
> comm_exec : 1, /* flag comm events that are due to an exec */
> use_clockid : 1, /* use @clockid for time fields */
> context_switch : 1, /* context switch data */
> - __reserved_1 : 37;
> + mmap3 : 1, /* include mmap with mtime */
> + __reserved_1 : 36;
>
> union {
> __u32 wakeup_events; /* wakeup every n events */
> @@ -856,6 +857,29 @@ enum perf_event_type {
> */
> PERF_RECORD_SWITCH_CPU_WIDE = 15,
>
> + /*
> + * The MMAP3 records are an augmented version of MMAP2, they add
> + * mtime.
> + *
> + * struct {
> + * struct perf_event_header header;
> + *
> + * u32 pid, tid;
> + * u64 addr;
> + * u64 len;
> + * u64 pgoff;
> + * u32 maj;
> + * u32 min;
> + * u64 ino;
> + * u64 ino_generation;
> + * u32 prot, flags;
> + * u64 mtime;
> + * char filename[];
> + * struct sample_id sample_id;
> + * }
> + */
> + PERF_RECORD_MMAP3 = 16,
> +
> PERF_RECORD_MAX, /* non-ABI */
> };
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index bf8244190d0f..d400da14b923 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5661,7 +5661,7 @@ perf_event_aux(perf_event_aux_output_cb output, void *data,
> /*
> * task tracking -- fork/exit
> *
> - * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap_data | attr.task
> + * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap3 | attr.mmap_data | attr.task
> */
>
> struct perf_task_event {
> @@ -5682,8 +5682,8 @@ struct perf_task_event {
> static int perf_event_task_match(struct perf_event *event)
> {
> return event->attr.comm || event->attr.mmap ||
> - event->attr.mmap2 || event->attr.mmap_data ||
> - event->attr.task;
> + event->attr.mmap2 || event->attr.mmap3 ||
> + event->attr.mmap_data || event->attr.task;
> }
>
> static void perf_event_task_output(struct perf_event *event,
> @@ -5872,6 +5872,7 @@ struct perf_mmap_event {
> u64 ino;
> u64 ino_generation;
> u32 prot, flags;
> + u64 mtime;
>
> struct {
> struct perf_event_header header;
> @@ -5892,7 +5893,7 @@ static int perf_event_mmap_match(struct perf_event *event,
> int executable = vma->vm_flags & VM_EXEC;
>
> return (!executable && event->attr.mmap_data) ||
> - (executable && (event->attr.mmap || event->attr.mmap2));
> + (executable && (event->attr.mmap || event->attr.mmap2 || event-attr.mmap3));
> }
>
> static void perf_event_mmap_output(struct perf_event *event,
> @@ -5916,6 +5917,9 @@ static void perf_event_mmap_output(struct perf_event *event,
> mmap_event->event_id.header.size += sizeof(mmap_event->prot);
> mmap_event->event_id.header.size += sizeof(mmap_event->flags);
> }
> + if (event->attr.mmap3) {
> + mmap_event->event_id.header.size += sizeof(mmap_event->mtime);
> + }
>
> perf_event_header__init_id(&mmap_event->event_id.header, &sample, event);
> ret = perf_output_begin(&handle, event,
> @@ -5936,6 +5940,9 @@ static void perf_event_mmap_output(struct perf_event *event,
> perf_output_put(&handle, mmap_event->prot);
> perf_output_put(&handle, mmap_event->flags);
> }
> + if (event->attr.mmap3) {
> + perf_output_put(&handle, mmap_event->mtime);
> + }
>
> __output_copy(&handle, mmap_event->file_name,
> mmap_event->file_size);
> @@ -5954,6 +5961,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> int maj = 0, min = 0;
> u64 ino = 0, gen = 0;
> u32 prot = 0, flags = 0;
> + u64 mtime = 0;
> unsigned int size;
> char tmp[16];
> char *buf = NULL;
> @@ -5984,6 +5992,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> gen = inode->i_generation;
> maj = MAJOR(dev);
> min = MINOR(dev);
> + mtime = timespec_to_ns(inode->i_mtime);
>
> if (vma->vm_flags & VM_READ)
> prot |= PROT_READ;
> @@ -6054,6 +6063,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> mmap_event->ino_generation = gen;
> mmap_event->prot = prot;
> mmap_event->flags = flags;
> + mmap_event->mtime = mtime;
>
> if (!(vma->vm_flags & VM_EXEC))
> mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA;
> @@ -6096,6 +6106,7 @@ void perf_event_mmap(struct vm_area_struct *vma)
> /* .ino_generation (attr_mmap2 only) */
> /* .prot (attr_mmap2 only) */
> /* .flags (attr_mmap2 only) */
> + /* .mtime (attr_mmap3 only) */
> };
>
> perf_event_mmap_event(&mmap_event);
next prev parent reply other threads:[~2016-01-12 17:15 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-07 21:56 [RFC] perf record: missing buildid for callstack modules Stephane Eranian
2016-01-07 21:57 ` Andi Kleen
2016-01-07 22:00 ` Stephane Eranian
2016-01-07 21:59 ` Arnaldo Carvalho de Melo
2016-01-07 22:00 ` Stephane Eranian
2016-01-07 22:47 ` Namhyung Kim
2016-01-07 23:47 ` Arnaldo Carvalho de Melo
2016-01-08 18:01 ` Stephane Eranian
2016-01-08 18:19 ` Arnaldo Carvalho de Melo
2016-01-11 17:30 ` Peter Zijlstra
2016-01-11 18:22 ` Arnaldo Carvalho de Melo
2016-01-11 20:06 ` Stephane Eranian
2016-01-12 10:39 ` Ingo Molnar
2016-01-12 11:35 ` Peter Zijlstra
2016-01-12 12:18 ` Ingo Molnar
2016-01-12 13:40 ` Peter Zijlstra
2016-01-12 14:38 ` Arnaldo Carvalho de Melo
2016-01-12 15:34 ` Peter Zijlstra
2016-01-12 15:48 ` Arnaldo Carvalho de Melo
2016-01-12 16:10 ` Peter Zijlstra
2016-01-12 16:27 ` Peter Zijlstra
2016-01-12 17:15 ` Arnaldo Carvalho de Melo [this message]
2016-01-13 10:21 ` Ingo Molnar
2016-01-13 12:40 ` Peter Zijlstra
2016-01-14 11:27 ` Ingo Molnar
2016-01-14 11:36 ` Peter Zijlstra
2016-01-15 1:59 ` Stephane Eranian
2016-01-15 9:34 ` Peter Zijlstra
2016-01-15 18:58 ` Stephane Eranian
2016-01-15 19:49 ` Arnaldo Carvalho de Melo
2016-01-15 21:49 ` Stephane Eranian
2016-01-15 21:36 ` Peter Zijlstra
2016-01-12 21:02 ` Stephane Eranian
2016-01-12 13:08 ` One Thousand Gnomes
2016-01-12 14:34 ` Arnaldo Carvalho de Melo
2016-01-12 15:37 ` Peter Zijlstra
2016-01-13 10:25 ` Ingo Molnar
2016-01-12 14:23 ` Arnaldo Carvalho de Melo
2016-01-13 9:57 ` Ingo Molnar
2016-01-13 15:27 ` Arnaldo Carvalho de Melo
2016-01-19 14:56 ` Namhyung Kim
2016-01-19 15:27 ` Peter Zijlstra
2016-01-19 15:48 ` Arnaldo Carvalho de Melo
2016-01-09 10:31 ` Namhyung Kim
2016-01-11 9:27 ` Adrian Hunter
2016-01-11 11:02 ` Namhyung Kim
2016-01-11 11:54 ` Adrian Hunter
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=20160112171517.GI18367@kernel.org \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=eranian@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@gmail.com \
--cc=namhyung@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.