Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303937 > unrolled thread

Re: [RFC] perf record: missing buildid for callstack modules

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-07 23:00 +0100
    Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-07 23:10 +0100
      Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@gmail.com> - 2016-01-08 00:30 +0100
        Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-08 00:50 +0100
          Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-08 19:10 +0100
            Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-08 19:20 +0100
              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-11 18:40 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-11 19:30 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-11 21:10 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-12 11:50 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 12:40 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-12 13:20 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 14:50 +0100
                        Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:40 +0100
                          Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 16:40 +0100
                            Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 16:50 +0100
                              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 17:20 +0100
                                Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 17:30 +0100
                                  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 18:20 +0100
                                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:30 +0100
                                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-13 13:50 +0100
                                        Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-14 12:30 +0100
                                          Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-14 12:40 +0100
                                            Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-15 03:00 +0100
                                              Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-15 10:40 +0100
                                                Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-15 20:00 +0100
                                                  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-15 20:50 +0100
                                                    Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-15 22:50 +0100
                                                  Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-15 22:40 +0100
                              Re: [RFC] perf record: missing buildid for callstack modules Stephane Eranian <eranian@google.com> - 2016-01-12 22:10 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-12 14:10 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:40 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-12 16:40 +0100
                        Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:30 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-12 15:30 +0100
                    Re: [RFC] perf record: missing buildid for callstack modules Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:00 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-13 16:30 +0100
                      Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@kernel.org> - 2016-01-19 16:00 +0100
                        Re: [RFC] perf record: missing buildid for callstack modules Peter Zijlstra <peterz@infradead.org> - 2016-01-19 16:30 +0100
                          Re: [RFC] perf record: missing buildid for callstack modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-19 16:50 +0100
            Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@kernel.org> - 2016-01-09 11:40 +0100
              Re: [RFC] perf record: missing buildid for callstack modules Adrian Hunter <adrian.hunter@intel.com> - 2016-01-11 10:40 +0100
                Re: [RFC] perf record: missing buildid for callstack modules Namhyung Kim <namhyung@kernel.org> - 2016-01-11 12:10 +0100
                  Re: [RFC] perf record: missing buildid for callstack modules Adrian Hunter <adrian.hunter@intel.com> - 2016-01-11 13:00 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1308372

On Wed, Jan 13, 2016 at 11:21:07AM +0100, Ingo Molnar wrote:
> 
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > > 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 ;-\
> 
> No version numbers please. Cannot we have a size field and be done with it? The 
> size is the 'version', if we only ever expand the record. (which is the typical 
> case)

The current problem with that is that we use the 'remaining' size as
the length field for a string (with the PERF_RECORD_MMAP* records).

We could of course fix that no problem.


---
 include/uapi/linux/perf_event.h | 27 ++++++++++++++++++++++++++-
 kernel/events/core.c            | 35 ++++++++++++++++++++++++-----------
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1afe9623c1a7..b7b673387581 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,30 @@ enum perf_event_type {
 	 */
 	PERF_RECORD_SWITCH_CPU_WIDE		= 15,
 
+	/*
+	 * The MMAP3 records are an augmented version of MMAP2, they add
+	 * mtime and filename_len, allowing for size based extensions.
+	 *
+	 * 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;
+	 *	u32				filename_len;
+	 *	char				filename[2+];
+	 *	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..1cf15793f96c 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,
@@ -5867,11 +5867,12 @@ struct perf_mmap_event {
 	struct vm_area_struct	*vma;
 
 	const char		*file_name;
-	int			file_size;
+	u32			file_size;
 	int			maj, min;
 	u64			ino;
 	u64			ino_generation;
 	u32			prot, flags;
+	u64			mtime;
 
 	struct {
 		struct perf_event_header	header;
@@ -5892,11 +5893,10 @@ 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,
-				   void *data)
+static void perf_event_mmap_output(struct perf_event *event, void *data)
 {
 	struct perf_mmap_event *mmap_event = data;
 	struct perf_output_handle handle;
@@ -5907,7 +5907,7 @@ static void perf_event_mmap_output(struct perf_event *event,
 	if (!perf_event_mmap_match(event, data))
 		return;
 
-	if (event->attr.mmap2) {
+	if (event->attr.mmap2 || event->attr.mmap3) {
 		mmap_event->event_id.header.type = PERF_RECORD_MMAP2;
 		mmap_event->event_id.header.size += sizeof(mmap_event->maj);
 		mmap_event->event_id.header.size += sizeof(mmap_event->min);
@@ -5916,6 +5916,11 @@ 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.type = PERF_RECORD_MMAP3;
+		mmap_event->event_id.header.size += sizeof(mmap_event->mtime);
+		mmap_event->event_id.header.size += sizeof(mmap_event->file_size);
+	}
 
 	perf_event_header__init_id(&mmap_event->event_id.header, &sample, event);
 	ret = perf_output_begin(&handle, event,
@@ -5928,7 +5933,7 @@ static void perf_event_mmap_output(struct perf_event *event,
 
 	perf_output_put(&handle, mmap_event->event_id);
 
-	if (event->attr.mmap2) {
+	if (event->attr.mmap2 || event->attr.mmap3) {
 		perf_output_put(&handle, mmap_event->maj);
 		perf_output_put(&handle, mmap_event->min);
 		perf_output_put(&handle, mmap_event->ino);
@@ -5936,6 +5941,10 @@ 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);
+		perf_output_put(&handle, mmap_event->file_size);
+	}
 
 	__output_copy(&handle, mmap_event->file_name,
 				   mmap_event->file_size);
@@ -5954,8 +5963,9 @@ 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 tmp[18];
 	char *buf = NULL;
 	char *name;
 
@@ -5984,6 +5994,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;
@@ -6043,7 +6054,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 	 * zero'd out to avoid leaking random bits to userspace.
 	 */
 	size = strlen(name)+1;
-	while (!IS_ALIGNED(size, sizeof(u64)))
+	while (!IS_ALIGNED(2+size, sizeof(u64)))
 		name[size++] = '\0';
 
 	mmap_event->file_name = name;
@@ -6054,6 +6065,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 +6108,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);

