Hello,
A big thanks to everybody who read and replied to my first e-mail; I

have tried my best to incorporate your feedback and suggestions. I

also added some CCs who recently participated in logging-related

discussions.
Changes (since Sept. 22):
  * Extensibility -> Allowing the compiler to eliminate messages below

    a certain threshold requires changing the API.

  * Add some special-purpose logging functions

    (printk_detected(), _registered(), _settings(), and _copyright())

  * Fine-grained log-level control. "Everything above" or "everything

    below" can be emulated by turning the specific log-levels on or off.

  * Define an extra header containing the (optional) secondary

    interface (err()/warn()/info())

  * Remove kprint_*() aliases.

  * kprint_<level>() is better than kprint(<level), ) because in the

    case of the default log-level, the argument can be omitted.

  * Memory allocated for entries and arguments is done in a ring-buffer

    with variable-sized chunks. Arguments are chained with a singly-

    linked list.

  * Use SUBSYSTEM and KBUILD_MODNAME

  * Rename kprint buffers to kprint blocks
Vegard
1. Requirements
   * Backwards compatibility with printk(), syslog(), etc. There is no

way the whole kernel can be converted to a new interface in one go.

printk() is used all over the kernel, in many different ways,

including calls from assembly, multi-line prints spread over several

calls, etc.
   * Extensibility. Features like eliminating messages below a given

threshold or recording the location (i.e. source file/line) of a

message [1] should be both selectable at compile-time and (if compiled

in) at run-time.
2. API
2.1. linux/kprint.h
  This header defines the primary (i.e. lowest-level) interface to

kprint that is made available to the rest of the kernel.
2.1.1. kprint()
	#define kprint(fmt, ...)
  This function is the equivalent of the old printk(), except that it

does not take a log-level parameter, but e...
[ message continues ]

This changes to KPRINT_SUBSYSTEM and KPRINT_DRIVER. The KPRINT_ prefix

is to clearly say that this is something related to logging. The

reason we can't use KBUILD_MODNAME is that this is defined on the

command line. The declaration inside the header would thus be horribly

wrong. We can, however, use KBUILD_MODNAME as a default value for

KPRINT_DRIVER, like:

static const char *KPRINT_DRIVER = KBUILD_MODNAME;

which would pre-process to something like:

static const char *KPRINT_DRIVER = "bcm43xx";
This value can still be overridden using #define KPRINT_DRIVER "new

name". In this case, it is possible that the original KPRINT_DRIVER

symbol can cause an "unused variable"-warning. I guess this is fixable

with the gcc "unused" variable attribute.
Vegard

-

Which has been known to result in the string getting written out to the .o

file even if it's never used, just in case something tries to take its 
Not with -D on the command line though.  Your #define would have to come after

the declaration or else the declaration turns into 'char *"fred" = "george";'

and you have a syntax error.  Again, not synonymous with a #define...
Rob

--

"One of my most productive days was throwing away 1000 lines of code."

  - Ken Thompson.

-

Logic tells me that an unused static variable should never go into the
Yeah, that's exactly what my e-mail was about. The macros

KPRINT_SUBSYSTEM and KPRINT_DRIVER are not defined on the command

line, but in each source file that wants this prefix, after the

variables with the same names have been declared in kprint.h. This is

intentional; they can be overridden with a define, otherwise, they'll

default to static-const string variables.
Vegard

-

Nice. Although the word "DRIVER" may not represent every module, I

think it is the correct option as I suggested, as the word is

meaningful (speaks by itself) and almost every message in the kernel

is printed out by drivers (whatever the big subsystem they belong to:
Yep, then, in a year or two, we will be able to delete such attribute.
Will there be a team to change main subsystems/drivers to the new API?
--

Miguel Ojeda

http://maxextreme.googlepages.com/index.htm

-

Actually, no, since it will throw a warning only if a source file

#defines KPRINT_SUBSYSTEM (i.e. overrides the constant variable

(oxymoron!) with the same name). What you're hoping is that some time

