Re: [PATCH] perf: Fix "Track with sched_switch" test by not printing warnings in quiet mode

[James Clark <james.clark@arm.com>]

On 12/10/2022 17:50, Namhyung Kim wrote:
> On Wed, Oct 12, 2022 at 4:13 AM James Clark <james.clark@arm.com> wrote:
>>
>>
>>
>> On 12/10/2022 12:10, James Clark wrote:
>>> Especially when CONFIG_LOCKDEP and other debug configs are enabled,
>>> Perf can print the following warning when running the "Track with
>>> sched_switch" test:
>>
>> Oops got the wrong test name here and in the title. Should be "kernel
>> lock contention analysis test"
> 
> Could you please resend?
> 
>>
>>>
>>>   Warning:
>>>   Processed 1378918 events and lost 4 chunks!
>>>
>>>   Check IO/CPU overload!
>>>
>>>   Warning:
>>>   Processed 4593325 samples and lost 70.00%!
>>>
>>> The test already supplies -q to run in quiet mode, so extend quiet mode
>>> to perf_stdio__warning() and also ui__warning() for consistency.
> 
> I'm not sure if suppressing the warnings with -q is a good thing.
> Maybe we need to separate warning/debug messages from the output.

I don't see the issue with warnings being suppressed in quiet mode as
long as errors are still printed. In other cases warnings have already
been suppressed by quiet mode and this site is the odd one out.

What use case are you thinking of where someone explicitly adds -q but
wants to see non fatal warnings?

Thanks
James

> 
> Thanks,
> Namhyung
> 
> 
>>>
>>> This fixes the following failure due to the extra lines counted:
>>>
>>>   perf test "lock cont" -vvv
>>>
>>>   82: kernel lock contention analysis test                            :
>>>   --- start ---
>>>   test child forked, pid 3125
>>>   Testing perf lock record and perf lock contention
>>>   [Fail] Recorded result count is not 1: 9
>>>   test child finished with -1
>>>   ---- end ----
>>>   kernel lock contention analysis test: FAILED!
>>>
>>> Fixes: ec685de25b67 ("perf test: Add kernel lock contention test")
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>> ---
>>>  tools/perf/ui/util.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/tools/perf/ui/util.c b/tools/perf/ui/util.c
>>> index 689b27c34246..1d38ddf01b60 100644
>>> --- a/tools/perf/ui/util.c
>>> +++ b/tools/perf/ui/util.c
>>> @@ -15,6 +15,9 @@ static int perf_stdio__error(const char *format, va_list args)
>>>
>>>  static int perf_stdio__warning(const char *format, va_list args)
>>>  {
>>> +     if (quiet)
>>> +             return 0;
>>> +
>>>       fprintf(stderr, "Warning:\n");
>>>       vfprintf(stderr, format, args);
>>>       return 0;
>>> @@ -45,6 +48,8 @@ int ui__warning(const char *format, ...)
>>>  {
>>>       int ret;
>>>       va_list args;
>>> +     if (quiet)
>>> +             return 0;
>>>
>>>       va_start(args, format);
>>>       ret = perf_eops->warning(format, args);