Messages in this thread | | | Date | Mon, 19 Dec 2022 12:28:20 +0200 | Subject | Re: [PATCH] perf tool: Fix output unexpected messages in quiet mode | From | Adrian Hunter <> |
| |
On 19/12/22 11:28, Yang Jihong wrote: > Hello, > > On 2022/12/19 14:59, Adrian Hunter wrote: >> On 19/12/22 04:49, Yang Jihong wrote: >>> When perf uses quiet mode, perf_quiet_option sets debug_peo_args to -1, >> >> Seems like redirect_to_stderr has similar issue? > The redirect_to_stderr is used only in the veprintf function: > > int veprintf(int level, int var, const char *fmt, va_list args) > { > int ret = 0; > > if (var >= level) { > if (use_browser >= 1 && redirect_to_stderr <= 0) { > ui_helpline__vshow(fmt, args); > } else { > ret = fprintf_time(debug_file); > ret += vfprintf(debug_file, fmt, args); > } > } > > return ret; > } > > If use quiet mode, verbose sets to -1. (also assigned in the perf_quiet_option function) > Because "var >= level" is false, veprintf function returns directly, which avoids this problem. > > However, there are cases where: > # perf --debug stderr=-1 report -vvv 2>/tmp/debug > > If stderr is -1, should we redirect pr_debug in this case?
tools/perf/Documentation/perf.txt says debug variables are in value range (0, 10), so -1 is invalid anyway.
> > Because I'm not sure if this is a problem, > if redirect_to_stderr needs to be fixed as well, > let me know and I'll submit a patch to fix it.
perf_quiet_option() sets redirect_to_stderr = -1 with the intention that it turns it off, but it doesn't, although it gets turned of by verbose = -1. Perhaps set redirect_to_stderr = 0 in perf_quiet_option() ?
I see we have another problem as well: places that check "if (verbose)" instead of "if (verbose > 0)"
> >> >>> 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 >>> # >>> >>> Fixes: ccd26741f5e6 ("perf tool: Provide an option to print perf_event_open args and return value") >>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com> >> >> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> >> > Thanks for reviewed-by. > > Thanks, > Yang
| |