in the future, EVERY source file will come equipped with these

definitions, and yes, at that point, the entire declaration can be
No. First of all, this is a specification draft; there is no code yet.

Also, very possibly, this is such a violent change that nobody really

wants to use it anyway. But we can hope. ;-)
Vegard

-

Yes, that was my point. Far far far away, but possible, and if this

RFC ever meets the real kernel, then bringing every source file to the

API should be a objective. A good project for kernel janitors, for
--

Miguel Ojeda

http://maxextreme.googlepages.com/index.htm

-

This is overall, quite a lot. I suggest one-thing-at-a-time,

starting with kprint_<level>() that is compiled out if desired

and no fancy block or newline stuff.
_Then_, will see how it flies. All of this smells like a bit of

overdesigning, aka http://en.wikipedia.org/wiki/YAGNI

-

Well, that's why I'm writing it down before actually coding it.

Designing is also trying to make things fit together without actually

having the physical parts at hand. So I'm trying to make an interface

that CAN support multi-line blocks in the future, since it's obviously

a desired (and currently missing) feature.
But I agree; It *is* hard to see how multi-line blocks can be

implemented without actually spelling it out in code. I've tried to do

it, and failed. Until a brilliant solution comes up, I'll skip it.
Vegard

-

If-blocks spanning macros are really dangerous!
E.g. an Ethernet driver may want to do:
	kprint_block(&block, "MAC ");

	for (i = 0; i < 6; i++) {

		card->mac[i] = obtain_mac_byte_from_hw(i);

		kprint_block(&block, "%02x", card->mac[i]);

	}
This looks (and should be) innocent, but the actual MAC addres retrieval

would never be executed if loglevel <= CONFIG_KPRINT_LOGLEVEL_MAX.
Can't you store the loglevel in the kprint_block and check it in all

successive kprint_*() macros? If gcc knows it's constant, it can optimize

the non-wanted code away. As other fields in struct kprint_block cannot be

constant (they store internal state), you have to split it like:
    struct kprint_block {

	    int loglevel;

	    struct real_kprint_block real;  /* internal state */

    }
and pass &block.real() instead of &block to all successive internal functions.

I haven't tried this, so let's hope gcc is actually smart enough...
Gr{oetje,eeting}s,
						Geert
--

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But

when I'm talking to journalists I just say "programmer" or something like that.

							    -- Linus Torvalds

-

It isn't, apparently. Or not with my test, anyway. Either way, it's

probably better not to make those assumptions about or rely too much

on the smartness of the compiler (we don't have *any* guarantees).
The best solution for now is probably to pass the log-level into each

line, as Dick Streefland suggested, though it would lead to a hairier

syntax, or just skip the whole interface for now, as Jan Engelhardt

suggested. Thanks.
Vegard

-

Well actually, I believe you could just do:
struct kprint_block {

	const int loglevel;

	[...];

};
Then cast away the constness to actually set it initially:

*((int *)&block.loglevel) = LOGLEVEL;
Cheers,

Kyle Moffett
-

This doesn't seem to work either (i.e. it is not optimized out). I

tried initializing the struct statically too, to no avail. But thanks

for the tip.
Vegard

-

Assuming that kprint_block_flush() is a combination of

kprint_block_printit() and kprint_block_abort(), you

coulld make a macro wrapper for this to preclude leaks:
#define KPRINT_BLOCK(block, level, code) \

do { \

	struct kprint_block block; \

	kprint_block_init(&block, KPRINT_##level); \

	do { \

		code ; \

		kprint_block_printit(&block); \

	while (0); \

	kprint_block_abort(&block); \

} while(0)
The inner do { } while(0) region is so you can abort with "break".
(Or you can split it into KPRINT_BEGIN() and KPRINT_END() macros,

if that works out to be cleaner.)

-