Skip to contentSkip to conversation

treewide: Add and use vsprintf extension %pSR

PATCH Applied Joe Perches 13 participants 62 messages Dec 12, 2012 – Apr 30, 2013 2 messages not in the archive

Subsystems (22)
lore
Share & tools
lore
#0 lore raw

cover treewide: Add and use vsprintf extension %pSR

Remove the somewhat awkward uses of print_symbol and convert all the existing uses to a new vsprintf pointer type of %pSR.

print_symbol can be interleaved when it is used in a sequence like:

	printk("something: ...");
	print_symbol("%s", addr);
	printk("\n");

Instead use:

	printk("something: %pSR\n", (void *)addr);

Add a new %p[SsFf]R vsprintf extension that can perform the same symbol function/address/offset formatting as print_symbol to reduce the number and styles of message logging functions.

print_symbol used __builtin_extract_return_addr for those architectures like S/390 and SPARC that have offset or masked addressing. %p[FfSs]R uses the same gcc __builtin

Joe Perches (26):
vsprintf: Add extension %pSR - print_symbol replacement
alpha: Convert print_symbol to %pSR
arm: Convert print_symbol to %pSR
arm64: Convert print_symbol to %pSR
avr32: Convert print_symbol to %pSR
c6x: Convert print_symbol to %pSR
ia64: Convert print_symbol to %pSR
m32r: Convert print_symbol to %pSR
mn10300: Convert print_symbol to %pSR
openrisc: Convert print_symbol to %pSR
powerpc: Convert print_symbol to %pSR
s390: Convert print_symbol to %pSR
sh: Convert print_symbol to %pSR
um: Convert print_symbol to %pSR
unicore32: Convert print_symbol to %pSR
x86: Convert print_symbol to %pSR
xtensa: Convert print_symbol to %pSR
drivers: base: Convert print_symbol to %pSR
gfs2: Convert print_symbol to %pSR
sysfs: Convert print_symbol to %pSR
irq: Convert print_symbol to %pSR
smp_processor_id: Convert print_symbol to %pSR
mm: Convert print_symbol to %pSR
xtensa: Convert print_symbol to %pSR
x86: head_64.S: Use vsprintf extension %pSR not print_symbol
kallsyms: Remove print_symbol

 Documentation/filesystems/sysfs.txt         |    4 +-
 Documentation/printk-formats.txt            |    2 +
 Documentation/zh_CN/filesystems/sysfs.txt   |    4 +-
 arch/alpha/kernel/traps.c                   |    8 ++----
 arch/arm/kernel/process.c                   |    4 +-
 arch/arm64/kernel/process.c                 |    4 +-
 arch/avr32/kernel/process.c                 |   25 ++++++-----------------
 arch/c6x/kernel/traps.c                     |    3 +-
 arch/ia64/kernel/process.c                  |   13 ++++-------
 arch/m32r/kernel/traps.c                    |    6 +---
 arch/mn10300/kernel/traps.c                 |    8 +++---
 arch/openrisc/kernel/traps.c                |    7 +----
 arch/powerpc/platforms/cell/spu_callbacks.c |   12 ++++------
 arch/s390/kernel/traps.c                    |   28 +++++++++++++++-----------
 arch/sh/kernel/process_32.c                 |    4 +-
 arch/um/kernel/sysrq.c                      |    6 +---
 arch/unicore32/kernel/process.c             |    5 ++-
 arch/x86/kernel/cpu/mcheck/mce.c            |   13 ++++++-----
 arch/x86/kernel/dumpstack.c                 |    5 +--
 arch/x86/kernel/head_64.S                   |    4 +-
 arch/x86/kernel/process_32.c                |    2 +-
 arch/x86/mm/mmio-mod.c                      |    4 +-
 arch/x86/um/sysrq_32.c                      |    9 ++-----
 arch/xtensa/kernel/traps.c                  |    6 +---
 drivers/base/core.c                         |    4 +-
 fs/gfs2/glock.c                             |    4 +-
 fs/gfs2/trans.c                             |    3 +-
 fs/sysfs/file.c                             |    4 +-
 include/linux/kallsyms.h                    |   18 -----------------
 kernel/irq/debug.h                          |   15 ++++++-------
 kernel/kallsyms.c                           |   11 ----------
 lib/smp_processor_id.c                      |    2 +-
 lib/vsprintf.c                              |   18 ++++++++++++----
 mm/memory.c                                 |    8 +++---
 mm/slab.c                                   |    8 ++----
 35 files changed, 117 insertions(+), 164 deletions(-)
Signature · 1 line
1.7.8.112.g3fd21
#1 lore raw

PATCH 1/26 vsprintf: Add extension %pSR - print_symbol replacement

print_symbol takes a long and converts it to a function name and offset. %pS does something similar, but doesn't translate the address via __builtin_extract_return_addr. %pSR does the translation.

This will enable replacing multiple calls like

	printk(...);
	printk_symbol(addr);
	printk("\n");
with a single non-interleavable in dmesg
	printk("... %pSR\n", (void *)addr);

Update documentation too.

---
 Documentation/printk-formats.txt |    2 ++
 lib/vsprintf.c                   |   18 +++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)
Documentation/printk-formats.txt +2 -0
index 8ffb274..69a5ca1 100644
@@ -17,6 +17,8 @@ Symbols/Function Pointers:
 	%pF	versatile_init+0x0/0x110
 	%pf	versatile_init
 	%pS	versatile_init+0x0/0x110
+	%pSR	versatile_init+0x9/0x110
+		(with __builtin_extract_return_addr() translation)
 	%ps	versatile_init
 	%pB	prev_fn_of_versatile_init+0x88/0x88
 