[toc] | [prev] | [next] | [standalone]

#1309192

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

> The current problem with that is that we use the 'remaining' size as
> the length field for a string (with the PERF_RECORD_MMAP* records).
> 
> We could of course fix that no problem.
> 
> 
> ---
>  include/uapi/linux/perf_event.h | 27 ++++++++++++++++++++++++++-
>  kernel/events/core.c            | 35 ++++++++++++++++++++++++-----------
>  2 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 1afe9623c1a7..b7b673387581 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,30 @@ enum perf_event_type {
>  	 */
>  	PERF_RECORD_SWITCH_CPU_WIDE		= 15,
>  
> +	/*
> +	 * The MMAP3 records are an augmented version of MMAP2, they add
> +	 * mtime and filename_len, allowing for size based extensions.
> +	 *
> +	 * 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;
> +	 *	u32				filename_len;
> +	 *	char				filename[2+];
> +	 *	struct sample_id		sample_id;
> +	 * }
> +	 */
> +	PERF_RECORD_MMAP3			= 16,
> +
>  	PERF_RECORD_MAX,			/* non-ABI */
>  };

Yeah, very nice!

And this means v3 should be the last ever version - all future extensions can 
happen via the length field.

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1309195

On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
> > +	 *	u32				filename_len;
> > +	 *	char				filename[2+];

> Acked-by: Ingo Molnar <mingo@kernel.org>

except of course that sizeof(u32) == 4 :/

[toc] | [prev] | [next] | [standalone]

#1309807

Peter,

On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
>> > +    *      u32                             filename_len;
>> > +    *      char                            filename[2+];
>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>
> except of course that sizeof(u32) == 4 :/
There is no padding here. Are you concerned about running out of bits
in filename_len?
Any extension possible because header.size - sizeof(mmap3) -
filename_len sizing what's after filename, right?

[toc] | [prev] | [next] | [standalone]

#1309996

On Thu, Jan 14, 2016 at 05:59:48PM -0800, Stephane Eranian wrote:
> Peter,
> 
> On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
> >> > +    *      u32                             filename_len;
> >> > +    *      char                            filename[2+];
> >
> >> Acked-by: Ingo Molnar <mingo@kernel.org>
> >
> > except of course that sizeof(u32) == 4 :/

> There is no padding here. Are you concerned about running out of bits
> in filename_len?

No, I just made a mess of it :-)

filename_len should have been u16 and filename should then be 6+8n in
size.

> Any extension possible because header.size - sizeof(mmap3) -
> filename_len sizing what's after filename, right?

Right, current MMAP records use the remaining size as the filename
length, but by explicitly specifying that we can add optional fields.

These fields must be after filename_len, otherwise you'd not be able to
find filename_len and you could not compute the extra size. And given
alignment constraints it makes sense to do it after filename[].

So suppose we wanted to also add atime and ctime, we could do.

  PERF_RECORD_MMAP3 {
    ...
    u16 filename_len;
    char filename[6+8n];

    if (extra_size >= 16) {
      u64 stime;
      u64 ctime;
    };
  }

