* [PATCH 0/3] Fixes for debug variables
@ 2022-12-20 3:56 Yang Jihong
2022-12-20 3:57 ` [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option Yang Jihong
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Yang Jihong @ 2022-12-20 3:56 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria,
martin.lau, adrian.hunter, ak, masami.hiramatsu.pt,
linux-perf-users, linux-kernel
Cc: yangjihong1
patch1 is a fix for debug_peo_args and redirect_to_stderr.
patch2 is a fix for the verbose variable.
patch3 is a correction for checking the quiet option.
Yang Jihong (3):
perf tools: Set debug_peo_args and redirect_to_stderr to correct
values in perf_quiet_option
perf tools: Fix usage of the verbose variable
perf probe: Check -v and -q options in the right place
tools/perf/builtin-lock.c | 6 +++---
tools/perf/builtin-probe.c | 17 +++++++++--------
tools/perf/builtin-record.c | 4 ++--
tools/perf/builtin-script.c | 2 +-
tools/perf/builtin-stat.c | 4 ++--
tools/perf/dlfilters/dlfilter-test-api-v0.c | 2 +-
tools/perf/tests/builtin-test.c | 2 +-
tools/perf/tests/dlfilter-test.c | 2 +-
tools/perf/util/bpf_lock_contention.c | 2 +-
tools/perf/util/debug.c | 4 ++++
tools/perf/util/dlfilter.c | 2 +-
11 files changed, 26 insertions(+), 21 deletions(-)
--
2.30.GIT
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option 2022-12-20 3:56 [PATCH 0/3] Fixes for debug variables Yang Jihong @ 2022-12-20 3:57 ` Yang Jihong 2022-12-20 7:35 ` Adrian Hunter 2022-12-20 3:57 ` [PATCH 2/3] perf tools: Fix usage of the verbose variable Yang Jihong 2022-12-20 3:57 ` [PATCH 3/3] perf probe: Check -v and -q options in the right place Yang Jihong 2 siblings, 1 reply; 8+ messages in thread From: Yang Jihong @ 2022-12-20 3:57 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, adrian.hunter, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel Cc: yangjihong1 When perf uses quiet mode, perf_quiet_option sets debug_peo_args to -1, and display_attr incorrectly determines the value of debug_peo_args. As a result, unexpected information is displayed. Before: # perf record --quiet -- ls > /dev/null ------------------------------------------------------------ perf_event_attr: size 128 { sample_period, sample_freq } 4000 sample_type IP|TID|TIME|PERIOD read_format ID|LOST disabled 1 inherit 1 mmap 1 comm 1 freq 1 enable_on_exec 1 task 1 precise_ip 3 sample_id_all 1 exclude_guest 1 mmap2 1 comm_exec 1 ksymbol 1 bpf_event 1 ------------------------------------------------------------ ... After: # perf record --quiet -- ls > /dev/null # redirect_to_stderr is a similar problem. Fixes: f78eaef0e049 ("perf tools: Allow to force redirect pr_debug to stderr.") Fixes: ccd26741f5e6 ("perf tool: Provide an option to print perf_event_open args and return value") Suggested-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Yang Jihong <yangjihong1@huawei.com> --- tools/perf/util/debug.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c index 65e6c22f38e4..190e818a0717 100644 --- a/tools/perf/util/debug.c +++ b/tools/perf/util/debug.c @@ -241,6 +241,10 @@ int perf_quiet_option(void) opt++; } + /* For debug variables that are used as bool types, set to 0. */ + redirect_to_stderr = 0; + debug_peo_args = 0; + return 0; } -- 2.30.GIT ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option 2022-12-20 3:57 ` [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option Yang Jihong @ 2022-12-20 7:35 ` Adrian Hunter 2022-12-20 18:17 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2022-12-20 7:35 UTC (permalink / raw) To: Yang Jihong, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel On 20/12/22 05:57, Yang Jihong wrote: > When perf uses quiet mode, perf_quiet_option sets debug_peo_args to -1, > and display_attr incorrectly determines the value of debug_peo_args. > As a result, unexpected information is displayed. > > Before: > # perf record --quiet -- ls > /dev/null > ------------------------------------------------------------ > perf_event_attr: > size 128 > { sample_period, sample_freq } 4000 > sample_type IP|TID|TIME|PERIOD > read_format ID|LOST > disabled 1 > inherit 1 > mmap 1 > comm 1 > freq 1 > enable_on_exec 1 > task 1 > precise_ip 3 > sample_id_all 1 > exclude_guest 1 > mmap2 1 > comm_exec 1 > ksymbol 1 > bpf_event 1 > ------------------------------------------------------------ > ... > > After: > # perf record --quiet -- ls > /dev/null > # > > redirect_to_stderr is a similar problem. > > Fixes: f78eaef0e049 ("perf tools: Allow to force redirect pr_debug to stderr.") > Fixes: ccd26741f5e6 ("perf tool: Provide an option to print perf_event_open args and return value") > Suggested-by: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/util/debug.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c > index 65e6c22f38e4..190e818a0717 100644 > --- a/tools/perf/util/debug.c > +++ b/tools/perf/util/debug.c > @@ -241,6 +241,10 @@ int perf_quiet_option(void) > opt++; > } > > + /* For debug variables that are used as bool types, set to 0. */ > + redirect_to_stderr = 0; > + debug_peo_args = 0; > + > return 0; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option 2022-12-20 7:35 ` Adrian Hunter @ 2022-12-20 18:17 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2022-12-20 18:17 UTC (permalink / raw) To: Adrian Hunter Cc: Yang Jihong, peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel Em Tue, Dec 20, 2022 at 09:35:36AM +0200, Adrian Hunter escreveu: > On 20/12/22 05:57, Yang Jihong wrote: > > When perf uses quiet mode, perf_quiet_option sets debug_peo_args to -1, > > and display_attr incorrectly determines the value of debug_peo_args. > > As a result, unexpected information is displayed. > > > > Before: > > # perf record --quiet -- ls > /dev/null > > ------------------------------------------------------------ > > perf_event_attr: > > size 128 > > { sample_period, sample_freq } 4000 > > sample_type IP|TID|TIME|PERIOD > > read_format ID|LOST > > disabled 1 > > inherit 1 > > mmap 1 > > comm 1 > > freq 1 > > enable_on_exec 1 > > task 1 > > precise_ip 3 > > sample_id_all 1 > > exclude_guest 1 > > mmap2 1 > > comm_exec 1 > > ksymbol 1 > > bpf_event 1 > > ------------------------------------------------------------ > > ... > > > > After: > > # perf record --quiet -- ls > /dev/null > > # > > > > redirect_to_stderr is a similar problem. > > > > Fixes: f78eaef0e049 ("perf tools: Allow to force redirect pr_debug to stderr.") > > Fixes: ccd26741f5e6 ("perf tool: Provide an option to print perf_event_open args and return value") > > Suggested-by: Adrian Hunter <adrian.hunter@intel.com> > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> > > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> Thanks, applied all patches in this series. - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] perf tools: Fix usage of the verbose variable 2022-12-20 3:56 [PATCH 0/3] Fixes for debug variables Yang Jihong 2022-12-20 3:57 ` [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option Yang Jihong @ 2022-12-20 3:57 ` Yang Jihong 2022-12-20 7:44 ` Adrian Hunter 2022-12-20 3:57 ` [PATCH 3/3] perf probe: Check -v and -q options in the right place Yang Jihong 2 siblings, 1 reply; 8+ messages in thread From: Yang Jihong @ 2022-12-20 3:57 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, adrian.hunter, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel Cc: yangjihong1 The data type of the verbose variable is integer and can be negative, replace improperly used cases in a unified manner: 1. if (verbose) => if (verbose > 0) 2. if (!verbose) => if (verbose <= 0) 3. if (XX && verbose) => if (XX && verbose > 0) 4. if (XX && !verbose) => if (XX && verbose <= 0) Signed-off-by: Yang Jihong <yangjihong1@huawei.com> --- tools/perf/builtin-lock.c | 6 +++--- tools/perf/builtin-record.c | 4 ++-- tools/perf/builtin-script.c | 2 +- tools/perf/builtin-stat.c | 4 ++-- tools/perf/dlfilters/dlfilter-test-api-v0.c | 2 +- tools/perf/tests/builtin-test.c | 2 +- tools/perf/tests/dlfilter-test.c | 2 +- tools/perf/util/bpf_lock_contention.c | 2 +- tools/perf/util/dlfilter.c | 2 +- 9 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c index 25c0a5e5051f..6276dfbc94a1 100644 --- a/tools/perf/builtin-lock.c +++ b/tools/perf/builtin-lock.c @@ -1029,7 +1029,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel, if (!ls) return -ENOMEM; - if (aggr_mode == LOCK_AGGR_CALLER && verbose) { + if (aggr_mode == LOCK_AGGR_CALLER && verbose > 0) { ls->callstack = get_callstack(sample, max_stack_depth); if (ls->callstack == NULL) return -ENOMEM; @@ -1214,7 +1214,7 @@ static void print_bad_events(int bad, int total) for (i = 0; i < BROKEN_MAX; i++) broken += bad_hist[i]; - if (quiet || (broken == 0 && !verbose)) + if (quiet || (broken == 0 && verbose <= 0)) return; pr_info("\n=== output for debug===\n\n"); @@ -1529,7 +1529,7 @@ static void print_contention_result(struct lock_contention *con) break; } - if (aggr_mode == LOCK_AGGR_CALLER && verbose) { + if (aggr_mode == LOCK_AGGR_CALLER && verbose > 0) { struct map *kmap; struct symbol *sym; char buf[128]; diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 8ecffa696ce3..29dcd454b8e2 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -3629,7 +3629,7 @@ static int record__init_thread_cpu_masks(struct record *rec, struct perf_cpu_map for (t = 0; t < rec->nr_threads; t++) { __set_bit(perf_cpu_map__cpu(cpus, t).cpu, rec->thread_masks[t].maps.bits); __set_bit(perf_cpu_map__cpu(cpus, t).cpu, rec->thread_masks[t].affinity.bits); - if (verbose) { + if (verbose > 0) { pr_debug("thread_masks[%d]: ", t); mmap_cpu_mask__scnprintf(&rec->thread_masks[t].maps, "maps"); pr_debug("thread_masks[%d]: ", t); @@ -3726,7 +3726,7 @@ static int record__init_thread_masks_spec(struct record *rec, struct perf_cpu_ma } rec->thread_masks = thread_masks; rec->thread_masks[t] = thread_mask; - if (verbose) { + if (verbose > 0) { pr_debug("thread_masks[%d]: ", t); mmap_cpu_mask__scnprintf(&rec->thread_masks[t].maps, "maps"); pr_debug("thread_masks[%d]: ", t); diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 88888fb885c8..69394ac0a20d 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -2233,7 +2233,7 @@ static void process_event(struct perf_script *script, if (PRINT_FIELD(METRIC)) perf_sample__fprint_metric(script, thread, evsel, sample, fp); - if (verbose) + if (verbose > 0) fflush(fp); } diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index bf640abc9c41..9f3e4b257516 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -266,7 +266,7 @@ static void evlist__check_cpu_maps(struct evlist *evlist) evsel__group_desc(leader, buf, sizeof(buf)); pr_warning(" %s\n", buf); - if (verbose) { + if (verbose > 0) { cpu_map__snprint(leader->core.cpus, buf, sizeof(buf)); pr_warning(" %s: %s\n", leader->name, buf); cpu_map__snprint(evsel->core.cpus, buf, sizeof(buf)); @@ -2493,7 +2493,7 @@ int cmd_stat(int argc, const char **argv) if (iostat_mode == IOSTAT_LIST) { iostat_list(evsel_list, &stat_config); goto out; - } else if (verbose) + } else if (verbose > 0) iostat_list(evsel_list, &stat_config); if (iostat_mode == IOSTAT_RUN && !target__has_cpu(&target)) target.system_wide = true; diff --git a/tools/perf/dlfilters/dlfilter-test-api-v0.c b/tools/perf/dlfilters/dlfilter-test-api-v0.c index b17eb52a0694..b1f51efd67d6 100644 --- a/tools/perf/dlfilters/dlfilter-test-api-v0.c +++ b/tools/perf/dlfilters/dlfilter-test-api-v0.c @@ -119,7 +119,7 @@ struct perf_dlfilter_fns perf_dlfilter_fns; static int verbose; #define pr_debug(fmt, ...) do { \ - if (verbose) \ + if (verbose > 0) \ fprintf(stderr, fmt, ##__VA_ARGS__); \ } while (0) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index f6c16ad8ed50..cfa61493c750 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -305,7 +305,7 @@ static int shell_test__run(struct test_suite *test, int subdir __maybe_unused) path__join(script, sizeof(script) - 3, st->dir, st->file); - if (verbose) + if (verbose > 0) strncat(script, " -v", sizeof(script) - strlen(script) - 1); err = system(script); diff --git a/tools/perf/tests/dlfilter-test.c b/tools/perf/tests/dlfilter-test.c index 99aa72e425e4..086fd2179e41 100644 --- a/tools/perf/tests/dlfilter-test.c +++ b/tools/perf/tests/dlfilter-test.c @@ -88,7 +88,7 @@ static __printf(1, 2) int system_cmd(const char *fmt, ...) if (ret <= 0 || ret >= MAXCMD) return -1; - if (!verbose) + if (verbose <= 0) strcat(cmd, REDIRECT_TO_DEV_NULL); pr_debug("Command: %s\n", cmd); diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c index 8e1b791dc58f..df8dbb5191b4 100644 --- a/tools/perf/util/bpf_lock_contention.c +++ b/tools/perf/util/bpf_lock_contention.c @@ -215,7 +215,7 @@ int lock_contention_read(struct lock_contention *con) break; } - if (verbose) { + if (verbose > 0) { st->callstack = memdup(stack_trace, stack_size); if (st->callstack == NULL) break; diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c index 54e4d4495e00..37beb7530288 100644 --- a/tools/perf/util/dlfilter.c +++ b/tools/perf/util/dlfilter.c @@ -579,7 +579,7 @@ static void list_filters(const char *dirname) if (!get_filter_desc(dirname, entry->d_name, &desc, &long_desc)) continue; printf(" %-36s %s\n", entry->d_name, desc ? desc : ""); - if (verbose) { + if (verbose > 0) { char *p = long_desc; char *line; -- 2.30.GIT ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] perf tools: Fix usage of the verbose variable 2022-12-20 3:57 ` [PATCH 2/3] perf tools: Fix usage of the verbose variable Yang Jihong @ 2022-12-20 7:44 ` Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: Adrian Hunter @ 2022-12-20 7:44 UTC (permalink / raw) To: Yang Jihong, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel On 20/12/22 05:57, Yang Jihong wrote: > The data type of the verbose variable is integer and can be negative, > replace improperly used cases in a unified manner: > 1. if (verbose) => if (verbose > 0) > 2. if (!verbose) => if (verbose <= 0) > 3. if (XX && verbose) => if (XX && verbose > 0) > 4. if (XX && !verbose) => if (XX && verbose <= 0) > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/builtin-lock.c | 6 +++--- > tools/perf/builtin-record.c | 4 ++-- > tools/perf/builtin-script.c | 2 +- > tools/perf/builtin-stat.c | 4 ++-- > tools/perf/dlfilters/dlfilter-test-api-v0.c | 2 +- > tools/perf/tests/builtin-test.c | 2 +- > tools/perf/tests/dlfilter-test.c | 2 +- > tools/perf/util/bpf_lock_contention.c | 2 +- > tools/perf/util/dlfilter.c | 2 +- > 9 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index 25c0a5e5051f..6276dfbc94a1 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -1029,7 +1029,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel, > if (!ls) > return -ENOMEM; > > - if (aggr_mode == LOCK_AGGR_CALLER && verbose) { > + if (aggr_mode == LOCK_AGGR_CALLER && verbose > 0) { > ls->callstack = get_callstack(sample, max_stack_depth); > if (ls->callstack == NULL) > return -ENOMEM; > @@ -1214,7 +1214,7 @@ static void print_bad_events(int bad, int total) > for (i = 0; i < BROKEN_MAX; i++) > broken += bad_hist[i]; > > - if (quiet || (broken == 0 && !verbose)) > + if (quiet || (broken == 0 && verbose <= 0)) > return; > > pr_info("\n=== output for debug===\n\n"); > @@ -1529,7 +1529,7 @@ static void print_contention_result(struct lock_contention *con) > break; > } > > - if (aggr_mode == LOCK_AGGR_CALLER && verbose) { > + if (aggr_mode == LOCK_AGGR_CALLER && verbose > 0) { > struct map *kmap; > struct symbol *sym; > char buf[128]; > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 8ecffa696ce3..29dcd454b8e2 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -3629,7 +3629,7 @@ static int record__init_thread_cpu_masks(struct record *rec, struct perf_cpu_map > for (t = 0; t < rec->nr_threads; t++) { > __set_bit(perf_cpu_map__cpu(cpus, t).cpu, rec->thread_masks[t].maps.bits); > __set_bit(perf_cpu_map__cpu(cpus, t).cpu, rec->thread_masks[t].affinity.bits); > - if (verbose) { > + if (verbose > 0) { > pr_debug("thread_masks[%d]: ", t); > mmap_cpu_mask__scnprintf(&rec->thread_masks[t].maps, "maps"); > pr_debug("thread_masks[%d]: ", t); > @@ -3726,7 +3726,7 @@ static int record__init_thread_masks_spec(struct record *rec, struct perf_cpu_ma > } > rec->thread_masks = thread_masks; > rec->thread_masks[t] = thread_mask; > - if (verbose) { > + if (verbose > 0) { > pr_debug("thread_masks[%d]: ", t); > mmap_cpu_mask__scnprintf(&rec->thread_masks[t].maps, "maps"); > pr_debug("thread_masks[%d]: ", t); > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 88888fb885c8..69394ac0a20d 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -2233,7 +2233,7 @@ static void process_event(struct perf_script *script, > if (PRINT_FIELD(METRIC)) > perf_sample__fprint_metric(script, thread, evsel, sample, fp); > > - if (verbose) > + if (verbose > 0) > fflush(fp); > } > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index bf640abc9c41..9f3e4b257516 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -266,7 +266,7 @@ static void evlist__check_cpu_maps(struct evlist *evlist) > evsel__group_desc(leader, buf, sizeof(buf)); > pr_warning(" %s\n", buf); > > - if (verbose) { > + if (verbose > 0) { > cpu_map__snprint(leader->core.cpus, buf, sizeof(buf)); > pr_warning(" %s: %s\n", leader->name, buf); > cpu_map__snprint(evsel->core.cpus, buf, sizeof(buf)); > @@ -2493,7 +2493,7 @@ int cmd_stat(int argc, const char **argv) > if (iostat_mode == IOSTAT_LIST) { > iostat_list(evsel_list, &stat_config); > goto out; > - } else if (verbose) > + } else if (verbose > 0) > iostat_list(evsel_list, &stat_config); > if (iostat_mode == IOSTAT_RUN && !target__has_cpu(&target)) > target.system_wide = true; > diff --git a/tools/perf/dlfilters/dlfilter-test-api-v0.c b/tools/perf/dlfilters/dlfilter-test-api-v0.c > index b17eb52a0694..b1f51efd67d6 100644 > --- a/tools/perf/dlfilters/dlfilter-test-api-v0.c > +++ b/tools/perf/dlfilters/dlfilter-test-api-v0.c > @@ -119,7 +119,7 @@ struct perf_dlfilter_fns perf_dlfilter_fns; > static int verbose; > > #define pr_debug(fmt, ...) do { \ > - if (verbose) \ > + if (verbose > 0) \ > fprintf(stderr, fmt, ##__VA_ARGS__); \ > } while (0) > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index f6c16ad8ed50..cfa61493c750 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -305,7 +305,7 @@ static int shell_test__run(struct test_suite *test, int subdir __maybe_unused) > > path__join(script, sizeof(script) - 3, st->dir, st->file); > > - if (verbose) > + if (verbose > 0) > strncat(script, " -v", sizeof(script) - strlen(script) - 1); > > err = system(script); > diff --git a/tools/perf/tests/dlfilter-test.c b/tools/perf/tests/dlfilter-test.c > index 99aa72e425e4..086fd2179e41 100644 > --- a/tools/perf/tests/dlfilter-test.c > +++ b/tools/perf/tests/dlfilter-test.c > @@ -88,7 +88,7 @@ static __printf(1, 2) int system_cmd(const char *fmt, ...) > if (ret <= 0 || ret >= MAXCMD) > return -1; > > - if (!verbose) > + if (verbose <= 0) > strcat(cmd, REDIRECT_TO_DEV_NULL); > > pr_debug("Command: %s\n", cmd); > diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c > index 8e1b791dc58f..df8dbb5191b4 100644 > --- a/tools/perf/util/bpf_lock_contention.c > +++ b/tools/perf/util/bpf_lock_contention.c > @@ -215,7 +215,7 @@ int lock_contention_read(struct lock_contention *con) > break; > } > > - if (verbose) { > + if (verbose > 0) { > st->callstack = memdup(stack_trace, stack_size); > if (st->callstack == NULL) > break; > diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c > index 54e4d4495e00..37beb7530288 100644 > --- a/tools/perf/util/dlfilter.c > +++ b/tools/perf/util/dlfilter.c > @@ -579,7 +579,7 @@ static void list_filters(const char *dirname) > if (!get_filter_desc(dirname, entry->d_name, &desc, &long_desc)) > continue; > printf(" %-36s %s\n", entry->d_name, desc ? desc : ""); > - if (verbose) { > + if (verbose > 0) { > char *p = long_desc; > char *line; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] perf probe: Check -v and -q options in the right place 2022-12-20 3:56 [PATCH 0/3] Fixes for debug variables Yang Jihong 2022-12-20 3:57 ` [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option Yang Jihong 2022-12-20 3:57 ` [PATCH 2/3] perf tools: Fix usage of the verbose variable Yang Jihong @ 2022-12-20 3:57 ` Yang Jihong 2022-12-20 11:12 ` Adrian Hunter 2 siblings, 1 reply; 8+ messages in thread From: Yang Jihong @ 2022-12-20 3:57 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, adrian.hunter, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel Cc: yangjihong1 Check the -q and -v options first to return earlier on error. Before: # perf probe -q -v test probe-definition(0): test symbol:test file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Error: -v and -q are exclusive. After: # perf probe -q -v test Error: -v and -q are exclusive. Fixes: 5e17b28f1e24 ("perf probe: Add --quiet option to suppress output result message") Signed-off-by: Yang Jihong <yangjihong1@huawei.com> --- tools/perf/builtin-probe.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c index 2ae50fc9e597..ed73d0b89ca2 100644 --- a/tools/perf/builtin-probe.c +++ b/tools/perf/builtin-probe.c @@ -612,6 +612,15 @@ __cmd_probe(int argc, const char **argv) argc = parse_options(argc, argv, options, probe_usage, PARSE_OPT_STOP_AT_NON_OPTION); + + if (quiet) { + if (verbose != 0) { + pr_err(" Error: -v and -q are exclusive.\n"); + return -EINVAL; + } + verbose = -1; + } + if (argc > 0) { if (strcmp(argv[0], "-") == 0) { usage_with_options_msg(probe_usage, options, @@ -633,14 +642,6 @@ __cmd_probe(int argc, const char **argv) if (ret) return ret; - if (quiet) { - if (verbose != 0) { - pr_err(" Error: -v and -q are exclusive.\n"); - return -EINVAL; - } - verbose = -1; - } - if (probe_conf.max_probes == 0) probe_conf.max_probes = MAX_PROBES; -- 2.30.GIT ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] perf probe: Check -v and -q options in the right place 2022-12-20 3:57 ` [PATCH 3/3] perf probe: Check -v and -q options in the right place Yang Jihong @ 2022-12-20 11:12 ` Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: Adrian Hunter @ 2022-12-20 11:12 UTC (permalink / raw) To: Yang Jihong, peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, irogers, carsten.haitzler, leo.yan, ravi.bangoria, martin.lau, ak, masami.hiramatsu.pt, linux-perf-users, linux-kernel On 20/12/22 05:57, Yang Jihong wrote: > Check the -q and -v options first to return earlier on error. > > Before: > # perf probe -q -v test > probe-definition(0): test > symbol:test file:(null) line:0 offset:0 return:0 lazy:(null) > 0 arguments > Error: -v and -q are exclusive. > > After: > # perf probe -q -v test > Error: -v and -q are exclusive. > > Fixes: 5e17b28f1e24 ("perf probe: Add --quiet option to suppress output result message") > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/builtin-probe.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c > index 2ae50fc9e597..ed73d0b89ca2 100644 > --- a/tools/perf/builtin-probe.c > +++ b/tools/perf/builtin-probe.c > @@ -612,6 +612,15 @@ __cmd_probe(int argc, const char **argv) > > argc = parse_options(argc, argv, options, probe_usage, > PARSE_OPT_STOP_AT_NON_OPTION); > + > + if (quiet) { > + if (verbose != 0) { > + pr_err(" Error: -v and -q are exclusive.\n"); > + return -EINVAL; > + } > + verbose = -1; > + } > + > if (argc > 0) { > if (strcmp(argv[0], "-") == 0) { > usage_with_options_msg(probe_usage, options, > @@ -633,14 +642,6 @@ __cmd_probe(int argc, const char **argv) > if (ret) > return ret; > > - if (quiet) { > - if (verbose != 0) { > - pr_err(" Error: -v and -q are exclusive.\n"); > - return -EINVAL; > - } > - verbose = -1; > - } > - > if (probe_conf.max_probes == 0) > probe_conf.max_probes = MAX_PROBES; > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-20 18:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-20 3:56 [PATCH 0/3] Fixes for debug variables Yang Jihong 2022-12-20 3:57 ` [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option Yang Jihong 2022-12-20 7:35 ` Adrian Hunter 2022-12-20 18:17 ` Arnaldo Carvalho de Melo 2022-12-20 3:57 ` [PATCH 2/3] perf tools: Fix usage of the verbose variable Yang Jihong 2022-12-20 7:44 ` Adrian Hunter 2022-12-20 3:57 ` [PATCH 3/3] perf probe: Check -v and -q options in the right place Yang Jihong 2022-12-20 11:12 ` Adrian Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).