lib/vsprintf.c +13 -5
index fab33a9..832ff0b 100644
@@ -534,14 +534,21 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 
 static noinline_for_stack
 char *symbol_string(char *buf, char *end, void *ptr,
-		    struct printf_spec spec, char ext)
+		    struct printf_spec spec, const char *fmt)
 {
-	unsigned long value = (unsigned long) ptr;
+	unsigned long value;
 #ifdef CONFIG_KALLSYMS
 	char sym[KSYM_SYMBOL_LEN];
-	if (ext == 'B')
+#endif
+
+	if (fmt[1] == 'R')
+		ptr = __builtin_extract_return_addr(ptr);
+	value = (unsigned long)ptr;
+
+#ifdef CONFIG_KALLSYMS
+	if (*fmt == 'B')
 		sprint_backtrace(sym, value);
-	else if (ext != 'f' && ext != 's')
+	else if (*fmt != 'f' && *fmt != 's')
 		sprint_symbol(sym, value);
 	else
 		sprint_symbol_no_offset(sym, value);
@@ -987,6 +994,7 @@ int kptr_restrict __read_mostly;
  * - 'f' For simple symbolic function names without offset
  * - 'S' For symbolic direct pointers with offset
  * - 's' For symbolic direct pointers without offset
+ * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
  * - 'B' For backtraced symbolic direct pointers with offset
  * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
  * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
@@ -1059,7 +1067,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'S':
 	case 's':
 	case 'B':
-		return symbol_string(buf, end, ptr, spec, *fmt);
+		return symbol_string(buf, end, ptr, spec, fmt);
 	case 'R':
 	case 'r':
 		return resource_string(buf, end, ptr, spec, fmt);
Signature · 1 line
1.7.8.112.g3fd21
#2 lore raw

PATCH 2/26 alpha: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/alpha/kernel/traps.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
arch/alpha/kernel/traps.c +3 -5
index 272666d..9ac7318 100644
@@ -66,8 +66,8 @@ dik_show_regs(struct pt_regs *regs, unsigned long *r9_15)
 {
 	printk("pc = [<%016lx>]  ra = [<%016lx>]  ps = %04lx    %s\n",
 	       regs->pc, regs->r26, regs->ps, print_tainted());
-	print_symbol("pc is at %s\n", regs->pc);
-	print_symbol("ra is at %s\n", regs->r26 );
+	printk("pc is at %pSR\n", (void *)regs->pc);
+	printk("ra is at %pSR\n", (void *)regs->r26);
 	printk("v0 = %016lx  t0 = %016lx  t1 = %016lx\n",
 	       regs->r0, regs->r1, regs->r2);
 	printk("t2 = %016lx  t3 = %016lx  t4 = %016lx\n",
@@ -132,9 +132,7 @@ dik_show_trace(unsigned long *sp)
 			continue;
 		if (tmp >= (unsigned long) &_etext)
 			continue;
-		printk("[<%lx>]", tmp);
-		print_symbol(" %s", tmp);
-		printk("\n");
+		printk("[<%lx>] %pSR\n", tmp, (void *)tmp);
 		if (i > 40) {
 			printk(" ...");
 			break;
Signature · 1 line
1.7.8.112.g3fd21
#3 lore raw

PATCH 3/26 arm: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/arm/kernel/process.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/kernel/process.c +2 -2
index f79dd1e..de3966a 100644
@@ -283,8 +283,8 @@ void __show_regs(struct pt_regs *regs)
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", regs->ARM_lr);
+	printk("PC is at %pSR\n", (void *)instruction_pointer(regs));
+	printk("LR is at %pSR\n", (void *)regs->ARM_lr);
 	printk("pc : [<%08lx>]    lr : [<%08lx>]    psr: %08lx\n"
 	       "sp : %08lx  ip : %08lx  fp : %08lx\n",
 		regs->ARM_pc, regs->ARM_lr, regs->ARM_cpsr,
Signature · 1 line
1.7.8.112.g3fd21
#4 lore raw

PATCH 4/26 arm64: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/arm64/kernel/process.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/process.c +2 -2
index cb0956b..55fb84e 100644
@@ -187,8 +187,8 @@ void __show_regs(struct pt_regs *regs)
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", regs->regs[30]);
+	printk("PC is at %pSR\n", (void *)instruction_pointer(regs));
+	printk("LR is at %pSR\n", (void *)regs->regs[30]);
 	printk("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
 	       regs->pc, regs->regs[30], regs->pstate);
 	printk("sp : %016llx\n", regs->sp);
Signature · 1 line
1.7.8.112.g3fd21
#6 lore raw

PATCH 5/26 avr32: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

Remove a couple of #ifdef CONFIG_KALLSYMS branches that are now the same code.

---
 arch/avr32/kernel/process.c |   25 +++++++------------------
 1 files changed, 7 insertions(+), 18 deletions(-)
arch/avr32/kernel/process.c +7 -18
index fd78f58..7a56e86 100644
@@ -148,12 +148,7 @@ static void show_trace_log_lvl(struct task_struct *tsk, unsigned long *sp,
 		unsigned long new_fp;
 
 		lr = *(unsigned long *)fp;
-#ifdef CONFIG_KALLSYMS
-		printk("%s [<%08lx>] ", log_lvl, lr);
-#else
-		printk(" [<%08lx>] ", lr);
-#endif
-		print_symbol("%s\n", lr);
+		printk("%s [<%08lx>] %pSR\n", log_lvl, lr, (void *)lr);
 
 		new_fp = *(unsigned long *)(fp + 4);
 		if (new_fp <= fp)
@@ -172,14 +167,9 @@ static void show_trace_log_lvl(struct task_struct *tsk, unsigned long *sp,
 
 	while (!kstack_end(sp)) {
 		addr = *sp++;
-		if (kernel_text_address(addr)) {
-#ifdef CONFIG_KALLSYMS
-			printk("%s [<%08lx>] ", log_lvl, addr);
-#else
-			printk(" [<%08lx>] ", addr);
-#endif
-			print_symbol("%s\n", addr);
-		}
+		if (kernel_text_address(addr))
+			printk("%s [<%08lx>] %pSR\n",
+			       log_lvl, addr, (void *)addr);
 	}
 	printk("\n");
 }
@@ -235,10 +225,9 @@ void show_regs_log_lvl(struct pt_regs *regs, const char *log_lvl)
 	if (!user_mode(regs)) {
 		sp = (unsigned long)regs + FRAME_SIZE_FULL;
 
-		printk("%s", log_lvl);
-		print_symbol("PC is at %s\n", instruction_pointer(regs));
-		printk("%s", log_lvl);
-		print_symbol("LR is at %s\n", lr);
+		printk("%sPC is at %pSR\n",
+		       log_lvl, (void *)instruction_pointer(regs));
+		printk("%sLR is at %pSR\n", log_lvl, (void *)lr);
 	}
 
 	printk("%spc : [<%08lx>]    lr : [<%08lx>]    %s\n"
Signature · 1 line
1.7.8.112.g3fd21
#7 lore raw

PATCH 6/26 c6x: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/c6x/kernel/traps.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
arch/c6x/kernel/traps.c +1 -2
index 1be74e5..349cae0 100644
@@ -382,8 +382,7 @@ static void show_trace(unsigned long *stack, unsigned long *endstack)
 			if (i % 5 == 0)
 				pr_debug("\n	    ");
 #endif
-			pr_debug(" [<%08lx>]", addr);
-			print_symbol(" %s\n", addr);
+			pr_debug(" [<%08lx>] %pSR\n", addr, (void *)addr);
 			i++;
 		}
 	}
Signature · 1 line
1.7.8.112.g3fd21
#8 lore raw

PATCH 7/26 ia64: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/ia64/kernel/process.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
arch/ia64/kernel/process.c +5 -8
index 31360cb..f11105a 100644
@@ -1,4 +1,4 @@
-/*
+ /*
  * Architecture-specific setup.
  *
  * Copyright (C) 1998-2003 Hewlett-Packard Co
@@ -66,7 +66,6 @@ void
 ia64_do_show_stack (struct unw_frame_info *info, void *arg)
 {
 	unsigned long ip, sp, bsp;
-	char buf[128];			/* don't make it so big that it overflows the stack! */
 
 	printk("\nCall Trace:\n");
 	do {
@@ -76,11 +75,9 @@ ia64_do_show_stack (struct unw_frame_info *info, void *arg)
 
 		unw_get_sp(info, &sp);
 		unw_get_bsp(info, &bsp);
-		snprintf(buf, sizeof(buf),
-			 " [<%016lx>] %%s\n"
-			 "                                sp=%016lx bsp=%016lx\n",
-			 ip, sp, bsp);
-		print_symbol(buf, ip);
+		printk(" [<%016lx>] %pSR\n"
+		       "                                sp=%016lx bsp=%016lx\n",
+		       ip, (void *)ip, sp, bsp);
 	} while (unw_unwind(info) >= 0);
 }
 
@@ -116,7 +113,7 @@ show_regs (struct pt_regs *regs)
 	printk("psr : %016lx ifs : %016lx ip  : [<%016lx>]    %s (%s)\n",
 	       regs->cr_ipsr, regs->cr_ifs, ip, print_tainted(),
 	       init_utsname()->release);
-	print_symbol("ip is at %s\n", ip);
+	printk("ip is at %pSR\n", (void *)ip);
 	printk("unat: %016lx pfs : %016lx rsc : %016lx\n",
 	       regs->ar_unat, regs->ar_pfs, regs->ar_rsc);
 	printk("rnat: %016lx bsps: %016lx pr  : %016lx\n",
Signature · 1 line
1.7.8.112.g3fd21
#9 lore raw

PATCH 8/26 m32r: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/m32r/kernel/traps.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/m32r/kernel/traps.c +2 -4
index 3bcb207..9fe3467 100644
@@ -132,10 +132,8 @@ static void show_trace(struct task_struct *task, unsigned long *stack)
 	printk("Call Trace: ");
 	while (!kstack_end(stack)) {
 		addr = *stack++;
-		if (__kernel_text_address(addr)) {
-			printk("[<%08lx>] ", addr);
-			print_symbol("%s\n", addr);
-		}
+		if (__kernel_text_address(addr))
+			printk("[<%08lx>] %pSR\n", addr, (void *)addr);
 	}
 	printk("\n");
 }
Signature · 1 line
1.7.8.112.g3fd21
Acked-by on v1 8/26
#10 lore raw

Re: [TRIVIAL PATCH 08/26] m32r: Convert print_symbol to %pSR

Thank you.

From: Joe Perches <[email protected]>
Subject: [TRIVIAL PATCH 08/26] m32r: Convert print_symbol to %pSR
Date: Wed, 12 Dec 2012 10:18:57 -0800

Quoted 28 lines

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/m32r/kernel/traps.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/m32r/kernel/traps.c +2 -4
index 3bcb207..9fe3467 100644
@@ -132,10 +132,8 @@ static void show_trace(struct task_struct *task, unsigned long *stack)
 	printk("Call Trace: ");
 	while (!kstack_end(stack)) {
 		addr = *stack++;
-		if (__kernel_text_address(addr)) {
-			printk("[<%08lx>] ", addr);
-			print_symbol("%s\n", addr);
-		}
+		if (__kernel_text_address(addr))
+			printk("[<%08lx>] %pSR\n", addr, (void *)addr);
 	}
 	printk("\n");
 }
Signature · 1 line
1.7.8.112.g3fd21
#12 lore raw

Re: [TRIVIAL PATCH 08/26] m32r: Convert print_symbol to %pSR

Quoted 2 lines · Jiri Kosina
Quoted 1 line · Hirokazu Takata

Hi Jiri.

It's really not useful to cherry-pick a few patches from this series.

You must first apply patch 1 of 26 before applying any of the others as a new vsprintf pointer extension %pSR is added by that patch.

#13 lore raw

Re: [TRIVIAL PATCH 08/26] m32r: Convert print_symbol to %pSR

Quoted 11 lines · Joe Perches
Quoted 2 lines · Hirokazu Takata
Quoted 1 line

Hi Jiri.

It's really not useful to cherry-pick a few patches from this series.

You must first apply patch 1 of 26 before applying any of the others as a new vsprintf pointer extension %pSR is added by that patch.

Omitting the actual addition of %pSR is of course completely my fault, caused by my incorrect merge into for-linus branch, thanks for noticing that. It's now fixed in Linus' tree.

OTOH, cherry-picking the patches that have been Acked by maintainers is what I usually do in the first round. For 3.11, I'd like to ask you to send me a remaining conversions as one condensed patch, and I'll decide whether I'll be taking those or not despite the missing Ack from maintainers.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
#14 lore raw

PATCH 9/26 mn10300: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

Consolidate multiple printks into a one.

---
 arch/mn10300/kernel/traps.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
arch/mn10300/kernel/traps.c +4 -4
index b900e5a..d263b70 100644
@@ -256,13 +256,13 @@ void show_trace(unsigned long *sp)
 		}
 
 		if (__kernel_text_address(addr)) {
-			printk(" [<%08lx>]", addr);
+			const char *known = "";
 			if (stack >= raslot)
 				raslot = ULONG_MAX;
 			else
-				printk(" ?");
-			print_symbol(" %s", addr);
-			printk("\n");
+				known = " ?";
+			printk(" [<%08lx>]%s %pSR\n",
+			       addr, known, (void *)addr);
 		}
 	}
 
Signature · 1 line
1.7.8.112.g3fd21
#15 lore raw

PATCH 10/26 openrisc: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/openrisc/kernel/traps.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)
arch/openrisc/kernel/traps.c +2 -5
index 5cce396..f5c1d0a 100644
@@ -56,11 +56,8 @@ void show_trace(struct task_struct *task, unsigned long *stack)
 
 	while (valid_stack_ptr(context, stack)) {
 		addr = *stack++;
-		if (__kernel_text_address(addr)) {
-			printk(" [<%08lx>]", addr);
-			print_symbol(" %s", addr);
-			printk("\n");
-		}
+		if (__kernel_text_address(addr))
+			printk(" [<%08lx>] %pSR\n", addr, (void *)addr);
 	}
 	printk(" =======================\n");
 }
