Re: [PATCH v2 1/2] perf: add attr->mmap2 attribute to an event

From: Arnaldo Carvalho de Melo
Date: Thu Aug 22 2013 - 11:58:10 EST

Next message: Boris BREZILLON: "[PATCH 2/2] ARM: at91/dt: define phy available on sama5d3 mother board"
Previous message: T Makphaibulchoke: "[PATCH v2 2/2] ext4: each filesystem creates and uses its own mc_cache"
In reply to: Stephane Eranian: "[PATCH v2 1/2] perf: add attr->mmap2 attribute to an event"
Next in thread: Stephane Eranian: "[PATCH v2 2/2] perf tools: add attr->mmap2 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Wed, Aug 21, 2013 at 12:10:24PM +0200, Stephane Eranian escreveu:
> Adds a new PERF_RECORD_MMAP2 record type.
>
> Used to request mmap records with more information about
> the mapping, including device major, minor and the inode
> number and generation for mappings associated with files
> or shared memory segments. Works for code and data
> (with attr->mmap_data set).
>
> Existing PERF_RECORD_MMAP record is unmodified by this patch.

So this is exactly what we get from PERF_RECORD_MMAP + a few other
fields, right?

I think we should take the opportunity and remove the fields that are
in PERF_RECORD_MMAP but can be selectively asked for using
->sample_id_all, i.e. we use sample_type to ask for:

PERF_SAMPLE_CPU
PERF_SAMPLE_STREAM_ID
PERF_SAMPLE_ID
PERF_SAMPLE_TIME
PERF_SAMPLE_TID

That should not be present in the PERF_RECORD_MMAP2 fixed payload.

This way we take the opportunity to compress the event by using an
existing facility: attr.sample_id_all and attr.sample_type.

But then this is "just" for the 8 bytes used for pid/tid, selectable via
PERF_SAMPLE_TID, so up to peterz, if you think its not worth the
complexity, Acked.

- Arnaldo

