[PATCH 0/3] Fixes for debug variables

[PATCH 0/3] Fixes for debug variables

[Yang Jihong]

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

[PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option

[Yang Jihong]

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

[PATCH 2/3] perf tools: Fix usage of the verbose variable

[Yang Jihong]

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

[PATCH 3/3] perf probe: Check -v and -q options in the right place

[Yang Jihong]

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

Re: [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option

[Adrian Hunter]

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

Re: [PATCH 2/3] perf tools: Fix usage of the verbose variable

[Adrian Hunter]

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

Re: [PATCH 3/3] perf probe: Check -v and -q options in the right place

[Adrian Hunter]

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

Re: [PATCH 1/3] perf tools: Set debug_peo_args and redirect_to_stderr to correct values in perf_quiet_option

[Arnaldo Carvalho de Melo]

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