Signature · 1 line
1.7.8.112.g3fd21
#16 lore raw

PATCH 11/26 powerpc: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

Convert the #ifdef DEBUG block to a single pr_debug.

---
 arch/powerpc/platforms/cell/spu_callbacks.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)
arch/powerpc/platforms/cell/spu_callbacks.c +5 -7
index 75d6133..c5fe6d2 100644
@@ -60,13 +60,11 @@ long spu_sys_callback(struct spu_syscall_block *s)
 
 	syscall = spu_syscall_table[s->nr_ret];
 
-#ifdef DEBUG
-	print_symbol(KERN_DEBUG "SPU-syscall %s:", (unsigned long)syscall);
-	printk("syscall%ld(%lx, %lx, %lx, %lx, %lx, %lx)\n",
-			s->nr_ret,
-			s->parm[0], s->parm[1], s->parm[2],
-			s->parm[3], s->parm[4], s->parm[5]);
-#endif
+	pr_debug("SPU-syscall %pSR:syscall%ld(%lx, %lx, %lx, %lx, %lx, %lx)\n",
+		 syscall,
+		 s->nr_ret,
+		 s->parm[0], s->parm[1], s->parm[2],
+		 s->parm[3], s->parm[4], s->parm[5]);
 
 	return syscall(s->parm[0], s->parm[1], s->parm[2],
 		       s->parm[3], s->parm[4], s->parm[5]);
Signature · 1 line
1.7.8.112.g3fd21
#18 lore raw

Re: [TRIVIAL PATCH 11/26] powerpc: Convert print_symbol to %pSR

Quoted 9 lines · Arnd Bergmann
Quoted 6 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

Convert the #ifdef DEBUG block to a single pr_debug.

nice cleanup!