or something like that.

[toc] | [prev] | [next] | [standalone]

#1310373

On Fri, Jan 15, 2016 at 1:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jan 14, 2016 at 05:59:48PM -0800, Stephane Eranian wrote:
>> Peter,
>>
>> On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
>> >> > +    *      u32                             filename_len;
>> >> > +    *      char                            filename[2+];
>> >
>> >> Acked-by: Ingo Molnar <mingo@kernel.org>
>> >
>> > except of course that sizeof(u32) == 4 :/
>
>> There is no padding here. Are you concerned about running out of bits
>> in filename_len?
>
> No, I just made a mess of it :-)
>
> filename_len should have been u16 and filename should then be 6+8n in
> size.
>
why don't you make it more explicit:
   u16 filename_len
   u16 extra_len

then it would be clear what is what, no field with dual meaning.
Now, that assumes that no pathname can be longer than 65535 bytes.

>> Any extension possible because header.size - sizeof(mmap3) -
>> filename_len sizing what's after filename, right?
>
> Right, current MMAP records use the remaining size as the filename
> length, but by explicitly specifying that we can add optional fields.
>
> These fields must be after filename_len, otherwise you'd not be able to
> find filename_len and you could not compute the extra size. And given
> alignment constraints it makes sense to do it after filename[].
>
> So suppose we wanted to also add atime and ctime, we could do.
>
>   PERF_RECORD_MMAP3 {
>     ...
>     u16 filename_len;
>     char filename[6+8n];
>
>     if (extra_size >= 16) {
>       u64 stime;
>       u64 ctime;
>     };
>   }
>
> or something like that.

[toc] | [prev] | [next] | [standalone]

#1310418

Em Fri, Jan 15, 2016 at 10:58:48AM -0800, Stephane Eranian escreveu:
> On Fri, Jan 15, 2016 at 1:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jan 14, 2016 at 05:59:48PM -0800, Stephane Eranian wrote:
> >> Peter,
> >>
> >> On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> > On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
> >> >> > +    *      u32                             filename_len;
> >> >> > +    *      char                            filename[2+];
> >> >
> >> >> Acked-by: Ingo Molnar <mingo@kernel.org>
> >> >
> >> > except of course that sizeof(u32) == 4 :/
> >
> >> There is no padding here. Are you concerned about running out of bits
> >> in filename_len?
> >
> > No, I just made a mess of it :-)
> >
> > filename_len should have been u16 and filename should then be 6+8n in
> > size.
> >
> why don't you make it more explicit:
>    u16 filename_len
>    u16 extra_len
 
> then it would be clear what is what, no field with dual meaning.
> Now, that assumes that no pathname can be longer than 65535 bytes.

There is no requirement for the filename to start at a u64 boundary,
right? So it can start right after the filename_len, be it u16 or u32,
and that extra_len can be computed as Peter described right here:
 
> >> Any extension possible because header.size - sizeof(mmap3) -
> >> filename_len sizing what's after filename, right?

So, no need to have that extra_len :-)

Ah PATH_MAX is 4096, so I guess u16 should be enough, no?

- Arnaldo

> > Right, current MMAP records use the remaining size as the filename
> > length, but by explicitly specifying that we can add optional fields.
> >
> > These fields must be after filename_len, otherwise you'd not be able to
> > find filename_len and you could not compute the extra size. And given
> > alignment constraints it makes sense to do it after filename[].
> >
> > So suppose we wanted to also add atime and ctime, we could do.
> >
> >   PERF_RECORD_MMAP3 {
> >     ...
> >     u16 filename_len;
> >     char filename[6+8n];
> >
> >     if (extra_size >= 16) {
> >       u64 stime;
> >       u64 ctime;
> >     };
> >   }
> >
> > or something like that.

[toc] | [prev] | [next] | [standalone]

#1310494

