[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170406003819.dgbkdixvh6tn7hk5@Haydn>
Date: Wed, 5 Apr 2017 17:38:19 -0700
From: Calvin Owens <calvinowens@...com>
To: Petr Mladek <pmladek@...e.com>
CC: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
Steven Rostedt <rostedt@...dmis.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Jiri Slaby" <jslaby@...e.cz>,
Andrew Morton <akpm@...ux-foundation.org>,
Manuel Schölling <manuel.schoelling@....de>,
Hans de Goede <hdegoede@...hat.com>,
Paul Burton <paul.burton@...tec.com>,
<linux-kernel@...r.kernel.org>, <kernel-team@...com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [RFC][PATCH 1/2] printk: Introduce per-console filtering of
messages by loglevel
On Wednesday 04/05 at 17:22 +0200, Petr Mladek wrote:
> On Wed 2017-04-05 11:16:28, Sergey Senozhatsky wrote:
> > On (04/05/17 11:08), Sergey Senozhatsky wrote:
> > [..]
> > > > stop_critical_timings(); /* don't trace print latency */
> > > > - call_console_drivers(ext_text, ext_len, text, len);
> > > > + call_console_drivers(ext_text, ext_len, text, len, msg->level);
> > > > start_critical_timings();
> > > > printk_safe_exit_irqrestore(flags);
> > >
> > > ok, so the idea is quite clear and reasonable.
> > >
> > >
> > > some thoughts,
> > > we have a system-wide suppress_message_printing() loglevel filtering
> > > in console_unlock() loop, which sets a limit on loglevel for all of
> > > the messages - we don't even msg_print_text() if the message has
> > > suppressible loglevel. and this implicitly restricts per-console
> > > maxlevels.
> > >
> > > console_unlock()
> > > {
> > > for (;;) {
> > > ...
> > > skip:
> > >
> > > if (suppress_message_printing(msg->level)) // console_loglevel
> > > goto skip;
> > >
> > > call_console_drivers(msg->level)
> > > {
> > > if (level > con->maxlevel) // con loglevel
> > > continue;
> > > ...
> > > }
> > > }
> > > }
> > >
> > > this can be slightly confusing. what do you think?
I think it makes sense as long as we're clear about the semantics: if a
message would normally be printed to the console according to the global
settings, this allows you to limit the loglevel a specific console will
print.
Petr suggested the opposite approach, I'll address that below.
> I think about a reasonable behavior. There seems to be three variables
> that are related and are in use:
>
> console_level
> minimum_console_loglevel
> ignore_loglevel
>
> The functions seems to be the following:
>
> + console_level defines the current maximum level of
> messages that appear on all enabled consoles; it
> allows to filter out less important ones
>
> + minimum_console_loglevel defines the minimum
> console_loglevel that might be set by userspace
> via syslog interface; it prevents userspace from
> hiding emergency messages
>
> + ignore_loglevel allows to see all messages
> easily; it is used for debugging
>
> IMPORTANT: console_level is increased in some special
> situations to see everything, e.g. in panic(), oops_begin(),
> __handle_sysrq().
>
> I guess that people want to see all messages even on the slow
> console during panic(), oops(), with ignore_loglevel. It means
> that the new per-console setting must not limit it. Also any
> console must not go below minimum_console_level.
I can definitely take oops_in_progress and minimum_console_level into
account in the drop condition. I can also send a patch to make the sysrq
handler reset all the maxlevels to LOGLEVEL_DEBUG if you like.
> What about doing it the other way and define min_loglevel
> for each console. It might be used to make selected consoles
> always more verbose (above current console_level) but it
> will not limit the more verbose modes.
I think it's more intuitive to let the global sysctl behave as it always
has, and allow additional filtering of higher levels downstream. I can
definitely see why users might find this a bit confusing, but IMHO
stacking two "filters" is more intuitive than a "filter" and a "bypass".
How about a read-only "functional_loglevel" attribute for each console
that displays:
max(min(console_level, con->maxlevel), minimum_console_level)
Would that make the semantics more obvious? I'll obviously also send
patches for Documentation once there's consensus about the interface.
Thanks,
Calvin
> Best Regards,
> Petr
Powered by blists - more mailing lists