... which also breaks the build :-(

Quoted 1 line

I'll fix it up locally.

Ben.

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#19 lore raw

Re: [TRIVIAL PATCH 11/26] powerpc: Convert print_symbol to %pSR

Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Arnd Bergmann
Quoted 6 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

Convert the #ifdef DEBUG block to a single pr_debug.

Quoted 7 lines
Quoted 1 line

nice cleanup!

... which also breaks the build :-(

Quoted 1 line

I'll fix it up locally.

OK, I didn't compile it.
How does it break the build?

#20 lore raw

PATCH 12/26 s390: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/s390/kernel/traps.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)
arch/s390/kernel/traps.c +16 -12
index 70ecfc5..f718e12 100644
@@ -91,8 +91,9 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 		if (sp < low || sp > high - sizeof(*sf))
 			return sp;
 		sf = (struct stack_frame *) sp;
-		printk("([<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-		print_symbol("%s)\n", sf->gprs[8] & PSW_ADDR_INSN);
+		printk("([<%016lx>] %pSR\n",
+		       sf->gprs[8] & PSW_ADDR_INSN,
+		       (void *)(sf->gprs[8] & PSW_ADDR_INSN));
 		/* Follow the backchain. */
 		while (1) {
 			low = sp;
@@ -102,16 +103,18 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 			if (sp <= low || sp > high - sizeof(*sf))
 				return sp;
 			sf = (struct stack_frame *) sp;
-			printk(" [<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-			print_symbol("%s\n", sf->gprs[8] & PSW_ADDR_INSN);
+			printk(" [<%016lx>] %pSR\n",
+			       sf->gprs[8] & PSW_ADDR_INSN,
+			       (void *)(sf->gprs[8] & PSW_ADDR_INSN));
 		}
 		/* Zero backchain detected, check for interrupt frame. */
 		sp = (unsigned long) (sf + 1);
 		if (sp <= low || sp > high - sizeof(*regs))
 			return sp;
 		regs = (struct pt_regs *) sp;
-		printk(" [<%016lx>] ", regs->psw.addr & PSW_ADDR_INSN);
-		print_symbol("%s\n", regs->psw.addr & PSW_ADDR_INSN);
+		printk(" [<%016lx>] %pSR\n",
+		       regs->psw.addr & PSW_ADDR_INSN,
+		       (void *)(regs->psw.addr & PSW_ADDR_INSN));
 		low = sp;
 		sp = regs->gprs[15];
 	}
@@ -169,8 +172,9 @@ static void show_last_breaking_event(struct pt_regs *regs)
 {
 #ifdef CONFIG_64BIT
 	printk("Last Breaking-Event-Address:\n");
-	printk(" [<%016lx>] ", regs->args[0] & PSW_ADDR_INSN);
-	print_symbol("%s\n", regs->args[0] & PSW_ADDR_INSN);
+	printk(" [<%016lx>] %pSR\n",
+	       regs->args[0] & PSW_ADDR_INSN,
+	       (void *)(regs->args[0] & PSW_ADDR_INSN));
 #endif
 }
 
@@ -201,10 +205,10 @@ void show_registers(struct pt_regs *regs)
 	char *mode;
 
 	mode = user_mode(regs) ? "User" : "Krnl";
-	printk("%s PSW : %p %p",
-	       mode, (void *) regs->psw.mask,
-	       (void *) regs->psw.addr);
-	print_symbol(" (%s)\n", regs->psw.addr & PSW_ADDR_INSN);
+	printk("%s PSW : %p %p, %pSR\n",
+	       mode, (void *)regs->psw.mask,
+	       (void *)regs->psw.addr,
+	       (void *)(regs->psw.addr & PSW_ADDR_INSN));
 	printk("           R:%x T:%x IO:%x EX:%x Key:%x M:%x W:%x "
 	       "P:%x AS:%x CC:%x PM:%x", mask_bits(regs, PSW_MASK_PER),
 	       mask_bits(regs, PSW_MASK_DAT), mask_bits(regs, PSW_MASK_IO),
Signature · 1 line
1.7.8.112.g3fd21
#21 lore raw

Re: [TRIVIAL PATCH 12/26] s390: Convert print_symbol to %pSR

Quoted 2 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

[]

arch/s390/kernel/traps.c

Quoted 1 line
arch/s390/kernel/traps.c +0 -0

[]

arch/s390/kernel/traps.c __show_trace(unsigned long sp, unsigned long low, unsigned long high) · line 94

Quoted 7 lines
arch/s390/kernel/traps.c +1 -2
@@ -91,8 +91,9 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 		if (sp < low || sp > high - sizeof(*sf))
 			return sp;
 		sf = (struct stack_frame *) sp;
-		printk("([<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-		print_symbol("%s)\n", sf->gprs[8] & PSW_ADDR_INSN);
+		printk("([<%016lx>] %pSR\n",

Sorry, this is defective, it needs a trailing ')' like:

+ printk("([<%016lx>] %pSR)\n",

arch/s390/kernel/traps.c __show_trace(unsigned long sp, unsigned long low, unsigned long high) · line 106

Quoted 7 lines
arch/s390/kernel/traps.c +1 -2
@@ -102,16 +103,18 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 			if (sp <= low || sp > high - sizeof(*sf))
 				return sp;
 			sf = (struct stack_frame *) sp;
-			printk(" [<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-			print_symbol("%s\n", sf->gprs[8] & PSW_ADDR_INSN);
+			printk(" [<%016lx>] %pSR\n",

here too

I'll resubmit.

#22 lore raw

PATCH 13/26 sh: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/sh/kernel/process_32.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/sh/kernel/process_32.c +2 -2
index 73eb66f..a0837ba 100644
@@ -38,8 +38,8 @@ void show_regs(struct pt_regs * regs)
 	       (int)strcspn(init_utsname()->version, " "),
 	       init_utsname()->version);
 
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("PR is at %s\n", regs->pr);
+	printk("PC is at %pSR\n", (void *)instruction_pointer(regs));
+	printk("PR is at %pSR\n", (void *)regs->pr);
 
 	printk("PC  : %08lx SP  : %08lx SR  : %08lx ",
 	       regs->pc, regs->regs[15], regs->sr);
Signature · 1 line
1.7.8.112.g3fd21
#23 lore raw

PATCH 14/26 um: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/um/kernel/sysrq.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/um/kernel/sysrq.c +2 -4
index e562ff8..6fa632c 100644
@@ -24,10 +24,8 @@ void show_trace(struct task_struct *task, unsigned long * stack)
 	while (((long) stack & (THREAD_SIZE-1)) != 0) {
 		addr = *stack;
 		if (__kernel_text_address(addr)) {
-			printk(KERN_INFO "%08lx:  [<%08lx>]",
-			       (unsigned long) stack, addr);
-			print_symbol(KERN_CONT " %s", addr);
-			printk(KERN_CONT "\n");
+			printk(KERN_INFO "%08lx:  [<%08lx>] %pSR\n",
+			       (unsigned long)stack, addr, (void *)addr);
 		}
 		stack++;
 	}
Signature · 1 line
1.7.8.112.g3fd21
#24 lore raw

PATCH 15/26 unicore32: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/unicore32/kernel/process.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/unicore32/kernel/process.c +3 -2
index 62bad9f..d17a893 100644
@@ -169,8 +169,9 @@ void __show_regs(struct pt_regs *regs)
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", regs->UCreg_lr);
+	printk(KERN_DEFAULT "PC is at %pSR\n",
+	       (void *)instruction_pointer(regs));
+	printk(KERN_DEFAULT "LR is at %pSR\n", (void *)regs->UCreg_lr);
 	printk(KERN_DEFAULT "pc : [<%08lx>]    lr : [<%08lx>]    psr: %08lx\n"
 	       "sp : %08lx  ip : %08lx  fp : %08lx\n",
 		regs->UCreg_pc, regs->UCreg_lr, regs->UCreg_asr,
Signature · 1 line
1.7.8.112.g3fd21

A message that is not in the archive (sent off-list or not yet received): [email protected]

#26 lore raw

Re: 答复: [TRIVIAL PATCH 15/26] unicore32: Convert print_symbol to %pSR

Quoted 1 line · Guan Xuetao

确认下面的patch可以正常编译,无warning信息

I'm afraid I can't read this.
Anyone have an English translation?

#27 lore raw

Re: 答复: [TRIVIAL PATCH 15/26] unicore32: Convert print_symbol to %pSR

Quoted 2 lines · Joe Perches
Quoted 1 line · Guan Xuetao

确认下面的patch可以正常编译,无warning信息

please be sure that the patch below can pass compiling, without warnings

Quoted 4 lines

I'm afraid I can't read this.
Anyone have an English translation?

I can try (although I do not known the details of this patch)

:-)

Quoted 7 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 3 lines
Chen Gang

Asianux Corporation
#28 lore raw

Re: 答复: [TRIVIAL PATCH 15/26] unicore32: Convert print_symbol to %pSR

In reply to a message not in the archive
Quoted 1 line · Guan Xuetao

确认下面的patch可以正常编译,无warning信息

There is no unicore32 cross compiler available
on kernel.org.

ftp://ftp.kernel.org/pub/tools/crosstool/index.html

These are unsigned long cast to (void *)
What is the warning and how does it differ
from any existing warning?

Quoted 1 line

Review

Quoted 1 line
(quoted diff) +0 -0

[]

Quoted 13 lines

arch/unicore32/kernel/process.c void __show_regs(struct pt_regs *regs) · line 175

Quoted 10 lines
arch/unicore32/kernel/process.c +3 -2
@@ -169,8 +169,9 @@ void __show_regs(struct pt_regs *regs)
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", regs->UCreg_lr);
+	printk(KERN_DEFAULT "PC is at %pSR\n",
+	       (void *)instruction_pointer(regs));
+	printk(KERN_DEFAULT "LR is at %pSR\n", (void *)regs->UCreg_lr);
 	printk(KERN_DEFAULT "pc : [<%08lx>]    lr : [<%08lx>]    psr:

%08lx\n"

Quoted 2 lines
arch/unicore32/kernel/process.c +0 -0
 	       "sp : %08lx  ip : %08lx  fp : %08lx\n",
 		regs->UCreg_pc, regs->UCreg_lr, regs->UCreg_asr,

A message that is not in the archive (sent off-list or not yet received): 19e6ea369e4455fc447c48f72bd433921bd0a703.1355335228.git.joe@perches.c om

Acked-by Guan Xuetao
#30 lore raw

Re: [TRIVIAL PATCH 15/26] unicore32: Convert print_symbol to %pSR

Quoted 4 lines

Use the new vsprintf extension to avoid any possible message interleaving.

Sorry for my last email, which should not reply all.

Quoted 24 lines
---
 arch/unicore32/kernel/process.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
arch/unicore32/kernel/process.c +3 -3
index 62bad9f..d17a893 100644
@@ -169,8 +169,9 @@ void __show_regs(struct pt_regs *regs)
 		init_utsname()->release,
 		(int)strcspn(init_utsname()->version, " "),
 		init_utsname()->version);
-	print_symbol("PC is at %s\n", instruction_pointer(regs));
-	print_symbol("LR is at %s\n", regs->UCreg_lr);
+	printk(KERN_DEFAULT "PC is at %pSR\n",
+	       (void *)instruction_pointer(regs));
+	printk(KERN_DEFAULT "LR is at %pSR\n", (void *)regs->UCreg_lr);
 	printk(KERN_DEFAULT "pc : [<%08lx>]    lr : [<%08lx>]    psr: %08lx\n"
 	       "sp : %08lx  ip : %08lx  fp : %08lx\n",
 		regs->UCreg_pc, regs->UCreg_lr, regs->UCreg_asr,
--

1.7.8.112.g3fd21

#31 lore raw

PATCH 16/26 x86: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/x86/kernel/cpu/mcheck/mce.c |   13 +++++++------
 arch/x86/kernel/dumpstack.c      |    5 ++---
 arch/x86/kernel/process_32.c     |    2 +-
 arch/x86/mm/mmio-mod.c           |    4 ++--
 arch/x86/um/sysrq_32.c           |    9 +++------
 5 files changed, 15 insertions(+), 18 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +7 -6
index 80dbda8..996f98a 100644
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
 	       m->extcpu, m->mcgstatus, m->bank, m->status);
 
 	if (m->ip) {
-		pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
-			!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
-				m->cs, m->ip);
-
 		if (m->cs == __KERNEL_CS)
-			print_symbol("{%s}", m->ip);
-		pr_cont("\n");
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip, (void *)m->ip);
+		else
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip);
 	}
 
 	pr_emerg(HW_ERR "TSC %llx ", m->tsc);
arch/x86/kernel/dumpstack.c +2 -3
index ae42418b..b6e5bdb 100644
@@ -281,9 +281,8 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
 		sp = kernel_stack_pointer(regs);
 		savesegment(ss, ss);
 	}
-	printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip);
-	print_symbol("%s", regs->ip);
-	printk(" SS:ESP %04x:%08lx\n", ss, sp);
+	printk(KERN_EMERG "EIP: [<%08lx>] %pSR SS:ESP %04x:%08lx\n",
+	       regs->ip, (void *)regs->ip, ss, sp);
 #else
 	/* Executive summary in case the oops scrolled away */
 	printk(KERN_ALERT "RIP ");
arch/x86/kernel/process_32.c +1 -1
index b5a8905..0db77e0 100644
@@ -89,7 +89,7 @@ void __show_regs(struct pt_regs *regs, int all)
 	printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n",
 			(u16)regs->cs, regs->ip, regs->flags,
 			smp_processor_id());
-	print_symbol("EIP is at %s\n", regs->ip);
+	printk(KERN_DEFAULT "EIP is at %pSR\n", (void *)regs->ip);
 
 	printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n",
 		regs->ax, regs->bx, regs->cx, regs->dx);
arch/x86/mm/mmio-mod.c +2 -2
index dc0b727..c0ca484 100644
@@ -123,8 +123,8 @@ static void die_kmmio_nesting_error(struct pt_regs *regs, unsigned long addr)
 	pr_emerg("unexpected fault for address: 0x%08lx, last fault for address: 0x%08lx\n",
 		 addr, my_reason->addr);
 	print_pte(addr);
