linux-kernel - Re: [PATCH] printk: Move printk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170714151738.GC32632@pathway.suse.cz>
Date:   Fri, 14 Jul 2017 17:17:38 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] printk: Move printk_delay to separate file

On Fri 2017-07-07 22:44:19, Joe Perches wrote:
> On Sat, 2017-07-08 at 14:24 +0900, Sergey Senozhatsky wrote:
> > On (07/07/17 11:08), Joe Perches wrote:
> > > printk.c is a huge file with too many local functions for a
> > > human to read and easily parse.
> > > 
> > > Start to separate out bits into smaller files.
> > > 
> > > Miscellanea:
> > > 
> > > o Rename suppress_message_printing to printk_suppress_message
> > > o Add function definitions to printk.h
> > 
> > I don't mind, in general, but I'm a bit hesitant. we want to have
> > automatic printk throttling (printk delay basically) and we need
> > some of those printk-internal *_seq numbers to see how far consoles
> > are behind the logbuf. so either we need to 'un-static' those *_seq
> > and extern them in delay.c or simply keep printk-delay machinery in
> > printk.c and add the new function.

I agree with Sergey here. Some split would make sense but I would
prefer to keep the delay stuff as is for now. It is not a big win.
And there is some demand to extent the throttling capabilities.
It would fit together with the delay stuff. But we did not think
much about it yet.

> > // p.s. I'll take a look at the patch a bit later. I'm on a sick leave now.
> 
> Hey Sergey.
> 
> Basically, this is a simple trial patch.
> 
> printk is getting nothing but more complex.
> 
> I believe printk is in real need of logical separation
> into multiple parts to isolate the various logic bits.
> 
> o console

I think that this might be the biggest win. IMHO, one confusing
thing is that big parts of printk.c are compiled only when
CONFIG_PRINTK is defined. There there are some small parts
that are always compiled. These are mostly console related.
I am sometimes not sure what is in what section and it is
"hard" to find.

> o kmsg/devkmsg

Sounds good. Well, I wonder how much code is shared with
syslog. It might make sense to join this stuff. But let's
see how it would look.

In each case, there are some functions (msg_print_text, ...)
that are shared between console, kmsg, and syslog. We would
need to put this somewhere and share.

> o logbuf

I do not have a strong opinion about this. I would leave this
for later when we see what has left in printk.c

I am going to comment some details in the existing patch.

Best Regards,
Petr