> Signed-off-by: Stephane Eranian <eranian@xxxxxxxxxx>
> ---
> include/uapi/linux/perf_event.h | 24 +++++++++++++++++++-
> kernel/events/core.c | 46 +++++++++++++++++++++++++++++++++++----
> 2 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 62c25a2..5268f78 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -275,8 +275,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 */
>
> - __reserved_1 : 41;
> + __reserved_1 : 40;
>
> union {
> __u32 wakeup_events; /* wakeup every n events */
> @@ -638,6 +639,27 @@ enum perf_event_type {
> */
> PERF_RECORD_SAMPLE = 9,
>
> + /*
> + * The MMAP2 records are an augmented version of MMAP, they add
> + * maj, min, ino numbers to be used to uniquely identify each mapping
> + *
> + * struct {
> + * struct perf_event_header header;
> + *
> + * u32 pid, tid;
> + * u64 addr;
> + * u64 len;
> + * u64 pgoff;
> + * u32 maj;
> + * u32 min;
> + * u64 ino;
> + * u64 ino_generation;
> + * char filename[];
> + * struct sample_id sample_id;
> + * };
> + */
> + PERF_RECORD_MMAP2 = 10,
> +
> PERF_RECORD_MAX, /* non-ABI */
> };
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 928fae7..60a5bbd 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4767,7 +4767,7 @@ next:
> /*
> * task tracking -- fork/exit
> *
> - * enabled by: attr.comm | attr.mmap | attr.mmap_data | attr.task
> + * enabled by: attr.comm | attr.mmap | attr.mmap2 | attr.mmap_data | attr.task
> */
>
> struct perf_task_event {
> @@ -4787,8 +4787,9 @@ struct perf_task_event {
>
> static int perf_event_task_match(struct perf_event *event)
> {
> - return event->attr.comm || event->attr.mmap ||
> - event->attr.mmap_data || event->attr.task;
> + return event->attr.comm || event->attr.mmap ||
> + event->attr.mmap2 || event->attr.mmap_data ||
> + event->attr.task;
> }
>
> static void perf_event_task_output(struct perf_event *event,
> @@ -4983,6 +4984,9 @@ struct perf_mmap_event {
>
> const char *file_name;
> int file_size;
> + int maj, min;
> + u64 ino;
> + u64 ino_generation;
>
> struct {
> struct perf_event_header header;
> @@ -5003,7 +5007,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);
> + (executable && (event->attr.mmap || event->attr.mmap2));
> }
>
> static void perf_event_mmap_output(struct perf_event *event,
> @@ -5018,6 +5022,13 @@ static void perf_event_mmap_output(struct perf_event *event,
> if (!perf_event_mmap_match(event, data))
> return;
>
> + if (event->attr.mmap2) {
> + 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);
> + mmap_event->event_id.header.size += sizeof(mmap_event->ino);
> + }
> +
> perf_event_header__init_id(&mmap_event->event_id.header, &sample, event);
> ret = perf_output_begin(&handle, event,
> mmap_event->event_id.header.size);
> @@ -5028,6 +5039,14 @@ static void perf_event_mmap_output(struct perf_event *event,
> mmap_event->event_id.tid = perf_event_tid(event, current);
>
> perf_output_put(&handle, mmap_event->event_id);
> +
> + if (event->attr.mmap2) {
> + perf_output_put(&handle, mmap_event->maj);
> + perf_output_put(&handle, mmap_event->min);
> + perf_output_put(&handle, mmap_event->ino);
> + perf_output_put(&handle, mmap_event->ino_generation);
> + }
> +
> __output_copy(&handle, mmap_event->file_name,
> mmap_event->file_size);
>
> @@ -5042,6 +5061,8 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> {
> struct vm_area_struct *vma = mmap_event->vma;
> struct file *file = vma->vm_file;
> + int maj = 0, min = 0;
> + u64 ino = 0, gen = 0;
> unsigned int size;
> char tmp[16];
> char *buf = NULL;
> @@ -5050,6 +5071,8 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> memset(tmp, 0, sizeof(tmp));
>
> if (file) {
> + struct inode *inode;
> + dev_t dev;
> /*
> * d_path works from the end of the rb backwards, so we
> * need to add enough zero bytes after the string to handle
> @@ -5065,6 +5088,13 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
> name = strncpy(tmp, "//toolong", sizeof(tmp));
> goto got_name;
> }
> + inode = file_inode(vma->vm_file);
> + dev = inode->i_sb->s_dev;
> + ino = inode->i_ino;
> + gen = inode->i_generation;
> + maj = MAJOR(dev);
> + min = MINOR(dev);
> +
> } else {
> if (arch_vma_name(mmap_event->vma)) {
> name = strncpy(tmp, arch_vma_name(mmap_event->vma),
> @@ -5095,6 +5125,10 @@ got_name:
>
> mmap_event->file_name = name;
> mmap_event->file_size = size;
> + mmap_event->maj = maj;
> + mmap_event->min = min;
> + mmap_event->ino = ino;
> + mmap_event->ino_generation = gen;
>
> if (!(vma->vm_flags & VM_EXEC))
> mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA;
> @@ -5131,6 +5165,10 @@ void perf_event_mmap(struct vm_area_struct *vma)
> .len = vma->vm_end - vma->vm_start,
> .pgoff = (u64)vma->vm_pgoff << PAGE_SHIFT,
> },
> + /* .maj (attr_mmap2 only) */
> + /* .min (attr_mmap2 only) */
> + /* .ino (attr_mmap2 only) */
> + /* .ino_generation (attr_mmap2 only) */
> };
>
> perf_event_mmap_event(&mmap_event);
> --
> 1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Boris BREZILLON: "[PATCH 2/2] ARM: at91/dt: define phy available on sama5d3 mother board"
Previous message: T Makphaibulchoke: "[PATCH v2 2/2] ext4: each filesystem creates and uses its own mc_cache"
In reply to: Stephane Eranian: "[PATCH v2 1/2] perf: add attr->mmap2 attribute to an event"
Next in thread: Stephane Eranian: "[PATCH v2 2/2] perf tools: add attr->mmap2 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]