[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAM9d7ci0B03fKHydrXXPEnJOVbWA3tpirxec_A3VE3NsPU2rRw@mail.gmail.com>
Date: Mon, 17 Oct 2022 16:47:29 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: James Clark <james.clark@....com>,
linux-perf-users <linux-perf-users@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH] perf: Fix "Track with sched_switch" test by not printing
warnings in quiet mode
On Mon, Oct 17, 2022 at 5:33 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Fri, Oct 14, 2022 at 10:47:34AM +0100, James Clark escreveu:
> > On 13/10/2022 17:57, Namhyung Kim wrote:
> > > On Wed, Oct 12, 2022 at 10:12 AM James Clark <james.clark@....com> wrote:
> > >> On 12/10/2022 17:50, Namhyung Kim wrote:
> > >>> On Wed, Oct 12, 2022 at 4:13 AM James Clark <james.clark@....com> wrote:
> > >>>>> 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?
> > >
> > > I don't have any specific use case. If it's already suppressed in other
> > > cases, I'm fine with it.
> > >
> >
> > Actually I may have been mistaken. Seems like quiet is only used for
> > "extra info" type messages rather than warnings. Although the commit
> > message does say:
> >
> > The -q/--quiet option is to suppress any message. Sometimes users just
> > want to see the numbers and it can be used for that case.
> >
> > With 'any' that I would take to include warnings as well. I could move
> > warnings to stderr, but this has a much greater chance of breaking
> > anyone's workflows that might be looking for warnings on stdout than
> > removing warnings when -q is provided.
> >
> > Also if warnings are moved to stderr and quiet isn't used, there would
> > be no way to suppress warnings in the TUI which might actually be a
> > useful feature.
> >
> > So I'm still leaning towards the original change, if you are ok with
> > that even though it's not done elsewhere?
>
> Namhyung? I tend to agree with James.
I think it's a matter of choice. With this change, users won't see
warnings with -q anymore. This MIGHT affect some users as they
can see low quality data silently due to missing data or something.
If you're ok with the behavior, I'm fine. But then we need to update
the document to clarify the behavior.
Thanks,
Namhyung
Powered by blists - more mailing lists