-	print_symbol(KERN_EMERG "faulting IP is at %s\n", regs->ip);
-	print_symbol(KERN_EMERG "last faulting IP was at %s\n", my_reason->ip);
+	pr_emerg("faulting IP is at %pSR\n", (void *)regs->ip);
+	pr_emerg("last faulting IP was at %pSR\n", (void *)my_reason->ip);
 #ifdef __i386__
 	pr_emerg("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		 regs->ax, regs->bx, regs->cx, regs->dx);
arch/x86/um/sysrq_32.c +3 -6
index c9bee5b..30b4200 100644
@@ -50,18 +50,15 @@ static inline unsigned long print_context_stack(struct thread_info *tinfo,
 #ifdef CONFIG_FRAME_POINTER
 	while (valid_stack_ptr(tinfo, (void *)ebp)) {
 		addr = *(unsigned long *)(ebp + 4);
-		printk("%08lx:  [<%08lx>]", ebp + 4, addr);
-		print_symbol(" %s", addr);
-		printk("\n");
+		printk("%08lx:  [<%08lx>] %pSR\n", ebp + 4, addr, (void *)addr);
 		ebp = *(unsigned long *)ebp;
 	}
 #else
 	while (valid_stack_ptr(tinfo, stack)) {
 		addr = *stack;
 		if (__kernel_text_address(addr)) {
-			printk("%08lx:  [<%08lx>]", (unsigned long) stack, addr);
-			print_symbol(" %s", addr);
-			printk("\n");
+			printk("%08lx:  [<%08lx>] %pSR\n",
+			       (unsigned long)stack, addr, (void *)addr);
 		}
 		stack++;
 	}
Signature · 1 line
1.7.8.112.g3fd21
maintainer
#32 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 25 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/x86/kernel/cpu/mcheck/mce.c |   13 +++++++------
 arch/x86/kernel/dumpstack.c      |    5 ++---
 arch/x86/kernel/process_32.c     |    2 +-
 arch/x86/mm/mmio-mod.c           |    4 ++--
 arch/x86/um/sysrq_32.c           |    9 +++------
 5 files changed, 15 insertions(+), 18 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +0 -4
index 80dbda8..996f98a 100644
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
 	       m->extcpu, m->mcgstatus, m->bank, m->status);
 
 	if (m->ip) {
-		pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
-			!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
-				m->cs, m->ip);
-
 		if (m->cs == __KERNEL_CS)

arch/x86/kernel/cpu/mcheck/mce.c static void print_mce(struct mce *m) · line 253

Quoted 10 lines
arch/x86/kernel/cpu/mcheck/mce.c +7 -2
-			print_symbol("{%s}", m->ip);
-		pr_cont("\n");
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip, (void *)m->ip);
+		else
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip);
 	}

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

Or, Tony, what do you think, could we get away if we printed empty string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any userspace which is parsing that output and seeing "... {}" in that case.

This assumes that vsprintf can print (void *)0 when passed as an argument through %pSR. Joe?

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#33 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 3 lines · Borislav Petkov
Quoted 2 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

[]

Quoted 1 line

arch/x86/kernel/cpu/mcheck/mce.c

Quoted 1 line
arch/x86/kernel/cpu/mcheck/mce.c +0 -0

[]

Quoted 14 lines
Quoted 14 lines
arch/x86/kernel/cpu/mcheck/mce.c +3 -6
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
 	       m->extcpu, m->mcgstatus, m->bank, m->status);
 
 	if (m->ip) {
-		pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
-			!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
-				m->cs, m->ip);
-
 		if (m->cs == __KERNEL_CS)
-			print_symbol("{%s}", m->ip);
-		pr_cont("\n");
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip, (void *)m->ip);
Quoted 8 lines

arch/x86/kernel/cpu/mcheck/mce.c static void print_mce(struct mce *m) · line 253

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +4 -0
+		else
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip);
 	}

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

It was done to avoid interleaving.

Quoted 6 lines

Or, Tony, what do you think, could we get away if we printed empty string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any userspace which is parsing that output and seeing "... {}" in that case.

This assumes that vsprintf can print (void *)0 when passed as an argument through %pSR. Joe?

Definitely yes when not #defined CONFIG_KALLSYMS

I believe no object exists at address 0 for all arches so I believe yes for CONFIG_KALLSYMS too,

My preference is to eventually do away with all the

	"[%0(size)lx] %pSR", addr, (void *)addr

uses and create another %pSx that emits the pointer address and the function/offset in one go in a standardized style without caring about the pointer size.

Something like:

	"%pSp", (void *)addr

would emit

	[<7def0123>] function_name+offset/size
maintainer
#34 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 4 lines · Joe Perches
Quoted 2 lines

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

It was done to avoid interleaving.

Right.

Quoted 11 lines
Quoted 6 lines

Or, Tony, what do you think, could we get away if we printed empty string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any userspace which is parsing that output and seeing "... {}" in that case.

This assumes that vsprintf can print (void *)0 when passed as an argument through %pSR. Joe?

Definitely yes when not #defined CONFIG_KALLSYMS

I believe no object exists at address 0 for all arches so I believe yes for CONFIG_KALLSYMS too,

Ok, here's what I'm thinking more specifically. Have single call like this:

	pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
			!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
			m->cs, m->ip,
			(m->cs == __KERNEL_CS) ? (void *)m->ip : (void *)0);

and when vsprintf gets to it, it recognizes the special case of (void *)0 and dumps the empty string "" for that argument.

Hmm.

Quoted 7 lines

My preference is to eventually do away with all the

	"[%0(size)lx] %pSR", addr, (void *)addr

uses and create another %pSx that emits the pointer address and the function/offset in one go in a standardized style without caring about the pointer size.

Quoted 7 lines

Something like:

	"%pSp", (void *)addr

would emit

	[<7def0123>] function_name+offset/size

Right, this is another way of looking it. And in order to make it more robust, it should be able to handle the (void *)0 case so that callers don't have to check the arg.

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#35 lore raw

RE: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

In reply to Joe Perches
Quoted 2 lines

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

As written the patch has the nice property of not making any changes to the console output (except to eliminate the possibility of interleaved output that the original code had). Whereas making the change you suggest would end up with a useless "{}" in the usermode case (or worse if %pSR did something with m->ip .. which is most probably not NULL ... it should be a valid user mode instruction pointer when m->cs != KERNEL_CS).

So Acked-by: Tony Luck <[email protected]>

#36 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 6 lines · Tony
Quoted 2 lines

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

As written the patch has the nice property of not making any changes to the console output (except to eliminate the possibility of interleaved output that the original code had).

Well, it does eliminate a trailing space when m->cs != KERNEL_CS. That probably won't hurt anything, but...

maintainer Acked-by
#37 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 10 lines · Joe Perches
Quoted 6 lines · Tony
Quoted 2 lines

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

As written the patch has the nice property of not making any changes to the console output (except to eliminate the possibility of interleaved output that the original code had).

Well, it does eliminate a trailing space when m->cs != KERNEL_CS. That probably won't hurt anything, but...

Yeah, that won't be an issue. So let's take it as is, as Tony suggests.

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#38 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 5 lines · Borislav Petkov
Quoted 4 lines · Joe Perches
Quoted 3 lines · Tony
Quoted 2 lines

I think I'd go ahead and ACK this unless Tony has some comments. I'm not happy about the two pr_emerg calls based on the conditional.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

As written the patch has the nice property of not making any changes to the console output (except to eliminate the possibility of interleaved output that the original code had).

Well, it does eliminate a trailing space when m->cs != KERNEL_CS. That probably won't hurt anything, but...

Yeah, that won't be an issue. So let's take it as is, as Tony suggests.

Hi again Boris.

I should mention one more thing.

m->ip is a u64 so, when compiling x86-32, there's a new warning "cast to pointer from integer of different size". This isn't new different behavior, just a new warning. The previous print_symbol took a ulong and the u64 was silently truncated.

CC arch/x86/kernel/cpu/mcheck/mce.o
arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’:
arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

If appropriate, the code could be changed to

	(void *)(unsigned long)m->ip

to shut the compiler up.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

maintainer
#39 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 5 lines · Joe Perches

m->ip is a u64 so, when compiling x86-32, there's a new warning "cast to pointer from integer of different size". This isn't new different behavior, just a new warning. The previous print_symbol took a ulong and the u64 was silently truncated.

Quoted 7 lines

CC arch/x86/kernel/cpu/mcheck/mce.o
arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’:
arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

If appropriate, the code could be changed to

	(void *)(unsigned long)m->ip

Can we explicitly cast it to what it is so that we can be explicit as to what we're casting it? IOW:

	(void *)(__u64)m->ip;

Does that even work on 32bit?

Also, does the compiler bitch about this useless cast when building with W=123?

Thanks.

Signature · 10 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
#40 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 8 lines · Borislav Petkov
Quoted 4 lines

If appropriate, the code could be changed to

	(void *)(unsigned long)m->ip

Can we explicitly cast it to what it is so that we can be explicit as to what we're casting it? IOW:

Quoted 7 lines
	(void *)(__u64)m->ip;

Does that even work on 32bit?

Also, does the compiler bitch about this useless cast when building with W=123?

Uh... no.

The point is that (void *)(unsigned long) casts it to an integer of pointer size -- in userspace you would to (void *)(size_t) or (void *)(uintptr_t) -- so that the compiler knows "I meant to do that."

-hpa

maintainer
#41 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 17 lines · H. Peter Anvin
Quoted 8 lines · Borislav Petkov
Quoted 4 lines

If appropriate, the code could be changed to

	(void *)(unsigned long)m->ip

Can we explicitly cast it to what it is so that we can be explicit as to what we're casting it? IOW:

Quoted 7 lines
	(void *)(__u64)m->ip;

Does that even work on 32bit?

Also, does the compiler bitch about this useless cast when building with W=123?

Quoted 5 lines

Uh... no.

The point is that (void *)(unsigned long) casts it to an integer of pointer size -- in userspace you would to (void *)(size_t) or (void *)(uintptr_t) -- so that the compiler knows "I meant to do that."