On Fri, Jan 15, 2016 at 11:49 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Fri, Jan 15, 2016 at 10:58:48AM -0800, Stephane Eranian escreveu:
>> On Fri, Jan 15, 2016 at 1:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Thu, Jan 14, 2016 at 05:59:48PM -0800, Stephane Eranian wrote:
>> >> Peter,
>> >>
>> >> On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> >> > On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
>> >> >> > +    *      u32                             filename_len;
>> >> >> > +    *      char                            filename[2+];
>> >> >
>> >> >> Acked-by: Ingo Molnar <mingo@kernel.org>
>> >> >
>> >> > except of course that sizeof(u32) == 4 :/
>> >
>> >> There is no padding here. Are you concerned about running out of bits
>> >> in filename_len?
>> >
>> > No, I just made a mess of it :-)
>> >
>> > filename_len should have been u16 and filename should then be 6+8n in
>> > size.
>> >
>> why don't you make it more explicit:
>>    u16 filename_len
>>    u16 extra_len
>
>> then it would be clear what is what, no field with dual meaning.
>> Now, that assumes that no pathname can be longer than 65535 bytes.
>
> There is no requirement for the filename to start at a u64 boundary,
> right? So it can start right after the filename_len, be it u16 or u32,
> and that extra_len can be computed as Peter described right here:
>
>> >> Any extension possible because header.size - sizeof(mmap3) -
>> >> filename_len sizing what's after filename, right?
>
> So, no need to have that extra_len :-)
>
I know it can. I just find this cumbersome to have to have to compose
fields to get the extra length. That would be

   extra = header.size - sizeof(header) - sizeof(all_the_data_fields)
- filename_len;

That's ugly and error prone. I know it's been used for other record
types. But maybe
we could avoid it here.


> Ah PATH_MAX is 4096, so I guess u16 should be enough, no?
>
> - Arnaldo
>
>> > Right, current MMAP records use the remaining size as the filename
>> > length, but by explicitly specifying that we can add optional fields.
>> >
>> > These fields must be after filename_len, otherwise you'd not be able to
>> > find filename_len and you could not compute the extra size. And given
>> > alignment constraints it makes sense to do it after filename[].
>> >
>> > So suppose we wanted to also add atime and ctime, we could do.
>> >
>> >   PERF_RECORD_MMAP3 {
>> >     ...
>> >     u16 filename_len;
>> >     char filename[6+8n];
>> >
>> >     if (extra_size >= 16) {
>> >       u64 stime;
>> >       u64 ctime;
>> >     };
>> >   }
>> >
>> > or something like that.

[toc] | [prev] | [next] | [standalone]

#1310479

On Fri, Jan 15, 2016 at 10:58:48AM -0800, Stephane Eranian wrote:
> then it would be clear what is what, no field with dual meaning.
> Now, that assumes that no pathname can be longer than 65535 bytes.

include/uapi/linux/limits.h:#define PATH_MAX        4096        /* # chars in a path name including nul */

Also, our RECORDS cannot exceed 64k.

[toc] | [prev] | [next] | [standalone]

#1307831

On Tue, Jan 12, 2016 at 7:48 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Tue, Jan 12, 2016 at 04:34:40PM +0100, Peter Zijlstra escreveu:
>> On Tue, Jan 12, 2016 at 11:38:05AM -0300, Arnaldo Carvalho de Melo wrote:
>> > > Also, just parsing the gigabytes of data that comes out of perf-record
>> > > takes significant time, let alone poking around the filesystem and
>> >
>> > Right, that is what we would elliminate with stashing the content-based
>> > cookie into a PERF_RECORD_MMAP3 record.
>>
>> Again, how would you go about getting that cookie for a DSO? The whole
>> kernel isn't involved with dlopen(), all it sees is a mmap(PROT_EXEC).
>>
>> > BTW, mtime would incur in postprocessing it all.
>>
>> mtime can still warn you if things are non-matching at report time
>> without this post-processing, and thereby solves the problem of staring
>> at broken/wrong data.
>
> How will we collect the mtime for the DSOs in PERF_RECORD_MMAP records
> if we don't look at those records? What mtime are you talking about?
>
I think the post-processing of MMAP could be sped up if they were
saved out-of-band
inside of with the samples. Isn't that what the auxbuffer allows you to do?

>> It doesn't get you right data, but knowing your data is broken allows
>> you to manually do things 'right'.

[toc] | [prev] | [next] | [standalone]

#1307388

> And we cannot, at mmap() time, 'assume' the file is ELF and try prodding
> into it to find a buildid or whatnot.

And you cannot at any time assume that something like NFS won't change
the page contents under you. In fact it's valid (but completely insane)
to use MAP_DISCARD to throw away part of your program and page it back in
again so that a daemon on the other side of the NFS can change the code
you are executing 8)

Alan

[toc] | [prev] | [next] | [standalone]

#1307483

Em Tue, Jan 12, 2016 at 12:35:21PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar wrote:
> > > Does the kernel even know about the buildid crap? AFAIK the binfmt stuff doesn't 
> > > know or care about things like that. Heck, we support binfmts that do not even 
> > > have a buildid.
> > 
> > The kernel's exec() code does not care about the past, it will execute whatever is 
> > fit to execute right now.
> > 
> > But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> > display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> > binary mismatch can be very convincing and can send developers down the wrong path 
> > for hours. I'd expect my tooling to not do that.
> 
> Well, it really is rather a rare case, replacing binaries you're
> profiling. Sure, if it happens (by accident or otherwise) it can be a
> pain, but the cost of fixing this 'problem' is huge.

