Em Wed, Nov 07, 2012 at 04:27:09PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung....@lge.com>
> 
> Current perf_event__synthesize_mmap_events() only deals with
> executable mappings.  With upcoming memory access sampling,

Not "upcoming", "recently added", since your patch needs the patch, in
Stephane series, that introduces PERF_RECORD_MISC_MMAP_DATA.

> non-executable data mappings are needed also.
> 
> While at it, convert parsing code to use sscanf which makes
> the code cleaner IMHO.

Please split the patch in two, one that does the
PERF_RECORD_MISC_MMAP_DATA and other that converts from hex2u64 to
sscanf.

- Arnaldo
 
> Cc: Stephane Eranian <eran...@google.com>
> Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> ---
>  tools/perf/util/event.c | 78 
> ++++++++++++++++++++++---------------------------
>  1 file changed, 35 insertions(+), 43 deletions(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index ca9ca285406a..068acf606b40 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -193,55 +193,47 @@ static int perf_event__synthesize_mmap_events(struct 
> perf_tool *tool,
>       event->header.misc = PERF_RECORD_MISC_USER;
>  
>       while (1) {
> -             char bf[BUFSIZ], *pbf = bf;
> -             int n;
> +             char bf[BUFSIZ];
> +             char prot[5], *pprot = prot;
>               size_t size;
> +             char exec_name[PATH_MAX], *pexec = exec_name;
> +             char anonstr[] = "//anon";
> +
>               if (fgets(bf, sizeof(bf), fp) == NULL)
>                       break;
>  
> +             /* ensure null termination since stack will be reused */
> +             strcpy(exec_name, "");
> +
>               /* 00400000-0040c000 r-xp 00000000 fd:01 41038  /bin/cat */
> -             n = hex2u64(pbf, &event->mmap.start);
> -             if (n < 0)
> -                     continue;
> -             pbf += n + 1;
> -             n = hex2u64(pbf, &event->mmap.len);
> -             if (n < 0)
> +             sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
> +                    &event->mmap.start, &event->mmap.len, pprot,
> +                    &event->mmap.pgoff, pexec);
> +
> +             if (prot[2] == 'x' && !strcmp(exec_name, ""))
> +                     strcpy(exec_name, anonstr);
> +
> +             /* ignore non-executable anon mappings */
> +             if (!strcmp(exec_name, ""))
>                       continue;
> -             pbf += n + 3;
> -             if (*pbf == 'x') { /* vm_exec */
> -                     char anonstr[] = "//anon\n";
> -                     char *execname = strchr(bf, '/');
> -
> -                     /* Catch VDSO */
> -                     if (execname == NULL)
> -                             execname = strstr(bf, "[vdso]");
> -
> -                     /* Catch anonymous mmaps */
> -                     if ((execname == NULL) && !strstr(bf, "["))
> -                             execname = anonstr;
> -
> -                     if (execname == NULL)
> -                             continue;
> -
> -                     pbf += 3;
> -                     n = hex2u64(pbf, &event->mmap.pgoff);
> -
> -                     size = strlen(execname);
> -                     execname[size - 1] = '\0'; /* Remove \n */
> -                     memcpy(event->mmap.filename, execname, size);
> -                     size = PERF_ALIGN(size, sizeof(u64));
> -                     event->mmap.len -= event->mmap.start;
> -                     event->mmap.header.size = (sizeof(event->mmap) -
> -                                             (sizeof(event->mmap.filename) - 
> size));
> -                     memset(event->mmap.filename + size, 0, 
> machine->id_hdr_size);
> -                     event->mmap.header.size += machine->id_hdr_size;
> -                     event->mmap.pid = tgid;
> -                     event->mmap.tid = pid;
> -
> -                     if (process(tool, event, &synth_sample, machine) != 0) {
> -                             rc = -1;
> -                             break;
> -                     }
> +
> +             if (prot[2] != 'x')
> +                     event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;
> +
> +             size = strlen(exec_name) + 1;
> +             memcpy(event->mmap.filename, exec_name, size);
> +             size = PERF_ALIGN(size, sizeof(u64));
> +             event->mmap.len -= event->mmap.start;
> +             event->mmap.header.size = (sizeof(event->mmap) -
> +                                        (sizeof(event->mmap.filename) - 
> size));
> +             memset(event->mmap.filename + size, 0, machine->id_hdr_size);
> +             event->mmap.header.size += machine->id_hdr_size;
> +             event->mmap.pid = tgid;
> +             event->mmap.tid = pid;
> +
> +             if (process(tool, event, &synth_sample, machine) != 0) {
> +                     rc = -1;
> +                     break;
>               }
>       }
>  
> -- 
> 1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to