Re: [RFC][PATCH 1/2] printk: Introduce per-console filtering of messages by loglevel

[Calvin Owens <calvinowens@fb.com>]

On Thursday 04/06 at 16:02 +0200, Petr Mladek wrote:
> On Wed 2017-04-05 17:38:19, Calvin Owens wrote:
> > On Wednesday 04/05 at 17:22 +0200, Petr Mladek wrote:
> > > 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.
> 
> Please note that you must not call console_lock() in the sysrq
> handler. The function might sleep and it is irq context.
> By other words, you could not manipulate the console structures
> there.

Sure, I'd punt it to process context somehow.
 
> > > 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".
> 
> I do not have strong opinion here. I like the idea of this patch.
> Sadly, the console setting already is pretty confusing.
> 
> I know that many people, including me, have troubles to understand
> the meaning of the 4 numbers in /proc/sys/kernel/printk. They set
> 
> 	console_loglevel
> 	default_message_loglevel
> 	minimum_console_loglevel
> 	default_console_loglevel
> 
> And we are going to add another complexity :-(
> 
> 
> > How about a read-only "functional_loglevel" attribute for each console
> > that displays:
> > 
> > 	max(min(console_level, con->maxlevel), minimum_console_level)
> 
> I like this idea and it inspired me. What about creating the following
> structure under /sys
> 
>   /sys/consoles/<name1>/loglevel
> 		       /minimum_loglevel
> 	       /<name2>/loglevel
> 		       /minimum_loglevel
> 	       /loglevel
> 	       /minimum_loglevel
> 
> The semantic would be:
> 
>    + global loglevel will show the current default console_loglevel,
>      it must be above the global minimum_console_loglevel
> 
>    + the per-console loglevel will show the loglevel specific
>      for the given console; it must be above the per-console
>      minimum_loglevel while
> 
>    + the per-console minimum_loglevel must be above the global
>      minimum_console_loglevel
> 
>    The setting of the global values would affect the per-console
>    values but it must respect the above rules.
> 
>    It is still the "filter" and "bypass" logic. But we will just
>    repeat the existing terms and logic. Also note that
>    "ignore_loglevel" and the special modes in sysrq, panic, oops
>    use the "bypass" logic as well.

Okay, I see where you're coming from. Let me play with this a bit, I'll
send something concrete in the next day or two.

> > Would that make the semantics more obvious? I'll obviously also send
> > patches for Documentation once there's consensus about the interface.
> 
> Please add also linux-api@vger.kernel.org, especially for the
> patch adding the new toplevel directory under /sys.

Will do.

Thanks,
Calvin

> Best Regards,
> Petr