Re: [PATCH] printk: Move printk_delay to separate file
From: Joe Perches
Date: Fri Jul 14 2017 - 11:45:52 EST
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.