Date: Thu, 6 Apr 2017 15:52:50 -0700
From: Calvin Owens <calvinowens@fb.com>
To: Petr Mladek <pmladek@suse.com>
CC: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Jiri Slaby" <jslaby@suse.cz>,
        Andrew Morton <akpm@linux-foundation.org>,
        Manuel =?utf-8?Q?Sch=C3=B6lling?= <manuel.schoelling@gmx.de>,
        Hans de Goede <hdegoede@redhat.com>,
        Paul Burton <paul.burton@imgtec.com>, <linux-kernel@vger.kernel.org>,
        <kernel-team@fb.com>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [RFC][PATCH 1/2] printk: Introduce per-console filtering of
 messages by loglevel
Message-ID: <20170406225250.tz7ltv7s4d4xjrxy@Haydn>
References: <bc1fe800930be76761b2bb4e6108fe5876a005c9.1491345440.git.calvinowens@fb.com>
 <20170405020800.GB11669@jagdpanzerIV.localdomain>
 <20170405021628.GC11669@jagdpanzerIV.localdomain>
 <20170405152256.GS3452@pathway.suse.cz>
 <20170406003819.dgbkdixvh6tn7hk5@Haydn>
 <20170406140251.GC27578@pathway.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20170406140251.GC27578@pathway.suse.cz>
User-Agent: NeoMutt/20170306 (1.8.0)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Apr 2017 22:52:58.6608 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR15MB1224
X-OriginatorOrg: fb.com
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-04-06_14:,,
 signatures=0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4447
Lines: 119

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