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

[Peter Zijlstra <peterz@infradead.org>]

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);