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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1500047134.4457.74.camel@perches.com>
Date:   Fri, 14 Jul 2017 08:45:34 -0700
From:   Joe Perches <joe@...ches.com>
To:     Petr Mladek <pmladek@...e.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-14 at 17:17 +0200, Petr Mladek wrote:
> 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.

Start somewhere without regard to whatever new
stuff you have future
dreams to add.

The concept of logical separation is a big win.

Moving delay into a separate file is trivial and
making the symbols required to support whatever
symbols are required for additional throttling
is also trivial.

> > 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.

It's relatively simple to prefix the various
bits that have to become shared with printk_
and make them global symbols in separate files
in the kernel/printk/ directory.

It's also unfortunately tedious.

I did all of that several years ago.

https://lkml.org/lkml/2012/10/17/41