Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 5 Dec 2017 17:59:16 -0800 | Subject | Re: [PATCH V11 4/5] vsprintf: add printk specifier %px |
| |
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.
But yes, that "at %p" should definitely be %px.
In fact, it used to be a "%08lx" - and the value we print out is "unsigned long - but then when we unified the 32- and 64-bit architectures, using "%p" and a cast was a convenient way to unify the 32-bit %08lx and the 16-bit %016lx formats.
Will fix.
> a quick %p grep gives me the following list: ... > or is it OK to show hashes instead of pgd or pmd pointers?
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:
+ printk(KERN_ALERT "current->tss.cr3 = %08lx, %%cr3 = %08lx\n", + current->tss.cr3, user_esp); + user_esp = ((unsigned long *) user_esp)[address >> 22]; + printk(KERN_ALERT "*pde = %08lx\n", user_esp);
so it's more historical than sensible, I think.
Linus
| |