Humm, for most things today, i.e. ELF, most distros (all? maybe this is
a gcc switch that is default on, haven't checked) come with such
pre-computed cookie its just a way for efficiently passing it to the
tooling via a new record. With that, no post processing, etc. But then
someone would need to prototype this...

[acme@zoo linux]$ perf record -h build

 Usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -B, --no-buildid      do not collect buildids in perf.data
    -N, --no-buildid-cache
                          do not update the buildid cache

[acme@zoo linux]$

Have you ever played, when you noticed those overheads, with -N? Or just
used the -B big hammer and moved on?

- Arnaldo
 
> > Path names alone (the thing that exec() cares about) are not unique enough to 
> > identify the binary that was profiled. So we need a content hash - hence the 
> > build-ID.
> > 
> > Can you suggest a better solution than a build-time calculated content hash?
> 
> Not really, but the current 'solution' is a massive pain. The result is
> that perf-record needs to do a full scan of the recorded data after
> completion and look for buildids across the system.
> 
> On my system that pass takes longer than the actual workload (of
> building a kernel). Furthermore, the resulting data is useless for me.
> 
> > As for binary formats that suck and don't allow for a content hash: we do our 
> > best, but of course the risk of data mismatch is there. We could perhaps cache the 
> > binary inode's mtime field to at least produce a 'profile data is older than 
> > binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> > cross-system profiling data matches.)
> 
> So my problem with the kernel side thing is that I fear it will, again,
> be a partial solution, and we'll still end up scanning the perf-record
> output, ie. nothing better than we are now.
> 
> Sure, maybe we can have binfmt_elf read the buildid and cache it
> someplace, maybe we can even have the other binfmt thingies do something
> similar (at small cost, we obviously cannot compute hashes over files at
> exec() time, that would upset people).
> 
> But what do we do for DSOs, does dlopen() ever end up in the binfmt
> code? I would think not, I would fully expect the dynamic linker to just
> mmap() the relevant bits and be done with it.
> 
> And we cannot, at mmap() time, 'assume' the file is ELF and try prodding
> into it to find a buildid or whatnot.
> 
> And all for some weird corner case.
> 
> ~ Peter, who thinks buildid stuff stinks.

[toc] | [prev] | [next] | [standalone]

#1307554

On Tue, Jan 12, 2016 at 11:34:54AM -0300, Arnaldo Carvalho de Melo wrote:
> Have you ever played, when you noticed those overheads, with -N? Or just
> used the -B big hammer and moved on?

I yelled on irc, jolsa told me to use -B, I moved on ;-)

Not sure -N really buys me anything, its still slower and I really don't
need any of this.

[toc] | [prev] | [next] | [standalone]

#1308265

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

> On Tue, Jan 12, 2016 at 11:34:54AM -0300, Arnaldo Carvalho de Melo wrote:
>
> > Have you ever played, when you noticed those overheads, with -N? Or just used 
> > the -B big hammer and moved on?
> 
> I yelled on irc, jolsa told me to use -B, I moved on ;-)

So I think we should grow mtime protection, to not display incorrect data - and 
then that should become the default. (in addition to per CPU recording threads.)

Way too slow recording is something I experience as well - and to have a slow 
_performance_ profiling tool is pretty ironic! ;-)

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1307473

Em Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar escreveu:
> But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> binary mismatch can be very convincing and can send developers down the wrong path 
> for hours. I'd expect my tooling to not do that.
 
> Path names alone (the thing that exec() cares about) are not unique enough to 
> identify the binary that was profiled. So we need a content hash - hence the 
> build-ID.
 
> Can you suggest a better solution than a build-time calculated content hash?
 
> As for binary formats that suck and don't allow for a content hash: we do our 
> best, but of course the risk of data mismatch is there. We could perhaps cache the 
> binary inode's mtime field to at least produce a 'profile data is older than 
> binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> cross-system profiling data matches.)

So, we could think of this as: binary formats that want to aid
observability tools to:

1) Detect mismatches in contents for DSOs present at recording time to
   those to be used at analysis time.

2) Find symtabs, DSO binary contents, CFI tables, present in the DSO
   where samples were taken.

Using mtime, as suggested in other messages will help with #1, but not
with #2.