Ok, I grok it now. So, in most cases, mce.ip comes from pt_regs.ip which is unsigned long so we're fine for both 32- and 64-bit. There's only the case with precise RIP reporting where we get the rip from an MSR but even then, it should be 4 bytes on 32-bit since our addresses there are 4 bytes.

So, actually struct mce should've had ->ip defined as an unsigned long from the very beginning. But we can't change that now because this struct is shared with userspace :(

So yes Joe, make it (void *)(unsigned long).

Thanks guys.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#42 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

Quoted 11 lines · Borislav Petkov

Ok, I grok it now. So, in most cases, mce.ip comes from pt_regs.ip which is unsigned long so we're fine for both 32- and 64-bit. There's only the case with precise RIP reporting where we get the rip from an MSR but even then, it should be 4 bytes on 32-bit since our addresses there are 4 bytes.

So, actually struct mce should've had ->ip defined as an unsigned long from the very beginning. But we can't change that now because this struct is shared with userspace :(

Good thing, then,. otherwise we'd have a compat headache.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#43 lore raw

Re: [TRIVIAL PATCH 16/26] x86: Convert print_symbol to %pSR

In reply to Borislav Petkov
Quoted 12 lines · Borislav Petkov
Quoted 11 lines · Joe Perches

m->ip is a u64 so, when compiling x86-32, there's a new warning "cast to pointer from integer of different size". This isn't new different behavior, just a new warning. The previous print_symbol took a ulong and the u64 was silently truncated.

CC arch/x86/kernel/cpu/mcheck/mce.o
arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’:
arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

If appropriate, the code could be changed to

Quoted 8 lines
Quoted 1 line
	(void *)(unsigned long)m->ip

Can we explicitly cast it to what it is so that we can be explicit as to what we're casting it? IOW:

	(void *)(__u64)m->ip;

Does that even work on 32bit?

No, it already is __u64. You need (unsigned long) or something else the same size as (void *)

from: arch/x86/include/asm/mce.h

/* Fields are zero when not available */
struct mce {
...

	__u64 ip;
Quoted 2 lines

Also, does the compiler bitch about this useless cast when building with W=123?

For x86-32:

With the cast the line is silent.
Without the cast, just the warning about cast.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#44 lore raw

PATCH 17/26 xtensa: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 arch/xtensa/kernel/traps.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/xtensa/kernel/traps.c +2 -4
index 691a792..bf57478 100644
@@ -441,10 +441,8 @@ void show_trace(struct task_struct *task, unsigned long *sp)
 
 		pc = MAKE_PC_FROM_RA(a0, a1);
 
-		if (kernel_text_address(pc)) {
-			printk(" [<%08lx>] ", pc);
-			print_symbol("%s\n", pc);
-		}
+		if (kernel_text_address(pc))
+			printk(" [<%08lx>] %pSR\n", pc, (void *)pc);
 	}
 	printk("\n");
 }
Signature · 1 line
1.7.8.112.g3fd21
#45 lore raw

PATCH 18/26 drivers: base: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 drivers/base/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/base/core.c +2 -2
index a235085..95159e1 100644
@@ -97,8 +97,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
 	if (dev_attr->show)
 		ret = dev_attr->show(dev, dev_attr, buf);
 	if (ret >= (ssize_t)PAGE_SIZE) {
-		print_symbol("dev_attr_show: %s returned bad count\n",
-				(unsigned long)dev_attr->show);
+		printk("dev_attr_show: %pSR returned bad count\n",
+		       dev_attr->show);
 	}
 	return ret;
 }
Signature · 1 line
1.7.8.112.g3fd21
#46 lore raw

PATCH 19/26 gfs2: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 fs/gfs2/glock.c |    4 ++--
 fs/gfs2/trans.c |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)
fs/gfs2/glock.c +2 -2
index 992c5c0..453978b 100644
@@ -1016,11 +1016,11 @@ do_cancel:
 	return;
 
 trap_recursive:
-	print_symbol(KERN_ERR "original: %s\n", gh2->gh_ip);
+	printk(KERN_ERR "original: %pSR\n", (void *)gh2->gh_ip);
 	printk(KERN_ERR "pid: %d\n", pid_nr(gh2->gh_owner_pid));
 	printk(KERN_ERR "lock type: %d req lock state : %d\n",
 	       gh2->gh_gl->gl_name.ln_type, gh2->gh_state);
-	print_symbol(KERN_ERR "new: %s\n", gh->gh_ip);
+	printk(KERN_ERR "new: %pSR\n", (void *)gh->gh_ip);
 	printk(KERN_ERR "pid: %d\n", pid_nr(gh->gh_owner_pid));
 	printk(KERN_ERR "lock type: %d req lock state : %d\n",
 	       gh->gh_gl->gl_name.ln_type, gh->gh_state);
