Messages in this thread | | | Date | Tue, 21 Feb 2017 10:38:27 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH 2/6] perf utils: Check verbose flag properly |
| |
Hi Arnaldo,
On Mon, Feb 20, 2017 at 11:35:44AM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Feb 17, 2017 at 05:17:38PM +0900, Namhyung Kim escreveu: > > It now can have negative value to suppress the message entirely. So it > > needs to check it being positive. > > find tools/perf -name "*.[chly]" | xargs grep -w verbose > > Shows several other places, I'm trying to plug those, fixed it up in > your patch and left a committer note, please check and see if all is > right.
Looks ok to me. But most of them are part of builtin commands so it should be added with -q option. I'll cook a patchset for them.
Thanks, Namhyung
> > - Arnaldo > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c > index 70a289347591..7ad0d78ea743 100644 > --- a/tools/perf/builtin-diff.c > +++ b/tools/perf/builtin-diff.c > @@ -744,7 +744,7 @@ static void data_process(void) > > first = false; > > - if (verbose || data__files_cnt > 2) > + if (verbose > 0 || data__files_cnt > 2) > data__fprintf(); > > /* Don't sort callchain for perf diff */ > diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c > index cd7bc4d104e2..6114e07ca613 100644 > --- a/tools/perf/builtin-mem.c > +++ b/tools/perf/builtin-mem.c > @@ -42,8 +42,8 @@ static int parse_record_events(const struct option *opt, > > fprintf(stderr, "%-13s%-*s%s\n", > e->tag, > - verbose ? 25 : 0, > - verbose ? perf_mem_events__name(j) : "", > + verbose > 0 ? 25 : 0, > + verbose > 0 ? perf_mem_events__name(j) : "", > e->supported ? ": available" : ""); > } > exit(0); > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index b87bbef73394..451b11e35c26 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -432,7 +432,7 @@ static int record__open(struct record *rec) > try_again: > if (perf_evsel__open(pos, pos->cpus, pos->threads) < 0) { > if (perf_evsel__fallback(pos, errno, msg, sizeof(msg))) { > - if (verbose) > + if (verbose > 0) > ui__warning("%s\n", msg); > goto try_again; > } > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index dbd7fa028861..a94488114bbf 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -1009,7 +1009,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) > * providing it only in verbose mode not to bloat too > * much struct symbol. > */ > - if (verbose) { > + if (verbose > 0) { > /* > * XXX: Need to provide a less kludgy way to ask for > * more space per symbol, the u32 is for the index on > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index 270eb2d8ca6b..b94cf0de715a 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -460,7 +460,7 @@ static struct task_desc *register_pid(struct perf_sched *sched, > BUG_ON(!sched->tasks); > sched->tasks[task->nr] = task; > > - if (verbose) > + if (verbose > 0) > printf("registered task #%ld, PID %ld (%s)\n", sched->nr_tasks, pid, comm); > > return task; > @@ -794,7 +794,7 @@ replay_wakeup_event(struct perf_sched *sched, > const u32 pid = perf_evsel__intval(evsel, sample, "pid"); > struct task_desc *waker, *wakee; > > - if (verbose) { > + if (verbose > 0) { > printf("sched_wakeup event %p\n", evsel); > > printf(" ... pid %d woke up %s/%d\n", sample->tid, comm, pid); > @@ -822,7 +822,7 @@ static int replay_switch_event(struct perf_sched *sched, > int cpu = sample->cpu; > s64 delta; > > - if (verbose) > + if (verbose > 0) > printf("sched_switch event %p\n", evsel); > > if (cpu >= MAX_CPUS || cpu < 0) > @@ -870,7 +870,7 @@ static int replay_fork_event(struct perf_sched *sched, > goto out_put; > } > > - if (verbose) { > + if (verbose > 0) { > printf("fork event\n"); > printf("... parent: %s/%d\n", thread__comm_str(parent), parent->tid); > printf("... child: %s/%d\n", thread__comm_str(child), child->tid); > @@ -1573,7 +1573,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel, > > timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp)); > color_fprintf(stdout, color, " %12s secs ", stimestamp); > - if (new_shortname || (verbose && sched_in->tid)) { > + if (new_shortname || (verbose > 0 && sched_in->tid)) { > const char *pid_color = color; > > if (thread__has_color(sched_in)) > @@ -2050,7 +2050,7 @@ static void save_task_callchain(struct perf_sched *sched, > > if (thread__resolve_callchain(thread, cursor, evsel, sample, > NULL, NULL, sched->max_stack + 2) != 0) { > - if (verbose) > + if (verbose > 0) > error("Failed to resolve callchain. Skipping\n"); > > return; > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 9989b03c21f2..13b54999ad79 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -573,7 +573,7 @@ static int __run_perf_stat(int argc, const char **argv) > if (errno == EINVAL || errno == ENOSYS || > errno == ENOENT || errno == EOPNOTSUPP || > errno == ENXIO) { > - if (verbose) > + if (verbose > 0) > ui__warning("%s event is not supported by the kernel.\n", > perf_evsel__name(counter)); > counter->supported = false; > @@ -582,7 +582,7 @@ static int __run_perf_stat(int argc, const char **argv) > !(counter->leader->nr_members > 1)) > continue; > } else if (perf_evsel__fallback(counter, errno, msg, sizeof(msg))) { > - if (verbose) > + if (verbose > 0) > ui__warning("%s\n", msg); > goto try_again; > } > @@ -2539,7 +2539,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) > > status = 0; > for (run_idx = 0; forever || run_idx < run_count; run_idx++) { > - if (run_count != 1 && verbose) > + if (run_count != 1 && verbose > 0) > fprintf(output, "[ perf stat: executing run #%d ... ]\n", > run_idx + 1); > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 5a7fd7af3a6d..ab9077915763 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -871,7 +871,7 @@ static int perf_top__start_counters(struct perf_top *top) > if (perf_evsel__open(counter, top->evlist->cpus, > top->evlist->threads) < 0) { > if (perf_evsel__fallback(counter, errno, msg, sizeof(msg))) { > - if (verbose) > + if (verbose > 0) > ui__warning("%s\n", msg); > goto try_again; > } > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c > index 40ef9b293d1b..256f1fac6f7e 100644 > --- a/tools/perf/builtin-trace.c > +++ b/tools/perf/builtin-trace.c > @@ -1399,7 +1399,7 @@ static struct syscall *trace__syscall_info(struct trace *trace, > return &trace->syscalls.table[id]; > > out_cant_read: > - if (verbose) { > + if (verbose > 0) { > fprintf(trace->output, "Problems reading syscall %d", id); > if (id <= trace->syscalls.max && trace->syscalls.table[id].name != NULL) > fprintf(trace->output, "(%s)", trace->syscalls.table[id].name); > @@ -1801,10 +1801,10 @@ static void print_location(FILE *f, struct perf_sample *sample, > bool print_dso, bool print_sym) > { > > - if ((verbose || print_dso) && al->map) > + if ((verbose > 0 || print_dso) && al->map) > fprintf(f, "%s@", al->map->dso->long_name); > > - if ((verbose || print_sym) && al->sym) > + if ((verbose > 0 || print_sym) && al->sym) > fprintf(f, "%s+0x%" PRIx64, al->sym->name, > al->addr - al->sym->start); > else if (al->map) > diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c > index f67bbb0aa36e..0544398d6e2d 100644 > --- a/tools/perf/pmu-events/json.c > +++ b/tools/perf/pmu-events/json.c > @@ -49,7 +49,7 @@ static char *mapfile(const char *fn, size_t *size) > int err; > int fd = open(fn, O_RDONLY); > > - if (fd < 0 && verbose && fn) { > + if (fd < 0 && verbose > 0 && fn) { > pr_err("Error opening events file '%s': %s\n", fn, > strerror(errno)); > } > diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c > index 28d1605b0338..88dc51f4c27b 100644 > --- a/tools/perf/tests/attr.c > +++ b/tools/perf/tests/attr.c > @@ -144,7 +144,7 @@ static int run_dir(const char *d, const char *perf) > int vcnt = min(verbose, (int) sizeof(v) - 1); > char cmd[3*PATH_MAX]; > > - if (verbose) > + if (verbose > 0) > vcnt++; > > snprintf(cmd, 3*PATH_MAX, PYTHON " %s/attr.py -d %s/attr/ -p %s %.*s", > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index 37e326bfd2dc..83c4669cbc5b 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -299,7 +299,7 @@ static int run_test(struct test *test, int subtest) > if (!dont_fork) { > pr_debug("test child forked, pid %d\n", getpid()); > > - if (!verbose) { > + if (verbose <= 0) { > int nullfd = open("/dev/null", O_WRONLY); > > if (nullfd >= 0) { > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c > index ff5bc6363a79..d1f693041324 100644 > --- a/tools/perf/tests/code-reading.c > +++ b/tools/perf/tests/code-reading.c > @@ -599,7 +599,7 @@ static int do_test_code_reading(bool try_kcore) > continue; > } > > - if (verbose) { > + if (verbose > 0) { > char errbuf[512]; > perf_evlist__strerror_open(evlist, errno, errbuf, sizeof(errbuf)); > pr_debug("perf_evlist__open() failed!\n%s\n", errbuf); > diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c > index a2b5ff9bf83d..bc5982f42dc3 100644 > --- a/tools/perf/tests/fdarray.c > +++ b/tools/perf/tests/fdarray.c > @@ -19,7 +19,7 @@ static int fdarray__fprintf_prefix(struct fdarray *fda, const char *prefix, FILE > { > int printed = 0; > > - if (!verbose) > + if (verbose <= 0) > return 0; > > printed += fprintf(fp, "\n%s: ", prefix); > diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c > index d357dab72e68..482b5365e68d 100644 > --- a/tools/perf/tests/llvm.c > +++ b/tools/perf/tests/llvm.c > @@ -76,7 +76,7 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf, > * Skip this test if user's .perfconfig doesn't set [llvm] section > * and clang is not found in $PATH, and this is not perf test -v > */ > - if (!force && (verbose == 0 && > + if (!force && (verbose <= 0 && > !llvm_param.user_set_param && > llvm__search_clang())) { > pr_debug("No clang and no verbosive, skip this test\n"); > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c > index aa9276bfe3e9..1dc838014422 100644 > --- a/tools/perf/tests/parse-events.c > +++ b/tools/perf/tests/parse-events.c > @@ -1808,7 +1808,7 @@ static void debug_warn(const char *warn, va_list params) > { > char msg[1024]; > > - if (!verbose) > + if (verbose <= 0) > return; > > vsnprintf(msg, sizeof(msg), warn, params); > diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c > index 541da7a68f91..87893f3ba5f1 100644 > --- a/tools/perf/tests/perf-record.c > +++ b/tools/perf/tests/perf-record.c > @@ -172,13 +172,13 @@ int test__PERF_RECORD(int subtest __maybe_unused) > > err = perf_evlist__parse_sample(evlist, event, &sample); > if (err < 0) { > - if (verbose) > + if (verbose > 0) > perf_event__fprintf(event, stderr); > pr_debug("Couldn't parse sample\n"); > goto out_delete_evlist; > } > > - if (verbose) { > + if (verbose > 0) { > pr_info("%" PRIu64" %d ", sample.time, sample.cpu); > perf_event__fprintf(event, stderr); > } > diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c > index 7a52834ee0d0..fa79509da535 100644 > --- a/tools/perf/tests/python-use.c > +++ b/tools/perf/tests/python-use.c > @@ -15,7 +15,7 @@ int test__python_use(int subtest __maybe_unused) > int ret; > > if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s", > - PYTHONPATH, PYTHON, verbose ? "" : "2> /dev/null") < 0) > + PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0) > return -1; > > ret = system(cmd) ? -1 : 0; > diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c > index a4a4b4625ac3..f2d2e542d0ee 100644 > --- a/tools/perf/tests/thread-map.c > +++ b/tools/perf/tests/thread-map.c > @@ -109,7 +109,7 @@ int test__thread_map_remove(int subtest __maybe_unused) > TEST_ASSERT_VAL("failed to allocate thread_map", > threads); > > - if (verbose) > + if (verbose > 0) > thread_map__fprintf(threads, stderr); > > TEST_ASSERT_VAL("failed to remove thread", > @@ -117,7 +117,7 @@ int test__thread_map_remove(int subtest __maybe_unused) > > TEST_ASSERT_VAL("thread_map count != 1", threads->nr == 1); > > - if (verbose) > + if (verbose > 0) > thread_map__fprintf(threads, stderr); > > TEST_ASSERT_VAL("failed to remove thread", > @@ -125,7 +125,7 @@ int test__thread_map_remove(int subtest __maybe_unused) > > TEST_ASSERT_VAL("thread_map count != 0", threads->nr == 0); > > - if (verbose) > + if (verbose > 0) > thread_map__fprintf(threads, stderr); > > TEST_ASSERT_VAL("failed to not remove thread", > diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c > index a5082331f246..862b043e5924 100644 > --- a/tools/perf/tests/vmlinux-kallsyms.c > +++ b/tools/perf/tests/vmlinux-kallsyms.c > @@ -168,7 +168,7 @@ int test__vmlinux_matches_kallsyms(int subtest __maybe_unused) > err = -1; > } > > - if (!verbose) > + if (verbose <= 0) > goto out; > > header_printed = false; > diff --git a/tools/perf/ui/browsers/map.c b/tools/perf/ui/browsers/map.c > index 98a34664bb7e..9ce142de536d 100644 > --- a/tools/perf/ui/browsers/map.c > +++ b/tools/perf/ui/browsers/map.c > @@ -73,7 +73,7 @@ static int map_browser__run(struct map_browser *browser) > > if (ui_browser__show(&browser->b, browser->map->dso->long_name, > "Press ESC to exit, %s / to search", > - verbose ? "" : "restart with -v to use") < 0) > + verbose > 0 ? "" : "restart with -v to use") < 0) > return -1; > > while (1) { > @@ -81,7 +81,7 @@ static int map_browser__run(struct map_browser *browser) > > switch (key) { > case '/': > - if (verbose) > + if (verbose > 0) > map_browser__search(browser); > default: > break; > @@ -117,7 +117,7 @@ int map__browse(struct map *map) > > if (maxaddr < pos->end) > maxaddr = pos->end; > - if (verbose) { > + if (verbose > 0) { > u32 *idx = symbol__browser_index(pos); > *idx = mb.b.nr_entries; > } > diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c > index 18cfcdc90356..5d632dca672a 100644 > --- a/tools/perf/ui/hist.c > +++ b/tools/perf/ui/hist.c > @@ -648,7 +648,7 @@ unsigned int hists__sort_list_width(struct hists *hists) > ret += fmt->width(fmt, &dummy_hpp, hists); > } > > - if (verbose && hists__has(hists, sym)) /* Addr + origin */ > + if (verbose > 0 && hists__has(hists, sym)) /* Addr + origin */ > ret += 3 + BITS_PER_LONG / 4; > > return ret; > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 06cc04e5806a..273f21fa32b5 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -1768,7 +1768,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, > printf("%-*.*s----\n", > graph_dotted_len, graph_dotted_len, graph_dotted_line); > > - if (verbose) > + if (verbose > 0) > symbol__annotate_hits(sym, evsel); > > list_for_each_entry(pos, ¬es->src->source, node) { > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 35f5b7b7715c..28fb62c32678 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -594,7 +594,7 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp, > pr_debug("try to find information at %" PRIx64 " in %s\n", addr, > tp->module ? : "kernel"); > > - dinfo = debuginfo_cache__open(tp->module, verbose == 0); > + dinfo = debuginfo_cache__open(tp->module, verbose <= 0); > if (dinfo) > ret = debuginfo__find_probe_point(dinfo, > (unsigned long)addr, pp); > diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c > index 39345c2ddfc2..0d51334a9b46 100644 > --- a/tools/perf/util/stat.c > +++ b/tools/perf/util/stat.c > @@ -344,7 +344,7 @@ int perf_stat_process_counter(struct perf_stat_config *config, > for (i = 0; i < 3; i++) > update_stats(&ps->res_stats[i], count[i]); > > - if (verbose) { > + if (verbose > 0) { > fprintf(config->output, "%s: %" PRIu64 " %" PRIu64 " %" PRIu64 "\n", > perf_evsel__name(counter), count[0], count[1], count[2]); > }
| |