Checking for inefficiencies in the current approach of
right-after-recording post-processing looking for PERF_RECORD_MMAPs,
Adrian suggested something here, also disabling the saving into
~/.debug/ will help, collecting numbers would be great.

But the mtime thing also requires traversing the whole perf.data
contents looking for those paths in PERF_RECORD_MMAP records.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1308236

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar escreveu:
> > But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> > display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> > binary mismatch can be very convincing and can send developers down the wrong path 
> > for hours. I'd expect my tooling to not do that.
>  
> > Path names alone (the thing that exec() cares about) are not unique enough to 
> > identify the binary that was profiled. So we need a content hash - hence the 
> > build-ID.
>  
> > Can you suggest a better solution than a build-time calculated content hash?
>  
> > As for binary formats that suck and don't allow for a content hash: we do our 
> > best, but of course the risk of data mismatch is there. We could perhaps cache the 
> > binary inode's mtime field to at least produce a 'profile data is older than 
> > binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> > cross-system profiling data matches.)
> 
> So, we could think of this as: binary formats that want to aid
> observability tools to:
> 
> 1) Detect mismatches in contents for DSOs present at recording time to
>    those to be used at analysis time.
> 
> 2) Find symtabs, DSO binary contents, CFI tables, present in the DSO
>    where samples were taken.
> 
> Using mtime, as suggested in other messages will help with #1, but not
> with #2.

But but ... why is #2 a problem with mtime? If we have an out of date record in 
the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
out of date, most likely because the binary the developer is working on got 
rebuilt, or the system got upgraded - in both cases the developer does not care 
about the old records anymore...

What matters is #1, to detect mismatches, to be a reliable tool. Once we've 
detected that, we can inform the user and our job is mostly done.

But reliable != perfect time machine. Really, #2 is a second, third order concern 
that should never cause slowdowns on the magnitude that Peter is complaining 
about!

I realize that there might be special workflows (such as system-wide monitoring) 
where collecting at recording time might be useful, but those are not the common 
case - and they should not slow down the common case.

> Checking for inefficiencies in the current approach of
> right-after-recording post-processing looking for PERF_RECORD_MMAPs,
> Adrian suggested something here, also disabling the saving into
> ~/.debug/ will help, collecting numbers would be great.

I think Peter mentioned a number: the kernel build time almost _doubles_ with 
this. That's clearly unacceptable.

> But the mtime thing also requires traversing the whole perf.data
> contents looking for those paths in PERF_RECORD_MMAP records.

But why? Why cannot we do it at perf report time, when we will parse them anyway?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1308507

Em Wed, Jan 13, 2016 at 10:57:38AM +0100, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar escreveu:
> > > But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> > > display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> > > binary mismatch can be very convincing and can send developers down the wrong path 
> > > for hours. I'd expect my tooling to not do that.
 
> > > Path names alone (the thing that exec() cares about) are not unique enough to 
> > > identify the binary that was profiled. So we need a content hash - hence the 
> > > build-ID.
 
> > > Can you suggest a better solution than a build-time calculated content hash?
  
> > > As for binary formats that suck and don't allow for a content hash: we do our 
> > > best, but of course the risk of data mismatch is there. We could perhaps cache the 
> > > binary inode's mtime field to at least produce a 'profile data is older than 
> > > binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> > > cross-system profiling data matches.)

> > So, we could think of this as: binary formats that want to aid
> > observability tools to:

> > 1) Detect mismatches in contents for DSOs present at recording time to
> >    those to be used at analysis time.

> > 2) Find symtabs, DSO binary contents, CFI tables, present in the DSO
> >    where samples were taken.

> > Using mtime, as suggested in other messages will help with #1, but not
> > with #2.
> 
> But but ... why is #2 a problem with mtime? If we have an out of date record in 
> the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
> out of date, most likely because the binary the developer is working on got 
> rebuilt, or the system got upgraded - in both cases the developer does not care 
> about the old records anymore...

Oh, with mtime we'll be able to, with some effort, most of the time,
find the right symtab/DSO contents, it is not as good as the
content-based build-id, but it improves the current situation, for, to
reuse an euphemism, 99% of the cases ;-)

It is just a pity that what would arguably be the last step to make
content-based DSO identifiers a first class citizen, already available
mostly everywhere, i.e. in ELF binaries will have to wait a bit more.

With PeterZ's change to make the filename length part of the
PERF_RECORD_MMAP3 we at least leave the door open to including that
cookie without having to introduce PERF_RECORD_MMAP4 :-)