fs/gfs2/trans.c +2 -1
index 4136270..3d8aa7f 100644
@@ -95,7 +95,8 @@ static void gfs2_log_release(struct gfs2_sbd *sdp, unsigned int blks)
 
 static void gfs2_print_trans(const struct gfs2_trans *tr)
 {
-	print_symbol(KERN_WARNING "GFS2: Transaction created at: %s\n", tr->tr_ip);
+	printk(KERN_WARNING "GFS2: Transaction created at: %pSR\n",
+	       (void *)tr->tr_ip);
 	printk(KERN_WARNING "GFS2: blocks=%u revokes=%u reserved=%u touched=%d\n",
 	       tr->tr_blocks, tr->tr_revokes, tr->tr_reserved, tr->tr_touched);
 	printk(KERN_WARNING "GFS2: Buf %u/%u Databuf %u/%u Revoke %u/%u\n",
Signature · 1 line
1.7.8.112.g3fd21
Acked-by on v1 19/26
#47 lore raw

Re: [TRIVIAL PATCH 19/26] gfs2: Convert print_symbol to %pSR

Hi,

Steve.

Quoted 41 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 fs/gfs2/glock.c |    4 ++--
 fs/gfs2/trans.c |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)
fs/gfs2/glock.c +2 -2
index 992c5c0..453978b 100644
@@ -1016,11 +1016,11 @@ do_cancel:
 	return;
 
 trap_recursive:
-	print_symbol(KERN_ERR "original: %s\n", gh2->gh_ip);
+	printk(KERN_ERR "original: %pSR\n", (void *)gh2->gh_ip);
 	printk(KERN_ERR "pid: %d\n", pid_nr(gh2->gh_owner_pid));
 	printk(KERN_ERR "lock type: %d req lock state : %d\n",
 	       gh2->gh_gl->gl_name.ln_type, gh2->gh_state);
-	print_symbol(KERN_ERR "new: %s\n", gh->gh_ip);
+	printk(KERN_ERR "new: %pSR\n", (void *)gh->gh_ip);
 	printk(KERN_ERR "pid: %d\n", pid_nr(gh->gh_owner_pid));
 	printk(KERN_ERR "lock type: %d req lock state : %d\n",
 	       gh->gh_gl->gl_name.ln_type, gh->gh_state);
fs/gfs2/trans.c +2 -1
index 4136270..3d8aa7f 100644
@@ -95,7 +95,8 @@ static void gfs2_log_release(struct gfs2_sbd *sdp, unsigned int blks)
 
 static void gfs2_print_trans(const struct gfs2_trans *tr)
 {
-	print_symbol(KERN_WARNING "GFS2: Transaction created at: %s\n", tr->tr_ip);
+	printk(KERN_WARNING "GFS2: Transaction created at: %pSR\n",
+	       (void *)tr->tr_ip);
 	printk(KERN_WARNING "GFS2: blocks=%u revokes=%u reserved=%u touched=%d\n",
 	       tr->tr_blocks, tr->tr_revokes, tr->tr_reserved, tr->tr_touched);
 	printk(KERN_WARNING "GFS2: Buf %u/%u Databuf %u/%u Revoke %u/%u\n",
#49 lore raw

PATCH 20/26 sysfs: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 fs/sysfs/file.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fs/sysfs/file.c +2 -2
index 602f56d..bd7e59b 100644
@@ -91,8 +91,8 @@ static int fill_read_buffer(struct dentry * dentry, struct sysfs_buffer * buffer
 	 * indicate truncated result or overflow in normal use cases.
 	 */
 	if (count >= (ssize_t)PAGE_SIZE) {
-		print_symbol("fill_read_buffer: %s returned bad count\n",
-			(unsigned long)ops->show);
+		printk("fill_read_buffer: %pSR returned bad count\n",
+		       ops->show);
 		/* Try to struggle along */
 		count = PAGE_SIZE - 1;
 	}
Signature · 1 line
1.7.8.112.g3fd21
#50 lore raw

PATCH 21/26 irq: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 kernel/irq/debug.h |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)
kernel/irq/debug.h +7 -8
index e75e29e..70f487d9 100644
@@ -13,15 +13,14 @@ static inline void print_irq_desc(unsigned int irq, struct irq_desc *desc)
 {
 	printk("irq %d, desc: %p, depth: %d, count: %d, unhandled: %d\n",
 		irq, desc, desc->depth, desc->irq_count, desc->irqs_unhandled);
-	printk("->handle_irq():  %p, ", desc->handle_irq);
-	print_symbol("%s\n", (unsigned long)desc->handle_irq);
-	printk("->irq_data.chip(): %p, ", desc->irq_data.chip);
-	print_symbol("%s\n", (unsigned long)desc->irq_data.chip);
+	printk("->handle_irq():  %p, %pSR\n",
+	       desc->handle_irq, desc->handle_irq);
+	printk("->irq_data.chip(): %p, %pSR\n",
+	       desc->irq_data.chip, desc->irq_data.chip);
 	printk("->action(): %p\n", desc->action);
-	if (desc->action) {
-		printk("->action->handler(): %p, ", desc->action->handler);
-		print_symbol("%s\n", (unsigned long)desc->action->handler);
-	}
+	if (desc->action)
+		printk("->action->handler(): %p, %pSR\n",
+		       desc->action->handler, desc->action->handler);
 
 	___P(IRQ_LEVEL);
 	___P(IRQ_PER_CPU);
Signature · 1 line
1.7.8.112.g3fd21
#51 lore raw

PATCH 22/26 smp_processor_id: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 lib/smp_processor_id.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
lib/smp_processor_id.c +1 -1
index 4c0d0e5..37e59c4 100644
@@ -42,7 +42,7 @@ notrace unsigned int debug_smp_processor_id(void)
 	printk(KERN_ERR "BUG: using smp_processor_id() in preemptible [%08x] "
 			"code: %s/%d\n",
 			preempt_count() - 1, current->comm, current->pid);
-	print_symbol("caller is %s\n", (long)__builtin_return_address(0));
+	printk("caller is %pSR\n", __builtin_return_address(0));
 	dump_stack();
 
 out_enable:
Signature · 1 line
1.7.8.112.g3fd21
#52 lore raw

PATCH 23/26 mm: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---
 mm/memory.c |    8 ++++----
 mm/slab.c   |    8 +++-----
 2 files changed, 7 insertions(+), 9 deletions(-)
mm/memory.c +4 -4
index d81bdd7..a14c194 100644
@@ -711,11 +711,11 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 	 * Choose text because data symbols depend on CONFIG_KALLSYMS_ALL=y
 	 */
 	if (vma->vm_ops)
-		print_symbol(KERN_ALERT "vma->vm_ops->fault: %s\n",
-				(unsigned long)vma->vm_ops->fault);
+		printk(KERN_ALERT "vma->vm_ops->fault: %pSR\n",
+		       vma->vm_ops->fault);
 	if (vma->vm_file && vma->vm_file->f_op)
-		print_symbol(KERN_ALERT "vma->vm_file->f_op->mmap: %s\n",
-				(unsigned long)vma->vm_file->f_op->mmap);
+		printk(KERN_ALERT "vma->vm_file->f_op->mmap: %pSR\n",
+		       vma->vm_file->f_op->mmap);
 	dump_stack();
 	add_taint(TAINT_BAD_PAGE);
 }
mm/slab.c +3 -5
index 23daffa..d23ab4f 100644
@@ -2081,11 +2081,9 @@ static void print_objinfo(struct kmem_cache *cachep, void *objp, int lines)
 	}
 
 	if (cachep->flags & SLAB_STORE_USER) {
-		printk(KERN_ERR "Last user: [<%p>]",
-			*dbg_userword(cachep, objp));
-		print_symbol("(%s)",
-				(unsigned long)*dbg_userword(cachep, objp));
-		printk("\n");
+		printk(KERN_ERR "Last user: [<%p>](%pSR)\n",
+		       *dbg_userword(cachep, objp),
+		       *dbg_userword(cachep, objp));
 	}
 	realobj = (char *)objp + obj_offset(cachep);
 	size = cachep->object_size;
Signature · 7 lines
1.7.8.112.g3fd21

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Acked-by on v1 23/26
#53 lore raw

Re: [TRIVIAL PATCH 23/26] mm: Convert print_symbol to %pSR

Quoted 2 lines · Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#54 lore raw

Re: [TRIVIAL PATCH 23/26] mm: Convert print_symbol to %pSR

Quoted 4 lines · Christoph Lameter
Quoted 2 lines

Use the new vsprintf extension to avoid any possible message interleaving.

Doesn't seem to be in linux-next as of today, I am taking this one.

Signature · 8 lines
Jiri Kosina
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#55 lore raw

PATCH 24/26 xtensa: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving and set a good example for Documentation too.

---
 Documentation/filesystems/sysfs.txt       |    4 ++--
 Documentation/zh_CN/filesystems/sysfs.txt |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/filesystems/sysfs.txt +2 -2
index a6619b7..9cc7742 100644
@@ -154,8 +154,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pSR returned bad count\n",
+                       dev_attr->show);
         }
         return ret;
 }
Documentation/zh_CN/filesystems/sysfs.txt +2 -2
index e230eaa..42fc023 100644
@@ -167,8 +167,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pSR returned bad count\n",
+                       dev_attr->show);
         }
         return ret;
 }
Signature · 1 line
1.7.8.112.g3fd21
#56 lore raw

PATCH 25/26 x86: head_64.S: Use vsprintf extension %pSR not print_symbol

In reply to Joe Perches

print_symbol will be removed so replace this instance with the %pSR vsprintf extension.

---
 arch/x86/kernel/head_64.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/head_64.S +2 -2
index 980053c..37575a5 100644
@@ -344,7 +344,7 @@ ENTRY(early_idt_handler)
 #ifdef CONFIG_KALLSYMS	
 	leaq early_idt_ripmsg(%rip),%rdi
 	movq 40(%rsp),%rsi	# %rip again
-	call __print_symbol
+	call early_printk
 #endif
 #endif /* EARLY_PRINTK */
 1:	hlt
@@ -372,7 +372,7 @@ early_recursion_flag:
 early_idt_msg:
 	.asciz "PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"
 early_idt_ripmsg:
-	.asciz "RIP %s\n"
+	.asciz "RIP %pSR\n"
 #endif /* CONFIG_EARLY_PRINTK */
 	.previous
 
Signature · 1 line
1.7.8.112.g3fd21
#57 lore raw

PATCH 26/26 kallsyms: Remove print_symbol

In reply to Joe Perches

This is now unused, remove it.

---
 include/linux/kallsyms.h |   18 ------------------
 kernel/kallsyms.c        |   11 -----------
 2 files changed, 0 insertions(+), 29 deletions(-)
include/linux/kallsyms.h +0 -18
index 6883e19..b7a60ba 100644
@@ -39,9 +39,6 @@ extern int sprint_symbol(char *buffer, unsigned long address);
 extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
 extern int sprint_backtrace(char *buffer, unsigned long address);
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-extern void __print_symbol(const char *fmt, unsigned long address);
-
 int lookup_symbol_name(unsigned long addr, char *symname);
 int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
@@ -103,23 +100,8 @@ static inline int lookup_symbol_attrs(unsigned long addr, unsigned long *size, u
 	return -ERANGE;
 }
 
-/* Stupid that this does nothing, but I didn't create this mess. */
-#define __print_symbol(fmt, addr)
 #endif /*CONFIG_KALLSYMS*/
 
-/* This macro allows us to keep printk typechecking */
-static __printf(1, 2)
-void __check_printsym_format(const char *fmt, ...)
-{
-}
-
-static inline void print_symbol(const char *fmt, unsigned long addr)
-{
-	__check_printsym_format(fmt, "");
-	__print_symbol(fmt, (unsigned long)
-		       __builtin_extract_return_addr((void *)addr));
-}
-
 static inline void print_ip_sym(unsigned long ip)
 {
 	printk("[<%p>] %pS\n", (void *) ip, (void *) ip);
kernel/kallsyms.c +0 -11
index 2169fee..6733476 100644
@@ -422,17 +422,6 @@ int sprint_backtrace(char *buffer, unsigned long address)
 	return __sprint_symbol(buffer, address, -1, 1);
 }
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-void __print_symbol(const char *fmt, unsigned long address)
-{
-	char buffer[KSYM_SYMBOL_LEN];
-
-	sprint_symbol(buffer, address);
-
-	printk(fmt, buffer);
-}
-EXPORT_SYMBOL(__print_symbol);
-
 /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
 struct kallsym_iter {
 	loff_t pos;
Signature · 1 line
1.7.8.112.g3fd21
#58 lore raw

PATCH v2 12/26 s390: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---

V2: Add missing parentheses, remove comma

 arch/s390/kernel/traps.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)
arch/s390/kernel/traps.c +16 -12
index 70ecfc5..f718e12 100644
@@ -91,8 +91,9 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 		if (sp < low || sp > high - sizeof(*sf))
 			return sp;
 		sf = (struct stack_frame *) sp;
