Re: [PATCH v2] x86/kernel: use pr_<level>() and dev_<level>

From: Borislav Petkov
Date: Sun Feb 14 2016 - 09:08:43 EST


On Sun, Feb 14, 2016 at 12:10:47PM +0800, Chen Yucong wrote:
> arch/x86/kernel/* use a mixture of printk(KERN_<level> ) and pr_<level>().
> This patch converts the bulk of printk(KERN_<level> ) to pr_<level>() and
> uses dev_dbg() instead of the dev_printk(KERN_DEBUG,). All pr_warning()
> calls have been replaced with pr_warn().
>
> Not sure what to do about the printk(KERN_DEFAULT) and printk() without a
> log level.

...

> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 25f9093..0ecb579 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -59,7 +59,7 @@ __setup("noreplace-paravirt", setup_noreplace_paravirt);
> #define DPRINTK(fmt, args...) \
> do { \
> if (debug_alternative) \
> - printk(KERN_DEBUG "%s: " fmt "\n", __func__, ##args); \
> + pr_debug("%s: " fmt "\n", __func__, ##args); \
> } while (0)
>
> #define DUMP_BYTES(buf, len, fmt, args...) \
> @@ -70,10 +70,10 @@ do { \
> if (!(len)) \
> break; \
> \
> - printk(KERN_DEBUG fmt, ##args); \
> + pr_debug(fmt, ##args); \
> for (j = 0; j < (len) - 1; j++) \
> - printk(KERN_CONT "%02hhx ", buf[j]); \
> - printk(KERN_CONT "%02hhx\n", buf[j]); \
> + pr_cont("%02hhx ", buf[j]); \
> + pr_cont("%02hhx\n", buf[j]); \
> } \
> } while (0)
>

NAK the hell out of that hunk!

Did you actually look at how pr_debug() is defined?

Yeah, I don't think so. With your change, when I boot with
"debug-alternative" I get:

...
[ 0.064005] Last level dTLB entries: 4KB 512, 2MB 255, 4MB 127, 1GB 0
[ 0.068005] e9 d5 3e d3 00
[ 0.072004] e9 e8 92 21 ff
[ 0.075003] eb 11 0f 1f 00
[ 0.077906] e8 c5 b6 30 00
[ 0.084009] f3 48 0f b8 c7
[ 0.091241] f3 48 0f b8 c7
[ 0.094259] e8 d9 b5 30 00
[ 0.097611] f3 48 0f b8 c7
[ 0.100004] f3 48 0f b8 c7
[ 0.103067] 90 90 90
[ 0.105575] 0f ae f0
[ 0.108004] 0f ae f0
[ 0.112007] 90 90 90
[ 0.114331] 0f ae f0
[ 0.116004] 0f ae f0
[ 0.118365] e8 07 21 62 ff

How is that useful?!

Please stop for a second with those senseless conversions and think
first. Try the change you're doing in kvm, take a look at what it
affects and think hard whether it makes any sense at all. Only if it
does, *then* send out the patch.

I'm willing to bet that *all* pr_debug* conversions below are wrong too.

Geez :(

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.