> What matters is #1, to detect mismatches, to be a reliable tool. Once we've
> detected that, we can inform the user and our job is mostly done.

Well, we could tell the user to install the package with that symtab, in fedora
it is called 'foo-debuginfo' and can be installed via, for instance (example
taken from a gdb post somewhere):

  [root@zoo ~]# dnf --enablerepo='*debug*' install /usr/lib/debug/.build-id/3d/f5385c6be529423a8ae3dd39a3deb9425201cc
  <SNIP>
  Using metadata from Wed Jan 13 12:12:16 2016 (0:02:56 hours old)
  Dependencies resolved.
  ==================================================================
   Package            Arch    Version      Repository          Size
  ==================================================================
  Installing:
   glibc-debuginfo    x86_64  2.20-8.fc21  updates-debuginfo  9.2 M

  Transaction Summary
  ==================================================================
  Install  1 Package

  Total download size: 9.2 M
  Installed size: 58 M
  Is this ok [y/N]:

-----------------------------------

I.e. infrastructure is in place to get this "time machine" you mention below, is somewhat
in place to get a symtab for a DSO, be it the latest version of some versions ago.

Which could be useful to understand the behaviour of code in production while an
update can't be applied (not vetted by powers that be, whatever reason).
 
> But reliable != perfect time machine. Really, #2 is a second, third order concern 
> that should never cause slowdowns on the magnitude that Peter is complaining 
> about!

Sure, mtime will improve peterz's usecase, no question about it.
 
> I realize that there might be special workflows (such as system-wide monitoring) 
> where collecting at recording time might be useful, but those are not the common 
> case - and they should not slow down the common case.

Sure, no question about this.
 
> > Checking for inefficiencies in the current approach of
> > right-after-recording post-processing looking for PERF_RECORD_MMAPs,
> > Adrian suggested something here, also disabling the saving into
> > ~/.debug/ will help, collecting numbers would be great.
> 
> I think Peter mentioned a number: the kernel build time almost _doubles_ with 
> this. That's clearly unacceptable.

And for him, most of the time, not a problem, in fact we could argue that he
has not a problem, Stephane seems to have ;-)
 
> > But the mtime thing also requires traversing the whole perf.data
> > contents looking for those paths in PERF_RECORD_MMAP records.
 
> But why? Why cannot we do it at perf report time, when we will parse them anyway?

Hey, I was talking only about 'record' time.

And at record, we don'have to traverse the whole perf.data contents as soon as
we add a new PERF_RECORD_MMAP3, just not with the initial motivation for it (a
content-based cookie), but instead the DSOs's mtime :-)

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1312308

On Wed, Jan 13, 2016 at 10:57:38AM +0100, Ingo Molnar wrote:
> 
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Tue, Jan 12, 2016 at 11:39:43AM +0100, Ingo Molnar escreveu:
> > > But perf tooling cares very much: it can lead to subtle bugs and bad data if we 
> > > display a profile with the wrong DSO or binary. 'Bad' profiles resulting out of 
> > > binary mismatch can be very convincing and can send developers down the wrong path 
> > > for hours. I'd expect my tooling to not do that.
> >  
> > > Path names alone (the thing that exec() cares about) are not unique enough to 
> > > identify the binary that was profiled. So we need a content hash - hence the 
> > > build-ID.
> >  
> > > Can you suggest a better solution than a build-time calculated content hash?
> >  
> > > As for binary formats that suck and don't allow for a content hash: we do our 
> > > best, but of course the risk of data mismatch is there. We could perhaps cache the 
> > > binary inode's mtime field to at least produce a 'profile data is older than 
> > > binary/DSO modification date!' warning. (Which check won't catch all cases, like 
> > > cross-system profiling data matches.)
> > 
> > So, we could think of this as: binary formats that want to aid
> > observability tools to:
> > 
> > 1) Detect mismatches in contents for DSOs present at recording time to
> >    those to be used at analysis time.
> > 
> > 2) Find symtabs, DSO binary contents, CFI tables, present in the DSO
> >    where samples were taken.
> > 
> > Using mtime, as suggested in other messages will help with #1, but not
> > with #2.
> 
> But but ... why is #2 a problem with mtime? If we have an out of date record in 
> the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
> out of date, most likely because the binary the developer is working on got 
> rebuilt, or the system got upgraded - in both cases the developer does not care 
> about the old records anymore...

We have 'perf diff' command which compares old and new performance
results of a same program.  People can use it to see how much improved
in the new version than the baseline.  In this case, the old binary
should be found from the old perf.data.

Thanks,
Namhyung


