On 29/07/2013 9:28 a.m., Namhyung Kim wrote:
Hi Adrian,

Just a few nitpicks below..


On Thu, 25 Jul 2013 17:01:22 +0300, Adrian Hunter wrote:
Using the information in mmap events, perf tools can read object
code associated with sampled addresses.  A test is added that
compares bytes read by perf with the same bytes read using
objdump.

[SNIP]
+
+static int read_objdump_line(char *line, size_t line_len, void **buf,
+                            size_t *len)
+{
+       size_t i;
+
+       /* Skip to a colon */
+       for (i = 0; i<  line_len; i++) {
+               if (line[i] == ':')
+                       break;
+       }
+       if (line[i++] != ':')
+               return 0;

strchr() ?

Fixed in V2


+
+       /* Read bytes */
+       while (*len) {
+               char c1, c2;
+
+               /* Skip spaces */
+               for (; i<  line_len; i++) {
+                       if (!isspace(line[i]))
+                               break;
+               }
+               /* Get 2 hex digits */
+               if (i>= line_len || !isxdigit(line[i]))
+                       break;
+               c1 = line[i++];
+               if (i>= line_len || !isxdigit(line[i]))
+                       break;
+               c2 = line[i++];
+               /* Followed by a space */
+               if (i<  line_len&&  line[i]&&  !isspace(line[i]))
+                       break;
+               /* Store byte */
+               *(unsigned char *)*buf = (hex(c1)<<  4) | hex(c2);
+               *buf += 1;
+               *len -= 1;
+       }
+
+       return 0;

It seems this function always returns 0..

Fixed in V2



+}
+
+static int read_objdump_output(FILE *f, void **buf, size_t *len)
+{
+       char *line = NULL;
+       size_t line_len;
+       ssize_t ret;
+       int err = 0;
+
+       while (1) {
+               ret = getline(&line,&line_len, f);
+               if (feof(f))
+                       break;
+               if (ret<  0 || read_objdump_line(line, ret, buf, len)) {

so no need to check the return value.

Fixed in V2



+                       pr_debug("getline failed\n");
+                       err = -1;
+                       break;
+               }
+       }
+
+       free(line);
+
+       return err;
+}
+
[SNIP]
+
+static int do_test_code_reading(void)
+{
+       struct machines machines;
+       struct machine *machine;
+       struct thread *thread;
+       struct perf_record_opts opts = {
+               .mmap_pages          = UINT_MAX,
+               .user_freq           = UINT_MAX,
+               .user_interval       = ULLONG_MAX,
+               .freq                = 40000,

Is it intended to use the freq of 40000 instead of 4000 (default)?

Yes.  The "workload" is small so a higher sampling rate is preferable.



+               .target              = {
+                       .uses_mmap   = true,
+               },
+       };
+       struct thread_map *threads = NULL;
+       struct cpu_map *cpus = NULL;
+       struct perf_evlist *evlist = NULL;
+       struct perf_evsel *evsel = NULL;
+       int err = -1, ret;
+       pid_t pid;
+       struct map *map;
+       bool have_vmlinux, excl_kernel = false;
+
+       pid = getpid();
+
+       machines__init(&machines);
+       machine =&machines.host;
+
+       ret = machine__create_kernel_maps(machine);
+       if (ret<  0) {
+               pr_debug("machine__create_kernel_maps failed\n");
+               goto out_err;
+       }
+
+       /* Load kernel map */
+       map = machine->vmlinux_maps[MAP__FUNCTION];
+       ret = map__load(map, NULL);
+       if (ret<  0) {
+               pr_debug("map__load failed\n");
+               goto out_err;
+       }
+       have_vmlinux = map->dso->symtab_type == DSO_BINARY_TYPE__VMLINUX;
+       /* No point getting kernel events if there is no vmlinux */
+       if (!have_vmlinux)
+               excl_kernel = true;
+
+       threads = thread_map__new_by_tid(pid);
+       if (!threads) {
+               pr_debug("thread_map__new_by_tid failed\n");
+               goto out_err;
+       }
+
+       ret = perf_event__synthesize_thread_map(NULL, threads,
+                                               perf_event__process, machine);
+       if (ret<  0) {
+               pr_debug("perf_event__synthesize_thread_map failed\n");
+               goto out_err;
+       }
+
+       thread = machine__findnew_thread(machine, pid, pid);
+       if (!thread) {
+               pr_debug("machine__findnew_thread failed\n");
+               goto out_err;
+       }
+
+       cpus = cpu_map__new(NULL);
+       if (!cpus) {
+               pr_debug("cpu_map__new failed\n");
+               goto out_err;
+       }
+
+       while (1) {
+               const char *str;
+
+               evlist = perf_evlist__new();
+               if (!evlist) {
+                       pr_debug("perf_evlist__new failed\n");
+                       goto out_err;
+               }
+
+               perf_evlist__set_maps(evlist, cpus, threads);
+
+               if (excl_kernel)
+                       str =  "cycles:u";

A double whitespace.

Fixed in V2



+               else
+                       str = "cycles";
+               pr_debug("Parsing event '%s'\n", str);
+               ret = parse_events(evlist, str);
+               if (ret<  0) {
+                       pr_debug("parse_events failed\n");
+                       goto out_err;
+               }
+
+               perf_evlist__config(evlist,&opts);
+
+               evsel = perf_evlist__first(evlist);
+
+               evsel->attr.comm = 1;
+               evsel->attr.disabled = 1;
+               evsel->attr.enable_on_exec = 0;
+
+               ret = perf_evlist__open(evlist);
+               if (ret<  0) {
+                       if (!excl_kernel) {
+                               excl_kernel = true;
+                               continue;

It seems the evlist is leaked.  perf_evlist__delete(evlist) is needed.

Fixed in V2


Thanks,
Namhyung


+                       }
+                       pr_debug("perf_evlist__open failed\n");
+                       goto out_err;
+               }
+               break;
+       }
+
+       ret = perf_evlist__mmap(evlist, UINT_MAX, false);
+       if (ret<  0) {
+               pr_debug("perf_evlist__mmap failed\n");
+               goto out_err;
+       }
+
+       perf_evlist__enable(evlist);
+
+       do_something();
+
+       perf_evlist__disable(evlist);
+
+       ret = process_events(machine, evlist);
+       if (ret<  0)
+               goto out_err;
+
+       if (!have_vmlinux)
+               err = TEST_CODE_READING_NO_VMLINUX;
+       else if (excl_kernel)
+               err = TEST_CODE_READING_NO_ACCESS;
+       else
+               err = TEST_CODE_READING_OK;
+out_err:
+       if (evlist) {
+               perf_evlist__disable(evlist);
+               perf_evlist__munmap(evlist);
+               perf_evlist__close(evlist);
+               perf_evlist__delete(evlist);
+       }
+       if (cpus)
+               cpu_map__delete(cpus);
+       if (threads)
+               thread_map__delete(threads);
+       machines__destroy_kernel_maps(&machines);
+       machine__delete_threads(machine);
+       machines__exit(&machines);
+
+       return err;
+}
+
+int test__code_reading(void)
+{
+       int ret;
+
+       ret = do_test_code_reading();
+
+       switch (ret) {
+       case TEST_CODE_READING_OK:
+               return 0;
+       case TEST_CODE_READING_NO_VMLINUX:
+               fprintf(stderr, " (no vmlinux)");
+               return 0;
+       case TEST_CODE_READING_NO_ACCESS:
+               fprintf(stderr, " (no access)");
+               return 0;
+       default:
+               return -1;
+       };
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 90e3056..fda12266 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -36,5 +36,6 @@ int test__bp_signal_overflow(void);
  int test__task_exit(void);
  int test__sw_clock_freq(void);
  int test__sample_parsing(void);
+int test__code_reading(void);

  #endif /* TESTS_H */

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