| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Feb 2017 10:38:27 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Jiri Olsa <jolsa@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, kernel-team@....com
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]);
> }
Powered by blists - more mailing lists