Re: [PATCH V11 4/5] vsprintf: add printk specifier %px

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (12/05/17 17:59), Linus Torvalds wrote:
[..]
> On Tue, Dec 5, 2017 at 5:36 PM, Sergey Senozhatsky
> <sergey.senozhatsky.work@gmail.com> wrote:
> > I see some %p-s being used in _supposedly_ important output,
> > like arch/x86/mm/fault.c
> >
> > show_fault_oops(struct pt_regs *regs, unsigned long error_code,
> >                 unsigned long address)
> > ...
> >         printk(KERN_CONT " at %p\n", (void *) address);
> >         printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip);
> 
> So %pS isn't %p, and shows the symbolic name.

sure, agreed. by "some %p-s being used" I meant the grep result,
not just x86 show_fault_oops().


> But yes, that "at %p" should definitely be %px.

more %p grepping [filtering out all `%ps %pf %pb' variants] gives
a huge number of print outs that potentially can be broken now

arch/x86/kernel/kprobes/core.c:		printk(KERN_WARNING "Unrecoverable kprobe detected at %p.\n",
arch/x86/kernel/kprobes/core.c:			       "current sp %p does not match saved sp %p\n",
arch/x86/kernel/kprobes/core.c:			printk(KERN_ERR "Saved registers for jprobe %p\n", jp);

arch/x86/kernel/head_32.S:	.asciz "Unknown interrupt or fault at: %p %p %p\n"
arch/x86/kernel/irq_32.c:	printk(KERN_DEBUG "CPU %u irqstacks, hard=%p soft=%p\n",

arch/x86/kernel/smpboot.c:	pr_debug("Stack at about %p\n", &cpuid);
arch/x86/kernel/traps.c:	printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",


so I'm not in position to suggest the removal of those print outs or to
decide if those are important at all, just saying that that "I'm confused
by pointer values and can't debug" might be more likely that we thought.


> So my gut feel is that those printouts should probably just be
> removed. They have some very old historical reasons: we've printed out
> the page directory pointers (and followed the page tables) since at
> least back in the 1.1.x days. This is from the 1.1.7 patch, back when
> mm/memory.c was all about x86:

I see, thanks.

	-ss