-		printk("([<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-		print_symbol("%s)\n", sf->gprs[8] & PSW_ADDR_INSN);
+		printk("([<%016lx>] %pSR)\n",
+		       sf->gprs[8] & PSW_ADDR_INSN,
+		       (void *)(sf->gprs[8] & PSW_ADDR_INSN));
 		/* Follow the backchain. */
 		while (1) {
 			low = sp;
@@ -102,16 +103,18 @@ __show_trace(unsigned long sp, unsigned long low, unsigned long high)
 			if (sp <= low || sp > high - sizeof(*sf))
 				return sp;
 			sf = (struct stack_frame *) sp;
-			printk(" [<%016lx>] ", sf->gprs[8] & PSW_ADDR_INSN);
-			print_symbol("%s\n", sf->gprs[8] & PSW_ADDR_INSN);
+			printk(" [<%016lx>] %pSR\n",
+			       sf->gprs[8] & PSW_ADDR_INSN,
+			       (void *)(sf->gprs[8] & PSW_ADDR_INSN));
 		}
 		/* Zero backchain detected, check for interrupt frame. */
 		sp = (unsigned long) (sf + 1);
 		if (sp <= low || sp > high - sizeof(*regs))
 			return sp;
 		regs = (struct pt_regs *) sp;
-		printk(" [<%016lx>] ", regs->psw.addr & PSW_ADDR_INSN);
-		print_symbol("%s\n", regs->psw.addr & PSW_ADDR_INSN);
+		printk(" [<%016lx>] %pSR\n",
+		       regs->psw.addr & PSW_ADDR_INSN,
+		       (void *)(regs->psw.addr & PSW_ADDR_INSN));
 		low = sp;
 		sp = regs->gprs[15];
 	}
@@ -169,8 +172,9 @@ static void show_last_breaking_event(struct pt_regs *regs)
 {
 #ifdef CONFIG_64BIT
 	printk("Last Breaking-Event-Address:\n");
-	printk(" [<%016lx>] ", regs->args[0] & PSW_ADDR_INSN);
-	print_symbol("%s\n", regs->args[0] & PSW_ADDR_INSN);
+	printk(" [<%016lx>] %pSR\n",
+	       regs->args[0] & PSW_ADDR_INSN,
+	       (void *)(regs->args[0] & PSW_ADDR_INSN));
 #endif
 }
 
@@ -201,10 +205,10 @@ void show_registers(struct pt_regs *regs)
 	char *mode;
 
 	mode = user_mode(regs) ? "User" : "Krnl";
-	printk("%s PSW : %p %p",
-	       mode, (void *) regs->psw.mask,
-	       (void *) regs->psw.addr);
-	print_symbol(" (%s)\n", regs->psw.addr & PSW_ADDR_INSN);
+	printk("%s PSW : %p %p (%pSR)\n",
+	       mode, (void *)regs->psw.mask,
+	       (void *)regs->psw.addr,
+	       (void *)(regs->psw.addr & PSW_ADDR_INSN));
 	printk("           R:%x T:%x IO:%x EX:%x Key:%x M:%x W:%x "
 	       "P:%x AS:%x CC:%x PM:%x", mask_bits(regs, PSW_MASK_PER),
 	       mask_bits(regs, PSW_MASK_DAT), mask_bits(regs, PSW_MASK_IO),
Signature · 1 line
1.7.8.112.g3fd21
#59 lore raw

PATCH v2 16/26 x86: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving.

---

V2: mce.c: Add cast to (unsigned long) before cast to (void *)
to quiet the compiler on x86-32

 arch/x86/kernel/cpu/mcheck/mce.c |   13 +++++++------
 arch/x86/kernel/dumpstack.c      |    5 ++---
 arch/x86/kernel/process_32.c     |    2 +-
 arch/x86/mm/mmio-mod.c           |    4 ++--
 arch/x86/um/sysrq_32.c           |    9 +++------
 5 files changed, 15 insertions(+), 18 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +7 -6
index 80dbda8..996f98a 100644
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
 	       m->extcpu, m->mcgstatus, m->bank, m->status);
 
 	if (m->ip) {
-		pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
-			!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
-				m->cs, m->ip);
-
 		if (m->cs == __KERNEL_CS)
-			print_symbol("{%s}", m->ip);
-		pr_cont("\n");
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip, (void *)(unsigned long)m->ip);
+		else
+			pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+				 !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+				 m->cs, m->ip);
 	}
 
 	pr_emerg(HW_ERR "TSC %llx ", m->tsc);
arch/x86/kernel/dumpstack.c +2 -3
index ae42418b..b6e5bdb 100644
@@ -281,9 +281,8 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
 		sp = kernel_stack_pointer(regs);
 		savesegment(ss, ss);
 	}
-	printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip);
-	print_symbol("%s", regs->ip);
-	printk(" SS:ESP %04x:%08lx\n", ss, sp);
+	printk(KERN_EMERG "EIP: [<%08lx>] %pSR SS:ESP %04x:%08lx\n",
+	       regs->ip, (void *)regs->ip, ss, sp);
 #else
 	/* Executive summary in case the oops scrolled away */
 	printk(KERN_ALERT "RIP ");
arch/x86/kernel/process_32.c +1 -1
index b5a8905..0db77e0 100644
@@ -89,7 +89,7 @@ void __show_regs(struct pt_regs *regs, int all)
 	printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n",
 			(u16)regs->cs, regs->ip, regs->flags,
 			smp_processor_id());
-	print_symbol("EIP is at %s\n", regs->ip);
+	printk(KERN_DEFAULT "EIP is at %pSR\n", (void *)regs->ip);
 
 	printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n",
 		regs->ax, regs->bx, regs->cx, regs->dx);
arch/x86/mm/mmio-mod.c +2 -2
index dc0b727..c0ca484 100644
@@ -123,8 +123,8 @@ static void die_kmmio_nesting_error(struct pt_regs *regs, unsigned long addr)
 	pr_emerg("unexpected fault for address: 0x%08lx, last fault for address: 0x%08lx\n",
 		 addr, my_reason->addr);
 	print_pte(addr);
-	print_symbol(KERN_EMERG "faulting IP is at %s\n", regs->ip);
-	print_symbol(KERN_EMERG "last faulting IP was at %s\n", my_reason->ip);
+	pr_emerg("faulting IP is at %pSR\n", (void *)regs->ip);
+	pr_emerg("last faulting IP was at %pSR\n", (void *)my_reason->ip);
 #ifdef __i386__
 	pr_emerg("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		 regs->ax, regs->bx, regs->cx, regs->dx);
arch/x86/um/sysrq_32.c +3 -6
index c9bee5b..30b4200 100644
@@ -50,18 +50,15 @@ static inline unsigned long print_context_stack(struct thread_info *tinfo,
 #ifdef CONFIG_FRAME_POINTER
 	while (valid_stack_ptr(tinfo, (void *)ebp)) {
 		addr = *(unsigned long *)(ebp + 4);
-		printk("%08lx:  [<%08lx>]", ebp + 4, addr);
-		print_symbol(" %s", addr);
-		printk("\n");
+		printk("%08lx:  [<%08lx>] %pSR\n", ebp + 4, addr, (void *)addr);
 		ebp = *(unsigned long *)ebp;
 	}
 #else
 	while (valid_stack_ptr(tinfo, stack)) {
 		addr = *stack;
 		if (__kernel_text_address(addr)) {
-			printk("%08lx:  [<%08lx>]", (unsigned long) stack, addr);
-			print_symbol(" %s", addr);
-			printk("\n");
+			printk("%08lx:  [<%08lx>] %pSR\n",
+			       (unsigned long)stack, addr, (void *)addr);
 		}
 		stack++;
 	}
Signature · 7 lines
1.7.8.112.g3fd21

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
#60 lore raw

PATCH v2 24/26 Documentation: Convert print_symbol to %pSR

In reply to Joe Perches

Use the new vsprintf extension to avoid any possible message interleaving and set a good example for Documentation too.

---
V2: Fix commit subject line (Thanks to Marc Gauthier <[email protected]>)

 Documentation/filesystems/sysfs.txt       |    4 ++--
 Documentation/zh_CN/filesystems/sysfs.txt |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/filesystems/sysfs.txt +2 -2
index a6619b7..9cc7742 100644
@@ -154,8 +154,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pSR returned bad count\n",
+                       dev_attr->show);
         }
         return ret;
 }
Documentation/zh_CN/filesystems/sysfs.txt +2 -2
index e230eaa..42fc023 100644
@@ -167,8 +167,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pSR returned bad count\n",
+                       dev_attr->show);
         }
         return ret;
 }
Signature · 1 line
1.7.8.112.g3fd21
#61 lore raw
In reply to Joe Perches
Quoted 2 lines · Joe Perches

Remove the somewhat awkward uses of print_symbol and convert all the existing uses to a new vsprintf pointer type of %pSR.

Jiri? Are you going to do anything with this?

#62 lore raw
Quoted 4 lines · Joe Perches
Quoted 2 lines

Remove the somewhat awkward uses of print_symbol and convert all the existing uses to a new vsprintf pointer type of %pSR.

Jiri? Are you going to do anything with this?

Joe,

yup, it's still in my TODO queue for trivial.git.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
#63 lore raw
Quoted 10 lines · Jiri Kosina
Quoted 4 lines · Joe Perches
Quoted 2 lines

Remove the somewhat awkward uses of print_symbol and convert all the existing uses to a new vsprintf pointer type of %pSR.

Jiri? Are you going to do anything with this?

Joe,

yup, it's still in my TODO queue for trivial.git.

Jiri?

Is this still in your TODO queue?
If so, for what version?