> 
> What matters is #1, to detect mismatches, to be a reliable tool. Once we've 
> detected that, we can inform the user and our job is mostly done.
> 
> But reliable != perfect time machine. Really, #2 is a second, third order concern 
> that should never cause slowdowns on the magnitude that Peter is complaining 
> about!
> 
> I realize that there might be special workflows (such as system-wide monitoring) 
> where collecting at recording time might be useful, but those are not the common 
> case - and they should not slow down the common case.
> 
> > Checking for inefficiencies in the current approach of
> > right-after-recording post-processing looking for PERF_RECORD_MMAPs,
> > Adrian suggested something here, also disabling the saving into
> > ~/.debug/ will help, collecting numbers would be great.
> 
> I think Peter mentioned a number: the kernel build time almost _doubles_ with 
> this. That's clearly unacceptable.
> 
> > But the mtime thing also requires traversing the whole perf.data
> > contents looking for those paths in PERF_RECORD_MMAP records.
> 
> But why? Why cannot we do it at perf report time, when we will parse them anyway?
> 
> Thanks,
> 
> 	Ingo

[toc] | [prev] | [next] | [standalone]

#1312327

On Tue, Jan 19, 2016 at 11:56:40PM +0900, Namhyung Kim wrote:

> > But but ... why is #2 a problem with mtime? If we have an out of date record in 
> > the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
> > out of date, most likely because the binary the developer is working on got 
> > rebuilt, or the system got upgraded - in both cases the developer does not care 
> > about the old records anymore...
> 
> We have 'perf diff' command which compares old and new performance
> results of a same program.  People can use it to see how much improved
> in the new version than the baseline.  In this case, the old binary
> should be found from the old perf.data.

Just means they'll have to use perf-archive or whatnot before that
works. Making the regular perf-record dead slow just so that a few more
complex workloads work doesn't make sense.

[toc] | [prev] | [next] | [standalone]

#1312340

Em Tue, Jan 19, 2016 at 04:27:42PM +0100, Peter Zijlstra escreveu:
> On Tue, Jan 19, 2016 at 11:56:40PM +0900, Namhyung Kim wrote:
> 
> > > But but ... why is #2 a problem with mtime? If we have an out of date record in 
> > > the perf.data, then the perf.data is uninteresting in 99% of the usecases! It's 
> > > out of date, most likely because the binary the developer is working on got 
> > > rebuilt, or the system got upgraded - in both cases the developer does not care 
> > > about the old records anymore...

> > We have 'perf diff' command which compares old and new performance
> > results of a same program.  People can use it to see how much improved
> > in the new version than the baseline.  In this case, the old binary
> > should be found from the old perf.data.

> Just means they'll have to use perf-archive or whatnot before that
> works. Making the regular perf-record dead slow just so that a few more
> complex workloads work doesn't make sense.

And perf-diff will be able to find the right binaries in most cases, as
we'll end up using that mtime to pick the right binary for the baseline
and the other perf.data files used.

So, if both files are reachable via the usual path (/usr/lib/debug,
/bin/, /lib64, ~/.debug/, etc) it'll work.

For users wanting the convenience of auto-saving binaries + its sources,
then they will have to incur the cost of postprocessing the just
generated perf.data file to do that, and make sure debuginfo is enabled,
for having the sources embedded into those fat binaries.

With the current situation, without any disambiguation mechanism, we
_need_ to traverse the perf.data file to find that disambiguation bit,
the build-id (we could'be been using the mtime, but having to traverse
it all we may as well use something better, the build-id), and while
doing that we could as well do a hardlink into ~/.debug/ for the DSOs
found in PERF_RECORD_MMAP(2) meta events (a copy if that hardlink isn't
possible, more overhead).

Now we'll use that mtime, which should be good enough for both figuring
out if a file can be used to even choose among various entries for a
same pathname. And we'll cover the case where a DSO is replaced during
a record session, as newer PERF_RECORD_MMAP3 emitted after the update
will have a different mtime, yay!

The extra, content based verification to double check that is the real
DSO used, which we're not using anyway right now (but could) will not be
possible, i.e. regenerate the build-id from the DSO contents to check
that it is really what was present at the record phase.

But for those super stringent needs, the door is open to add the
content-based cookie at the end of the PERF_RECORD_MMAP3, as we have in
it the filename-len, etc.

So now, with PERF_RECORD_MMAP3 the onus of using this all is back at
where it belongs, away from kernel developers and into tools/perf/
developers, tooling should use build-ids if available and if not,
fallback to mtime, if even that is not available (older kernels) then
warn the user and hope the pathname is enough.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web