Hello Joe,
On (02/28/17 19:17), Joe Perches wrote:
> Can save the space that the KERN_<LEVEL> headers require.
>
> The biggest negative here is the %pV use which needs
> recursion and adds stack depth.
>
> $ size vmlinux.o* (defconfig, x86-64)
> text data bss dec hex filename
> 12586135 1909841 777528 15273504 e90e20 vmlinux.o.new
> 12590348 1909841 777528 15277717 e91e95 vmlinux.o.old
interesting. 4K.
[..]
> +#define define_pr_func(func, level) \
> +asmlinkage __visible int func(const char *fmt, ...) \
> +{ \
> + va_list args; \
> + int r; \
> + struct va_format vaf; \
> + \
> + va_start(args, fmt); \
> + vaf.fmt = fmt; \
> + vaf.va = &args; \
> + \
> + r = printk(level "%pV", &vaf); \
> + \
> + va_end(args); \
> + \
> + return r; \
> +} \
hm. that's really hacky (which is a compliment) and a bit complicated.
my quick thought was to tweak vprintk_emit() for 'facility != 0' so it
could get loglevel (and adjust lflags) from the passed level, not from
the text, and then do something like this
#define define_pr_func(func, level) asmlinkage __visible int func(const char
*fmt, ...)
{
va_start(args, fmt);
r = vprintk_emit(level[0], level[1], NULL, 0, fmt, args);
va_end();
}
but this won't do the trick. because func()->vprintk_emit() shortcut
disables the printk-safe mechanism:
func()->printk()->vprintk_func()->this_cpu(printk_context)::print()
this *probably* and *may be* works for dev_printk() /* assuming that
dev_printk() is never called fomr under sched/sempahore/etc locks */,
but for printk() in general it's a huge no-no-no.
so I see that you did the very same thing in 99bcf217183e02ebae for
dev_printk(), which is, once again, probably ok for dev_print(). and
now the question is do we want to add %pV recursion to non-dev
printk-s, and I'm, frankly, don't have any answers at the moment.
sorry.
there are already %pV pr_foo() calls in the kernel. and we even have %pV in
OOM path. now those calls are going to be %pV->%pV = recursion depth 2.
for example I can easily do this thing:
[ 38.006766] WARNING: CPU: 2 PID: 368 at lib/vsprintf.c:1901
format_decode+0x226/0x2fd
[ 38.006766] Please remove unsupported %] in format string
[..]
[ 38.006783] Call Trace:
[ 38.006783] dump_stack+0x68/0x92
[ 38.006784] __warn+0xc2/0xdd
[ 38.006784] warn_slowpath_fmt+0x4b/0x53
[ 38.006785] ? __lock_acquire+0x2ac/0x1501
[ 38.006785] format_decode+0x226/0x2fd
[ 38.006786] vsnprintf+0x89/0x3b7
[ 38.006786] pointer+0x1c3/0x378
[ 38.006787] vsnprintf+0x22d/0x3b7
[ 38.006787] pointer+0x1c3/0x378
[ 38.006788] vsnprintf+0x22d/0x3b7
[ 38.006788] vscnprintf+0xd/0x26
[ 38.006788] vprintk_emit+0x81/0x294
[ 38.006789] ? 0xffffffffa0096000
[ 38.006789] vprintk_default+0x1d/0x1f
[ 38.006790] vprintk_func+0x6c/0x73
[ 38.006790] ? 0xffffffffa0096000
[ 38.006791] printk+0x43/0x4b
[ 38.006791] pr_cont+0x56/0x5e
I wonder if it can get any worse than that. I sort of suspect
it can. in trivial case, dump_stack() can add at least one more
level of recursion by using pr_{err, etc.} instead of corresponding
printk(KERN_{ERR, etc}.
can it be even worse? um... NMI?
any thoughts?
dunno, at the moment I'm not really comfortable with %pV recursion
for every pr_foo() call.
-ss
