linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] early_printk: consolidate random copies of identical code
@ 2013-03-07 19:15 Paul Gortmaker
  2013-03-07 19:25 ` Andrew Morton
  0 siblings, 1 reply; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-07 19:15 UTC (permalink / raw)
  To: akpm
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger, Paul Gortmaker

From: Thomas Gleixner <tglx@linutronix.de>

The early console implementations are the same all over the place.  Move
the print function to kernel/printk and get rid of the copies.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Richard Weinberger <richard@nod.at>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---

[v2: essentially unchanged since v1, so I've left the acked/reviewed
 tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
 and PRINTK=n, because the early_console struct and early_printk calls
 were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
 exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
 and still works for everyday sane configs too.]
 [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   

 arch/arm/kernel/early_printk.c        | 17 +++--------------
 arch/blackfin/kernel/early_printk.c   |  2 --
 arch/microblaze/kernel/early_printk.c | 26 ++++----------------------
 arch/mips/kernel/early_printk.c       | 11 +++++------
 arch/powerpc/kernel/udbg.c            |  6 ++----
 arch/sh/kernel/sh_bios.c              |  2 --
 arch/sparc/kernel/setup_32.c          |  1 +
 arch/sparc/kernel/setup_64.c          |  8 +++++++-
 arch/tile/kernel/early_printk.c       | 27 +++++----------------------
 arch/um/kernel/early_printk.c         |  8 +++++---
 arch/unicore32/kernel/early_printk.c  | 12 ++++--------
 arch/x86/kernel/early_printk.c        | 21 ++-------------------
 include/linux/console.h               |  1 +
 include/linux/printk.h                |  6 ++++++
 kernel/printk.c                       | 30 +++++++++++++++++++++++-------
 15 files changed, 68 insertions(+), 110 deletions(-)

diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
index 85aa2b2..4307653 100644
--- a/arch/arm/kernel/early_printk.c
+++ b/arch/arm/kernel/early_printk.c
@@ -29,28 +29,17 @@ static void early_console_write(struct console *con, const char *s, unsigned n)
 	early_write(s, n);
 }
 
-static struct console early_console = {
+static struct console early_console_dev = {
 	.name =		"earlycon",
 	.write =	early_console_write,
 	.flags =	CON_PRINTBUFFER | CON_BOOT,
 	.index =	-1,
 };
 
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	va_start(ap, fmt);
-	n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_write(buf, n);
-	va_end(ap);
-}
-
 static int __init setup_early_printk(char *buf)
 {
-	register_console(&early_console);
+	early_console = &early_console_dev;
+	register_console(&early_console_dev);
 	return 0;
 }
 
diff --git a/arch/blackfin/kernel/early_printk.c b/arch/blackfin/kernel/early_printk.c
index 84ed837..61fbd2d 100644
--- a/arch/blackfin/kernel/early_printk.c
+++ b/arch/blackfin/kernel/early_printk.c
@@ -25,8 +25,6 @@ extern struct console *bfin_earlyserial_init(unsigned int port,
 extern struct console *bfin_jc_early_init(void);
 #endif
 
-static struct console *early_console;
-
 /* Default console */
 #define DEFAULT_PORT 0
 #define DEFAULT_CFLAG CS8|B57600
diff --git a/arch/microblaze/kernel/early_printk.c b/arch/microblaze/kernel/early_printk.c
index 60dcacc..365f2d5 100644
--- a/arch/microblaze/kernel/early_printk.c
+++ b/arch/microblaze/kernel/early_printk.c
@@ -21,7 +21,6 @@
 #include <asm/setup.h>
 #include <asm/prom.h>
 
-static u32 early_console_initialized;
 static u32 base_addr;
 
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
@@ -109,27 +108,11 @@ static struct console early_serial_uart16550_console = {
 };
 #endif /* CONFIG_SERIAL_8250_CONSOLE */
 
-static struct console *early_console;
-
-void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	if (early_console_initialized) {
-		va_start(ap, fmt);
-		n = vscnprintf(buf, 512, fmt, ap);
-		early_console->write(early_console, buf, n);
-		va_end(ap);
-	}
-}
-
 int __init setup_early_printk(char *opt)
 {
 	int version = 0;
 
-	if (early_console_initialized)
+	if (early_console)
 		return 1;
 
 	base_addr = of_early_console(&version);
@@ -159,7 +142,6 @@ int __init setup_early_printk(char *opt)
 		}
 
 		register_console(early_console);
-		early_console_initialized = 1;
 		return 0;
 	}
 	return 1;
@@ -169,7 +151,7 @@ int __init setup_early_printk(char *opt)
  * only for early console because of performance degression */
 void __init remap_early_printk(void)
 {
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	pr_info("early_printk_console remapping from 0x%x to ", base_addr);
 	base_addr = (u32) ioremap(base_addr, PAGE_SIZE);
@@ -194,9 +176,9 @@ void __init remap_early_printk(void)
 
 void __init disable_early_printk(void)
 {
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	pr_warn("disabling early console\n");
 	unregister_console(early_console);
-	early_console_initialized = 0;
+	early_console = NULL;
 }
diff --git a/arch/mips/kernel/early_printk.c b/arch/mips/kernel/early_printk.c
index 9e6440e..21150cd 100644
--- a/arch/mips/kernel/early_printk.c
+++ b/arch/mips/kernel/early_printk.c
@@ -8,6 +8,7 @@
  *   written by Ralf Baechle (ralf@linux-mips.org)
  */
 #include <linux/console.h>
+#include <linux/printk.h>
 #include <linux/init.h>
 
 #include <asm/setup.h>
@@ -24,20 +25,18 @@ static void early_console_write(struct console *con, const char *s, unsigned n)
 	}
 }
 
-static struct console early_console = {
+static struct console early_console_prom = {
 	.name	= "early",
 	.write	= early_console_write,
 	.flags	= CON_PRINTBUFFER | CON_BOOT,
 	.index	= -1
 };
 
-static int early_console_initialized __initdata;
-
 void __init setup_early_printk(void)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return;
-	early_console_initialized = 1;
+	early_console = &early_console_prom;
 
-	register_console(&early_console);
+	register_console(&early_console_prom);
 }
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index f974849..13b8670 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -156,15 +156,13 @@ static struct console udbg_console = {
 	.index	= 0,
 };
 
-static int early_console_initialized;
-
 /*
  * Called by setup_system after ppc_md->probe and ppc_md->early_init.
  * Call it again after setting udbg_putc in ppc_md->setup_arch.
  */
 void __init register_early_udbg_console(void)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return;
 
 	if (!udbg_putc)
@@ -174,7 +172,7 @@ void __init register_early_udbg_console(void)
 		printk(KERN_INFO "early console immortal !\n");
 		udbg_console.flags &= ~CON_BOOT;
 	}
-	early_console_initialized = 1;
+	early_console = &udbg_console;
 	register_console(&udbg_console);
 }
 
diff --git a/arch/sh/kernel/sh_bios.c b/arch/sh/kernel/sh_bios.c
index 47475cc..a5b51b9 100644
--- a/arch/sh/kernel/sh_bios.c
+++ b/arch/sh/kernel/sh_bios.c
@@ -144,8 +144,6 @@ static struct console bios_console = {
 	.index		= -1,
 };
 
-static struct console *early_console;
-
 static int __init setup_early_printk(char *buf)
 {
 	int keep_early = 0;
diff --git a/arch/sparc/kernel/setup_32.c b/arch/sparc/kernel/setup_32.c
index 38bf80a..f4fb00e 100644
--- a/arch/sparc/kernel/setup_32.c
+++ b/arch/sparc/kernel/setup_32.c
@@ -309,6 +309,7 @@ void __init setup_arch(char **cmdline_p)
 
 	boot_flags_init(*cmdline_p);
 
+	early_console = &prom_early_console;
 	register_console(&prom_early_console);
 
 	printk("ARCH: ");
diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
index 88a127b..d9c57e9 100644
--- a/arch/sparc/kernel/setup_64.c
+++ b/arch/sparc/kernel/setup_64.c
@@ -551,6 +551,12 @@ static void __init init_sparc64_elf_hwcap(void)
 		pause_patch();
 }
 
+static inline void register_prom_console(void)
+{
+	early_console = &prom_early_console;
+	register_console(&prom_early_console);
+}
+
 void __init setup_arch(char **cmdline_p)
 {
 	/* Initialize PROM console and command line. */
@@ -562,7 +568,7 @@ void __init setup_arch(char **cmdline_p)
 #ifdef CONFIG_EARLYFB
 	if (btext_find_display())
 #endif
-		register_console(&prom_early_console);
+		register_prom_console();
 
 	if (tlb_type == hypervisor)
 		printk("ARCH: SUN4V\n");
diff --git a/arch/tile/kernel/early_printk.c b/arch/tile/kernel/early_printk.c
index afb9c9a..34d72a1 100644
--- a/arch/tile/kernel/early_printk.c
+++ b/arch/tile/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/irqflags.h>
+#include <linux/printk.h>
 #include <asm/setup.h>
 #include <hv/hypervisor.h>
 
@@ -33,25 +34,8 @@ static struct console early_hv_console = {
 };
 
 /* Direct interface for emergencies */
-static struct console *early_console = &early_hv_console;
-static int early_console_initialized;
 static int early_console_complete;
 
-static void early_vprintk(const char *fmt, va_list ap)
-{
-	char buf[512];
-	int n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_console->write(early_console, buf, n);
-}
-
-void early_printk(const char *fmt, ...)
-{
-	va_list ap;
-	va_start(ap, fmt);
-	early_vprintk(fmt, ap);
-	va_end(ap);
-}
-
 void early_panic(const char *fmt, ...)
 {
 	va_list ap;
@@ -69,14 +53,13 @@ static int __initdata keep_early;
 
 static int __init setup_early_printk(char *str)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return 1;
 
 	if (str != NULL && strncmp(str, "keep", 4) == 0)
 		keep_early = 1;
 
 	early_console = &early_hv_console;
-	early_console_initialized = 1;
 	register_console(early_console);
 
 	return 0;
@@ -85,12 +68,12 @@ static int __init setup_early_printk(char *str)
 void __init disable_early_printk(void)
 {
 	early_console_complete = 1;
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	if (!keep_early) {
 		early_printk("disabling early console\n");
 		unregister_console(early_console);
-		early_console_initialized = 0;
+		early_console = NULL;
 	} else {
 		early_printk("keeping early console\n");
 	}
@@ -98,7 +81,7 @@ void __init disable_early_printk(void)
 
 void warn_early_printk(void)
 {
-	if (early_console_complete || early_console_initialized)
+	if (early_console_complete || early_console)
 		return;
 	early_printk("\
 Machine shutting down before console output is fully initialized.\n\
diff --git a/arch/um/kernel/early_printk.c b/arch/um/kernel/early_printk.c
index 49480f0..4a0800b 100644
--- a/arch/um/kernel/early_printk.c
+++ b/arch/um/kernel/early_printk.c
@@ -16,7 +16,7 @@ static void early_console_write(struct console *con, const char *s, unsigned int
 	um_early_printk(s, n);
 }
 
-static struct console early_console = {
+static struct console early_console_dev = {
 	.name = "earlycon",
 	.write = early_console_write,
 	.flags = CON_BOOT,
@@ -25,8 +25,10 @@ static struct console early_console = {
 
 static int __init setup_early_printk(char *buf)
 {
-	register_console(&early_console);
-
+	if (!early_console) {
+		early_console = &early_console_dev;
+		register_console(&early_console_dev);
+	}
 	return 0;
 }
 
diff --git a/arch/unicore32/kernel/early_printk.c b/arch/unicore32/kernel/early_printk.c
index 3922255..9be0d5d 100644
--- a/arch/unicore32/kernel/early_printk.c
+++ b/arch/unicore32/kernel/early_printk.c
@@ -33,21 +33,17 @@ static struct console early_ocd_console = {
 	.index =	-1,
 };
 
-/* Direct interface for emergencies */
-static struct console *early_console = &early_ocd_console;
-
-static int __initdata keep_early;
-
 static int __init setup_early_printk(char *buf)
 {
-	if (!buf)
+	int keep_early;
+
+	if (!buf || early_console)
 		return 0;
 
 	if (strstr(buf, "keep"))
 		keep_early = 1;
 
-	if (!strncmp(buf, "ocd", 3))
-		early_console = &early_ocd_console;
+	early_console = &early_ocd_console;
 
 	if (keep_early)
 		early_console->flags &= ~CON_BOOT;
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 9b9f18b..d15f575 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -169,25 +169,9 @@ static struct console early_serial_console = {
 	.index =	-1,
 };
 
-/* Direct interface for emergencies */
-static struct console *early_console = &early_vga_console;
-static int __initdata early_console_initialized;
-
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	va_start(ap, fmt);
-	n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_console->write(early_console, buf, n);
-	va_end(ap);
-}
-
 static inline void early_console_register(struct console *con, int keep_early)
 {
-	if (early_console->index != -1) {
+	if (con->index != -1) {
 		printk(KERN_CRIT "ERROR: earlyprintk= %s already used\n",
 		       con->name);
 		return;
@@ -207,9 +191,8 @@ static int __init setup_early_printk(char *buf)
 	if (!buf)
 		return 0;
 
-	if (early_console_initialized)
+	if (early_console)
 		return 0;
-	early_console_initialized = 1;
 
 	keep = (strstr(buf, "keep") != NULL);
 
diff --git a/include/linux/console.h b/include/linux/console.h
index 29680a8..73bab0f 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -141,6 +141,7 @@ struct console {
 	for (con = console_drivers; con != NULL; con = con->next)
 
 extern int console_set_on_cmdline;
+extern struct console *early_console;
 
 extern int add_preferred_console(char *name, int idx, char *options);
 extern int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options);
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1249a54..b846afd 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -95,8 +95,14 @@ int no_printk(const char *fmt, ...)
 	return 0;
 }
 
+#ifdef CONFIG_EARLY_PRINTK
 extern asmlinkage __printf(1, 2)
 void early_printk(const char *fmt, ...);
+void early_vprintk(const char *fmt, va_list ap);
+#else
+static inline __printf(1, 2) __cold
+void early_printk(const char *s, ...) { }
+#endif
 
 #ifdef CONFIG_PRINTK
 asmlinkage __printf(5, 0)
diff --git a/kernel/printk.c b/kernel/printk.c
index 0b31715..7664e49 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -49,13 +49,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/printk.h>
 
-/*
- * Architectures can override it:
- */
-void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
-{
-}
-
 /* printk's without a loglevel use this.. */
 #define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVEL
 
@@ -1724,6 +1717,29 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
 
 #endif /* CONFIG_PRINTK */
 
+#ifdef CONFIG_EARLY_PRINTK
+struct console *early_console;
+
+void early_vprintk(const char *fmt, va_list ap)
+{
+	if (early_console) {
+		char buf[512];
+		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
+
+		early_console->write(early_console, buf, n);
+	}
+}
+
+asmlinkage void early_printk(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	early_vprintk(fmt, ap);
+	va_end(ap);
+}
+#endif
+
 static int __add_preferred_console(char *name, int idx, char *options,
 				   char *brl_options)
 {
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 19:15 [PATCH v2] early_printk: consolidate random copies of identical code Paul Gortmaker
@ 2013-03-07 19:25 ` Andrew Morton
  2013-03-07 19:50   ` Paul Gortmaker
  2013-03-07 20:20   ` Paul Gortmaker
  0 siblings, 2 replies; 19+ messages in thread
From: Andrew Morton @ 2013-03-07 19:25 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013 14:15:54 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> [v2: essentially unchanged since v1, so I've left the acked/reviewed
>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
>  and PRINTK=n, because the early_console struct and early_printk calls
>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
>  and still works for everyday sane configs too.]
>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   

You did this:

--- a/kernel/printk.c~early_printk-consolidate-random-copies-of-identical-code-v2
+++ a/kernel/printk.c
@@ -759,29 +759,6 @@ module_param(ignore_loglevel, bool, S_IR
 MODULE_PARM_DESC(ignore_loglevel, "ignore loglevel setting, to"
 	"print all kernel messages to the console.");
 
-#ifdef CONFIG_EARLY_PRINTK
-struct console *early_console;
-
-void early_vprintk(const char *fmt, va_list ap)
-{
-	if (early_console) {
-		char buf[512];
-		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
-
-		early_console->write(early_console, buf, n);
-	}
-}
-
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	early_vprintk(fmt, ap);
-	va_end(ap);
-}
-#endif
-
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 
 static int boot_delay; /* msecs delay after each printk during bootup */
@@ -1743,6 +1720,29 @@ static size_t cont_print_text(char *text
 
 #endif /* CONFIG_PRINTK */
 
+#ifdef CONFIG_EARLY_PRINTK
+struct console *early_console;
+
+void early_vprintk(const char *fmt, va_list ap)
+{
+	if (early_console) {
+		char buf[512];
+		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
+
+		early_console->write(early_console, buf, n);
+	}
+}
+
+asmlinkage void early_printk(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	early_vprintk(fmt, ap);
+	va_end(ap);
+}
+#endif
+
 static int __add_preferred_console(char *name, int idx, char *options,
 				   char *brl_options)
 {
_

Problem is, that won't fix the various compilation problems we've had. 
See yesterday's lkml thread "linux-next: build failure after merge of
the final tree (akpm tree related)"

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 19:25 ` Andrew Morton
@ 2013-03-07 19:50   ` Paul Gortmaker
  2013-03-07 20:05     ` Joe Perches
  2013-03-07 21:35     ` Andrew Morton
  2013-03-07 20:20   ` Paul Gortmaker
  1 sibling, 2 replies; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-07 19:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On 13-03-07 02:25 PM, Andrew Morton wrote:
> On Thu, 7 Mar 2013 14:15:54 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
>> [v2: essentially unchanged since v1, so I've left the acked/reviewed
>>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
>>  and PRINTK=n, because the early_console struct and early_printk calls
>>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
>>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
>>  and still works for everyday sane configs too.]
>>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   
> 
> You did this:
> 
> --- a/kernel/printk.c~early_printk-consolidate-random-copies-of-identical-code-v2
> +++ a/kernel/printk.c
> @@ -759,29 +759,6 @@ module_param(ignore_loglevel, bool, S_IR
>  MODULE_PARM_DESC(ignore_loglevel, "ignore loglevel setting, to"
>  	"print all kernel messages to the console.");
>  
> -#ifdef CONFIG_EARLY_PRINTK
> -struct console *early_console;
> -
> -void early_vprintk(const char *fmt, va_list ap)
> -{
> -	if (early_console) {
> -		char buf[512];
> -		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
> -
> -		early_console->write(early_console, buf, n);
> -	}
> -}
> -
> -asmlinkage void early_printk(const char *fmt, ...)
> -{
> -	va_list ap;
> -
> -	va_start(ap, fmt);
> -	early_vprintk(fmt, ap);
> -	va_end(ap);
> -}
> -#endif
> -
>  #ifdef CONFIG_BOOT_PRINTK_DELAY
>  
>  static int boot_delay; /* msecs delay after each printk during bootup */
> @@ -1743,6 +1720,29 @@ static size_t cont_print_text(char *text
>  
>  #endif /* CONFIG_PRINTK */
>  
> +#ifdef CONFIG_EARLY_PRINTK
> +struct console *early_console;
> +
> +void early_vprintk(const char *fmt, va_list ap)
> +{
> +	if (early_console) {
> +		char buf[512];
> +		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
> +
> +		early_console->write(early_console, buf, n);
> +	}
> +}
> +
> +asmlinkage void early_printk(const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	early_vprintk(fmt, ap);
> +	va_end(ap);
> +}
> +#endif
> +
>  static int __add_preferred_console(char *name, int idx, char *options,
>  				   char *brl_options)
>  {
> _
> 
> Problem is, that won't fix the various compilation problems we've had. 
> See yesterday's lkml thread "linux-next: build failure after merge of
> the final tree (akpm tree related)"

Thanks for the pointer -- I'd only found Randy's original report
and had not seen this yet.  I'll go build test on sparc and have
a look there.

This brings up a recurring question.  I was tempted to just go make
CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
pushback when trying to "fix" things like seeing ARM OMAP USB options
for an x86 build[1], and GOLDFISH virt drivers being offered even
when the end user already said no to GOLDFISH[2].

Do we want to use dependencies to reflect the real world layout of
platforms/systems, or do we want to go the minimal dependency
approach, where we are building sparc specific drivers on mips just
because we can?

I think the former is better from a user specific point of view, as
the maze of Kconfig is better as a tree topology with branches that
have clear dependencies that exclude them, versus it being a flat
monolithic space where anything can select anything.

Arguments I've heard for the latter seem to be developer centric
(i.e forcing wider build coverage on the population as a whole, etc)

Thanks,
Paul.

[1] https://lkml.org/lkml/2013/2/27/204
[2] http://marc.info/?l=linux-kernel&m=136198970523568&w=3


> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 19:50   ` Paul Gortmaker
@ 2013-03-07 20:05     ` Joe Perches
  2013-03-07 21:35     ` Andrew Morton
  1 sibling, 0 replies; 19+ messages in thread
From: Joe Perches @ 2013-03-07 20:05 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Andrew Morton, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Thomas Gleixner, Russell King, Michal Simek,
	Ralf Baechle, Benjamin Herrenschmidt, Paul Mundt,
	David S. Miller, Chris Metcalf, Richard Weinberger

On Thu, 2013-03-07 at 14:50 -0500, Paul Gortmaker wrote:
> On 13-03-07 02:25 PM, Andrew Morton wrote:
> > On Thu, 7 Mar 2013 14:15:54 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> >> [v2: essentially unchanged since v1, so I've left the acked/reviewed
> >>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
> >>  and PRINTK=n, because the early_console struct and early_printk calls
> >>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
> >>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
> >>  and still works for everyday sane configs too.]
> >>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   
[]
> This brings up a recurring question.  I was tempted to just go make
> CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
> pushback when trying to "fix" things like seeing ARM OMAP USB options
> for an x86 build[1], and GOLDFISH virt drivers being offered even
> when the end user already said no to GOLDFISH[2].

I think that's the right solution and I see no
obvious insurmountable downside.

http://lkml.org/lkml/2012/9/2/97



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 19:25 ` Andrew Morton
  2013-03-07 19:50   ` Paul Gortmaker
@ 2013-03-07 20:20   ` Paul Gortmaker
  2013-03-07 21:25     ` Thomas Gleixner
  1 sibling, 1 reply; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-07 20:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On 13-03-07 02:25 PM, Andrew Morton wrote:
> On Thu, 7 Mar 2013 14:15:54 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
>> [v2: essentially unchanged since v1, so I've left the acked/reviewed
>>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
>>  and PRINTK=n, because the early_console struct and early_printk calls
>>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
>>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
>>  and still works for everyday sane configs too.]
>>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   
> 
> You did this:
> 

[...]

> _
> 
> Problem is, that won't fix the various compilation problems we've had. 
> See yesterday's lkml thread "linux-next: build failure after merge of
> the final tree (akpm tree related)"

Unless I'm missing something, the easy fix for that is to just
unconditionally have an early_console, i.e. this one line change
on top of the v2 patch:

diff --git a/kernel/printk.c b/kernel/printk.c
index 7664e49..86799bf 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -120,6 +120,7 @@ struct console_cmdline
 static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
 static int selected_console = -1;
 static int preferred_console = -1;
+struct console *early_console;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
@@ -1718,7 +1719,6 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
 #endif /* CONFIG_PRINTK */
 
 #ifdef CONFIG_EARLY_PRINTK
-struct console *early_console;
 
 void early_vprintk(const char *fmt, va_list ap)
 {


Then you don't have to spray any of those ifdefs into the sparc code.
I've build tested the above tweak for sparc32/64 defconfig.  Also built
for powerpc (sbc8548) -- note that you can't turn off EARLY_PRINTK
for ppc as it is:

config EARLY_PRINTK
        bool
        default y

i.e. no help text and no prompting. And the kooky randconfig and a
sane x86 defconfig are still OK too.  Unless you see something else
that I'm overlooking, I can send a v3 that incorporates the above
small tweak.

Thanks,
Paul.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 20:20   ` Paul Gortmaker
@ 2013-03-07 21:25     ` Thomas Gleixner
  2013-03-07 21:43       ` Andrew Morton
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Gleixner @ 2013-03-07 21:25 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Andrew Morton, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013, Paul Gortmaker wrote:
> On 13-03-07 02:25 PM, Andrew Morton wrote:
> > On Thu, 7 Mar 2013 14:15:54 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > 
> >> [v2: essentially unchanged since v1, so I've left the acked/reviewed
> >>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
> >>  and PRINTK=n, because the early_console struct and early_printk calls
> >>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
> >>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
> >>  and still works for everyday sane configs too.]
> >>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   
> > 
> > You did this:
> > 
> 
> [...]
> 
> > _
> > 
> > Problem is, that won't fix the various compilation problems we've had. 
> > See yesterday's lkml thread "linux-next: build failure after merge of
> > the final tree (akpm tree related)"
> 
> Unless I'm missing something, the easy fix for that is to just
> unconditionally have an early_console, i.e. this one line change
> on top of the v2 patch:
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 7664e49..86799bf 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -120,6 +120,7 @@ struct console_cmdline
>  static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
>  static int selected_console = -1;
>  static int preferred_console = -1;
> +struct console *early_console;
>  int console_set_on_cmdline;
>  EXPORT_SYMBOL(console_set_on_cmdline);
>  
> @@ -1718,7 +1719,6 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
>  #endif /* CONFIG_PRINTK */
>  
>  #ifdef CONFIG_EARLY_PRINTK
> -struct console *early_console;
>  
>  void early_vprintk(const char *fmt, va_list ap)
>  {
> 
> 
> Then you don't have to spray any of those ifdefs into the sparc code.

Yeah, that's what I thought as well. The extra pointer is not going to
create massive bloat :)

Btw, we should put that into the read_mostly section while at it.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 19:50   ` Paul Gortmaker
  2013-03-07 20:05     ` Joe Perches
@ 2013-03-07 21:35     ` Andrew Morton
  2013-03-07 21:41       ` Thomas Gleixner
                         ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Andrew Morton @ 2013-03-07 21:35 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013 14:50:23 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> This brings up a recurring question.  I was tempted to just go make
> CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
> pushback when trying to "fix" things like seeing ARM OMAP USB options
> for an x86 build[1], and GOLDFISH virt drivers being offered even
> when the end user already said no to GOLDFISH[2].
> 
> Do we want to use dependencies to reflect the real world layout of
> platforms/systems, or do we want to go the minimal dependency
> approach, where we are building sparc specific drivers on mips just
> because we can?
> 
> I think the former is better from a user specific point of view, as
> the maze of Kconfig is better as a tree topology with branches that
> have clear dependencies that exclude them, versus it being a flat
> monolithic space where anything can select anything.
> 
> Arguments I've heard for the latter seem to be developer centric
> (i.e forcing wider build coverage on the population as a whole, etc)

For me personally, I really really want good compilation coverage.  It
drives me bats when I merge a patch but have to jump through a series
of hoops (such as not having the appropriate cross-compiler!) to be
able to build the thing.

otoh, offering useless stuff to non-kernel-developers has downsides
with no balancing benefit, and we really should optimise things for
our users because there are so many more of them than there are of us.

I wish we could do both :(   CONFIG_AKPM?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 21:35     ` Andrew Morton
@ 2013-03-07 21:41       ` Thomas Gleixner
  2013-03-07 22:47       ` Rob Landley
  2013-03-08  0:49       ` Paul Gortmaker
  2 siblings, 0 replies; 19+ messages in thread
From: Thomas Gleixner @ 2013-03-07 21:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Paul Gortmaker, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013, Andrew Morton wrote:
> On Thu, 7 Mar 2013 14:50:23 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > This brings up a recurring question.  I was tempted to just go make
> > CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
> > pushback when trying to "fix" things like seeing ARM OMAP USB options
> > for an x86 build[1], and GOLDFISH virt drivers being offered even
> > when the end user already said no to GOLDFISH[2].
> > 
> > Do we want to use dependencies to reflect the real world layout of
> > platforms/systems, or do we want to go the minimal dependency
> > approach, where we are building sparc specific drivers on mips just
> > because we can?
> > 
> > I think the former is better from a user specific point of view, as
> > the maze of Kconfig is better as a tree topology with branches that
> > have clear dependencies that exclude them, versus it being a flat
> > monolithic space where anything can select anything.
> > 
> > Arguments I've heard for the latter seem to be developer centric
> > (i.e forcing wider build coverage on the population as a whole, etc)
> 
> For me personally, I really really want good compilation coverage.  It
> drives me bats when I merge a patch but have to jump through a series
> of hoops (such as not having the appropriate cross-compiler!) to be
> able to build the thing.
> 
> otoh, offering useless stuff to non-kernel-developers has downsides
> with no balancing benefit, and we really should optimise things for
> our users because there are so many more of them than there are of us.
> 
> I wish we could do both :(   CONFIG_AKPM?

Sure go ahead, and while at it could you please implement
CONFIG_DO_WHAT_I_MEAN as well ?

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 21:25     ` Thomas Gleixner
@ 2013-03-07 21:43       ` Andrew Morton
  2013-03-07 22:34         ` Thomas Gleixner
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2013-03-07 21:43 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Paul Gortmaker, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013 22:25:48 +0100 (CET) Thomas Gleixner <tglx@linutronix.de> wrote:

> > diff --git a/kernel/printk.c b/kernel/printk.c
> > index 7664e49..86799bf 100644
> > --- a/kernel/printk.c
> > +++ b/kernel/printk.c
> > @@ -120,6 +120,7 @@ struct console_cmdline
> >  static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
> >  static int selected_console = -1;
> >  static int preferred_console = -1;
> > +struct console *early_console;
> >  int console_set_on_cmdline;
> >  EXPORT_SYMBOL(console_set_on_cmdline);
> >  
> > @@ -1718,7 +1719,6 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
> >  #endif /* CONFIG_PRINTK */
> >  
> >  #ifdef CONFIG_EARLY_PRINTK
> > -struct console *early_console;
> >  
> >  void early_vprintk(const char *fmt, va_list ap)
> >  {
> > 
> > 
> > Then you don't have to spray any of those ifdefs into the sparc code.
> 
> Yeah, that's what I thought as well. The extra pointer is not going to
> create massive bloat :)

It's a bit hacky though.  If CONFIG_EARLY_PRINTK=n then that code just
shouldn't exist.  If we let it persist then maybe it's doing other
wrong stuff as well.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 21:43       ` Andrew Morton
@ 2013-03-07 22:34         ` Thomas Gleixner
  2013-03-08 16:11           ` [PATCH v3] " Paul Gortmaker
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Gleixner @ 2013-03-07 22:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Paul Gortmaker, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013, Andrew Morton wrote:

> On Thu, 7 Mar 2013 22:25:48 +0100 (CET) Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > > diff --git a/kernel/printk.c b/kernel/printk.c
> > > index 7664e49..86799bf 100644
> > > --- a/kernel/printk.c
> > > +++ b/kernel/printk.c
> > > @@ -120,6 +120,7 @@ struct console_cmdline
> > >  static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
> > >  static int selected_console = -1;
> > >  static int preferred_console = -1;
> > > +struct console *early_console;
> > >  int console_set_on_cmdline;
> > >  EXPORT_SYMBOL(console_set_on_cmdline);
> > >  
> > > @@ -1718,7 +1719,6 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
> > >  #endif /* CONFIG_PRINTK */
> > >  
> > >  #ifdef CONFIG_EARLY_PRINTK
> > > -struct console *early_console;
> > >  
> > >  void early_vprintk(const char *fmt, va_list ap)
> > >  {
> > > 
> > > 
> > > Then you don't have to spray any of those ifdefs into the sparc code.
> > 
> > Yeah, that's what I thought as well. The extra pointer is not going to
> > create massive bloat :)
> 
> It's a bit hacky though.  If CONFIG_EARLY_PRINTK=n then that code just
> shouldn't exist.  If we let it persist then maybe it's doing other
> wrong stuff as well.

Agreed.

Looking again it turns out that sparc does not fall into the early
console category. While it uses the early_printk naming convention it
does not use the early_vprintk stuff which I consolidated out of the
other architectures. So the sparc part of the patch simply wants to be
reverted.
 
Thanks,

	tglx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 21:35     ` Andrew Morton
  2013-03-07 21:41       ` Thomas Gleixner
@ 2013-03-07 22:47       ` Rob Landley
  2013-03-08  0:49       ` Paul Gortmaker
  2 siblings, 0 replies; 19+ messages in thread
From: Rob Landley @ 2013-03-07 22:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Paul Gortmaker, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Thomas Gleixner, Russell King, Michal Simek,
	Ralf Baechle, Benjamin Herrenschmidt, Paul Mundt,
	David S. Miller, Chris Metcalf, Richard Weinberger

On 03/07/2013 03:35:42 PM, Andrew Morton wrote:
> On Thu, 7 Mar 2013 14:50:23 -0500 Paul Gortmaker  
> <paul.gortmaker@windriver.com> wrote:
> 
> > This brings up a recurring question.  I was tempted to just go make
> > CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
> > pushback when trying to "fix" things like seeing ARM OMAP USB  
> options
> > for an x86 build[1], and GOLDFISH virt drivers being offered even
> > when the end user already said no to GOLDFISH[2].
> >
> > Do we want to use dependencies to reflect the real world layout of
> > platforms/systems, or do we want to go the minimal dependency
> > approach, where we are building sparc specific drivers on mips just
> > because we can?
> >
> > I think the former is better from a user specific point of view, as
> > the maze of Kconfig is better as a tree topology with branches that
> > have clear dependencies that exclude them, versus it being a flat
> > monolithic space where anything can select anything.
> >
> > Arguments I've heard for the latter seem to be developer centric
> > (i.e forcing wider build coverage on the population as a whole, etc)
> 
> For me personally, I really really want good compilation coverage.  It
> drives me bats when I merge a patch but have to jump through a series
> of hoops (such as not having the appropriate cross-compiler!) to be
> able to build the thing.

This doesn't solve everything, but:

http://landley.net/aboriginal/bin

cross-compiler-*.tar.bz2 is statically linked cross compilers for a lot  
of different architectures. Add the "bin" subdirectory to your path and  
CROSS_COMPILE=prefix- where prefix is whatever the * was (and the  
trailing - is important).

system-image-*.tar.bz2 is bootable system images with native  
development tools (./run-emulator.sh and ./dev-environment.sh are  
wrapper scripts to boot qemu with all the right arguments), for the  
same set of architectures. (There are a couple I have cross compilers  
for but insufficient board support in qemu as of yet. Working on it.  
I'm also working on adding more targets, but it's multitasked with  
several other projects...)

Note that if you have distccd installed on the host and the cross  
compiler is in your $PATH, ./dev-environment.sh should hook up distcc  
to the cross compiler through qemu's emulated network.

So at least _build_ coverage and basic "it booted to a shell prompt"  
smoke test coverage are reasonably straightforward. (I have automated  
build control images that build linux from scratch under the result,  
hence wanting distcc to make it slightly less slow.)

Driver testing for strange hardware continues to require strange  
hardware, though.

(If you're curious, http://landley.net/aboriginal/about.html goes on  
about it.)

Rob

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-07 21:35     ` Andrew Morton
  2013-03-07 21:41       ` Thomas Gleixner
  2013-03-07 22:47       ` Rob Landley
@ 2013-03-08  0:49       ` Paul Gortmaker
  2013-03-08  0:56         ` Andrew Morton
  2013-03-08  1:10         ` Steven Rostedt
  2 siblings, 2 replies; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-08  0:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, Mar 7, 2013 at 4:35 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 7 Mar 2013 14:50:23 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
>> This brings up a recurring question.  I was tempted to just go make
>> CONFIG_EARLY_PRINTK depend on CONFIG_PRINTK, but lately I've faced
>> pushback when trying to "fix" things like seeing ARM OMAP USB options
>> for an x86 build[1], and GOLDFISH virt drivers being offered even
>> when the end user already said no to GOLDFISH[2].
>>
>> Do we want to use dependencies to reflect the real world layout of
>> platforms/systems, or do we want to go the minimal dependency
>> approach, where we are building sparc specific drivers on mips just
>> because we can?
>>
>> I think the former is better from a user specific point of view, as
>> the maze of Kconfig is better as a tree topology with branches that
>> have clear dependencies that exclude them, versus it being a flat
>> monolithic space where anything can select anything.
>>
>> Arguments I've heard for the latter seem to be developer centric
>> (i.e forcing wider build coverage on the population as a whole, etc)
>
> For me personally, I really really want good compilation coverage.  It
> drives me bats when I merge a patch but have to jump through a series
> of hoops (such as not having the appropriate cross-compiler!) to be
> able to build the thing.

Interestingly enough, I discovered this has become automated to a
degree beyond any expectations I could have ever imagined.  To
the point where I really don't need any cross-compiler at all.

Recently I had some TIPC changes, destined for net-next, and I'd
pushed them to a personal branch on kernel.org called tipc-next, in
a paulg repo which I assumed nobody was looking at.

Within an hour, Fengguang's robots[1] found the branch, were compiling
it for fringe architectures, and running sparse on it, and sending me the
sparse regressions.  I'd listened to Fengguang's presentation while at
KS in San Diego, but I had no idea it was this proactive, until it did
autobot testing on my branch.

I have most of the prebuilt toolchains[2], and two line wrappers to set
the ARCH/CROSS_COMPILE, but as it stands, it seems I really don't
need those any more.  I can sanity test on a common arch and then
simply push to kernel.org to trigger build sanity across all arch.  I'll
probably still continue to use the toolchain prebuilts to test locally
though, just for the peace of mind.  But knowing FW bots are doing
testing before it goes into linux-next or anywhere else is really nice.

> otoh, offering useless stuff to non-kernel-developers has downsides
> with no balancing benefit, and we really should optimise things for
> our users because there are so many more of them than there are of us.

Glad to hear that, and I agree totally.  I hope the above three lines
will persuade people to merge practical/sane dependency lines that
have the end users in mind, instead of focusing on ease of local testing.

Thanks,
Paul.
---

[1] http://lwn.net/Articles/514278/
[2] ftp://ftp.kernel.org/pub/tools/crosstool/files/bin/x86_64/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-08  0:49       ` Paul Gortmaker
@ 2013-03-08  0:56         ` Andrew Morton
  2013-03-08  1:15           ` Paul Gortmaker
  2013-03-08  1:10         ` Steven Rostedt
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Morton @ 2013-03-08  0:56 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, 7 Mar 2013 19:49:35 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> > otoh, offering useless stuff to non-kernel-developers has downsides
> > with no balancing benefit, and we really should optimise things for
> > our users because there are so many more of them than there are of us.
> 
> Glad to hear that, and I agree totally.  I hope the above three lines
> will persuade people to merge practical/sane dependency lines that
> have the end users in mind, instead of focusing on ease of local testing.

It is possible to just ignore the Kconfig system and type "make
drivers/foo/bar.o".  Sometimes this actually works.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-08  0:49       ` Paul Gortmaker
  2013-03-08  0:56         ` Andrew Morton
@ 2013-03-08  1:10         ` Steven Rostedt
  2013-03-08 15:29           ` Guenter Roeck
  1 sibling, 1 reply; 19+ messages in thread
From: Steven Rostedt @ 2013-03-08  1:10 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Andrew Morton, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Thomas Gleixner, Russell King, Michal Simek,
	Ralf Baechle, Benjamin Herrenschmidt, Paul Mundt,
	David S. Miller, Chris Metcalf, Richard Weinberger

On Thu, Mar 07, 2013 at 07:49:35PM -0500, Paul Gortmaker wrote:
> 
> Within an hour, Fengguang's robots[1] found the branch, were compiling
> it for fringe architectures, and running sparse on it, and sending me the
> sparse regressions.  I'd listened to Fengguang's presentation while at
> KS in San Diego, but I had no idea it was this proactive, until it did
> autobot testing on my branch.

And people wonder why I wanted to give Fengguang a round of applauds
there. I guess I was one of the first people to get his testing, as it
found a lot of little things for me that my own tests missed.

> 
> I have most of the prebuilt toolchains[2], and two line wrappers to set
> the ARCH/CROSS_COMPILE, but as it stands, it seems I really don't
> need those any more.  I can sanity test on a common arch and then
> simply push to kernel.org to trigger build sanity across all arch.  I'll
> probably still continue to use the toolchain prebuilts to test locally
> though, just for the peace of mind.  But knowing FW bots are doing
> testing before it goes into linux-next or anywhere else is really nice.
> 

I should stress that there's a sample ktest.pl file for everyone:

 tools/testing/ktest/examples/crosstests.conf

Just download the cross compilers from:

 (assuming you're running on an x86_64 box)

  https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/

Install them somewhere like:

  /usr/local

ie.

  /usr/local/gcc-4.6.3-nolibc/mips-linux/bin/mips-linux-gcc

Have a pristine checkout, and a build directory to use:

 git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git linux.git
 mkdir cross-compile

Then just run:

  ktest.pl crosstests.conf

It will go arch by arch cross compiling each with the defconfig. Feel
free to modify that config. But really, there should be no more excuses
for kernel developers not doing cross compiles of most archs. It's
really that simple.

-- Steve


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-08  0:56         ` Andrew Morton
@ 2013-03-08  1:15           ` Paul Gortmaker
  0 siblings, 0 replies; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-08  1:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Thu, Mar 7, 2013 at 7:56 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 7 Mar 2013 19:49:35 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
>> > otoh, offering useless stuff to non-kernel-developers has downsides
>> > with no balancing benefit, and we really should optimise things for
>> > our users because there are so many more of them than there are of us.
>>
>> Glad to hear that, and I agree totally.  I hope the above three lines
>> will persuade people to merge practical/sane dependency lines that
>> have the end users in mind, instead of focusing on ease of local testing.
>
> It is possible to just ignore the Kconfig system and type "make
> drivers/foo/bar.o".  Sometimes this actually works.

Yes, I've used that many times too[1] -- it however fails on things like
sched which has C files including C files.  Dave Miller had posted a tip
to people a while back on how to ferret out basic compile failures without
having _any_ cross toolchains by at least doing the basic CPP part, but I
can't find the archived thread in my  rudimentary searches at the moment...

In any case, it sounds like all are in agreement that we should not
complicate the end user main Kconfig use case just to offload a trivial
component of the automated build testing burden onto general users.

[1] https://git.kernel.org/cgit/linux/kernel/git/paulg/longterm-queue-2.6.34.git/tree/scripts/buildcommits

Paul.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] early_printk: consolidate random copies of identical code
  2013-03-08  1:10         ` Steven Rostedt
@ 2013-03-08 15:29           ` Guenter Roeck
  0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2013-03-08 15:29 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Paul Gortmaker, Andrew Morton, Mike Frysinger, Ingo Molnar,
	Randy Dunlap, linux-kernel, Thomas Gleixner, Russell King

On Thu, Mar 07, 2013 at 08:10:43PM -0500, Steven Rostedt wrote:
> On Thu, Mar 07, 2013 at 07:49:35PM -0500, Paul Gortmaker wrote:
> > 
> > Within an hour, Fengguang's robots[1] found the branch, were compiling
> > it for fringe architectures, and running sparse on it, and sending me the
> > sparse regressions.  I'd listened to Fengguang's presentation while at
> > KS in San Diego, but I had no idea it was this proactive, until it did
> > autobot testing on my branch.
> 
> And people wonder why I wanted to give Fengguang a round of applauds
> there. I guess I was one of the first people to get his testing, as it
> found a lot of little things for me that my own tests missed.
> 
> > 
> > I have most of the prebuilt toolchains[2], and two line wrappers to set
> > the ARCH/CROSS_COMPILE, but as it stands, it seems I really don't
> > need those any more.  I can sanity test on a common arch and then
> > simply push to kernel.org to trigger build sanity across all arch.  I'll
> > probably still continue to use the toolchain prebuilts to test locally
> > though, just for the peace of mind.  But knowing FW bots are doing
> > testing before it goes into linux-next or anywhere else is really nice.
> > 
> 
> I should stress that there's a sample ktest.pl file for everyone:
> 
>  tools/testing/ktest/examples/crosstests.conf
> 
> Just download the cross compilers from:
> 
>  (assuming you're running on an x86_64 box)
> 
>   https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/
> 
Have you tried that link recently ?

I get:

Forbidden

You don't have permission to access /pub/tools/crosstool/files/bin/x86_64/4.6.3/
on this server.

Guenter

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3] early_printk: consolidate random copies of identical code
  2013-03-07 22:34         ` Thomas Gleixner
@ 2013-03-08 16:11           ` Paul Gortmaker
  2013-03-08 21:46             ` Andrew Morton
  2013-03-23 21:23             ` Geert Uytterhoeven
  0 siblings, 2 replies; 19+ messages in thread
From: Paul Gortmaker @ 2013-03-08 16:11 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger, Paul Gortmaker

From: Thomas Gleixner <tglx@linutronix.de>

The early console implementations are the same all over the place.  Move
the print function to kernel/printk and get rid of the copies.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Richard Weinberger <richard@nod.at>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---

[v3: drop sparc bits as suggested by tglx, redo build tests on sparc
 sparc32, Randy's randconfig, ppc, mips, arm...]

[v2: essentially unchanged since v1, so I've left the acked/reviewed
 tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
 and PRINTK=n, because the early_console struct and early_printk calls
 were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
 exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
 and still works for everyday sane configs too.]
 [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   


 arch/arm/kernel/early_printk.c        | 17 +++--------------
 arch/blackfin/kernel/early_printk.c   |  2 --
 arch/microblaze/kernel/early_printk.c | 26 ++++----------------------
 arch/mips/kernel/early_printk.c       | 11 +++++------
 arch/powerpc/kernel/udbg.c            |  6 ++----
 arch/sh/kernel/sh_bios.c              |  2 --
 arch/tile/kernel/early_printk.c       | 27 +++++----------------------
 arch/um/kernel/early_printk.c         |  8 +++++---
 arch/unicore32/kernel/early_printk.c  | 12 ++++--------
 arch/x86/kernel/early_printk.c        | 21 ++-------------------
 include/linux/console.h               |  1 +
 include/linux/printk.h                |  6 ++++++
 kernel/printk.c                       | 30 +++++++++++++++++++++++-------
 13 files changed, 60 insertions(+), 109 deletions(-)

diff --git a/arch/arm/kernel/early_printk.c b/arch/arm/kernel/early_printk.c
index 85aa2b2..4307653 100644
--- a/arch/arm/kernel/early_printk.c
+++ b/arch/arm/kernel/early_printk.c
@@ -29,28 +29,17 @@ static void early_console_write(struct console *con, const char *s, unsigned n)
 	early_write(s, n);
 }
 
-static struct console early_console = {
+static struct console early_console_dev = {
 	.name =		"earlycon",
 	.write =	early_console_write,
 	.flags =	CON_PRINTBUFFER | CON_BOOT,
 	.index =	-1,
 };
 
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	va_start(ap, fmt);
-	n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_write(buf, n);
-	va_end(ap);
-}
-
 static int __init setup_early_printk(char *buf)
 {
-	register_console(&early_console);
+	early_console = &early_console_dev;
+	register_console(&early_console_dev);
 	return 0;
 }
 
diff --git a/arch/blackfin/kernel/early_printk.c b/arch/blackfin/kernel/early_printk.c
index 84ed837..61fbd2d 100644
--- a/arch/blackfin/kernel/early_printk.c
+++ b/arch/blackfin/kernel/early_printk.c
@@ -25,8 +25,6 @@ extern struct console *bfin_earlyserial_init(unsigned int port,
 extern struct console *bfin_jc_early_init(void);
 #endif
 
-static struct console *early_console;
-
 /* Default console */
 #define DEFAULT_PORT 0
 #define DEFAULT_CFLAG CS8|B57600
diff --git a/arch/microblaze/kernel/early_printk.c b/arch/microblaze/kernel/early_printk.c
index 60dcacc..365f2d5 100644
--- a/arch/microblaze/kernel/early_printk.c
+++ b/arch/microblaze/kernel/early_printk.c
@@ -21,7 +21,6 @@
 #include <asm/setup.h>
 #include <asm/prom.h>
 
-static u32 early_console_initialized;
 static u32 base_addr;
 
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
@@ -109,27 +108,11 @@ static struct console early_serial_uart16550_console = {
 };
 #endif /* CONFIG_SERIAL_8250_CONSOLE */
 
-static struct console *early_console;
-
-void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	if (early_console_initialized) {
-		va_start(ap, fmt);
-		n = vscnprintf(buf, 512, fmt, ap);
-		early_console->write(early_console, buf, n);
-		va_end(ap);
-	}
-}
-
 int __init setup_early_printk(char *opt)
 {
 	int version = 0;
 
-	if (early_console_initialized)
+	if (early_console)
 		return 1;
 
 	base_addr = of_early_console(&version);
@@ -159,7 +142,6 @@ int __init setup_early_printk(char *opt)
 		}
 
 		register_console(early_console);
-		early_console_initialized = 1;
 		return 0;
 	}
 	return 1;
@@ -169,7 +151,7 @@ int __init setup_early_printk(char *opt)
  * only for early console because of performance degression */
 void __init remap_early_printk(void)
 {
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	pr_info("early_printk_console remapping from 0x%x to ", base_addr);
 	base_addr = (u32) ioremap(base_addr, PAGE_SIZE);
@@ -194,9 +176,9 @@ void __init remap_early_printk(void)
 
 void __init disable_early_printk(void)
 {
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	pr_warn("disabling early console\n");
 	unregister_console(early_console);
-	early_console_initialized = 0;
+	early_console = NULL;
 }
diff --git a/arch/mips/kernel/early_printk.c b/arch/mips/kernel/early_printk.c
index 9e6440e..21150cd 100644
--- a/arch/mips/kernel/early_printk.c
+++ b/arch/mips/kernel/early_printk.c
@@ -8,6 +8,7 @@
  *   written by Ralf Baechle (ralf@linux-mips.org)
  */
 #include <linux/console.h>
+#include <linux/printk.h>
 #include <linux/init.h>
 
 #include <asm/setup.h>
@@ -24,20 +25,18 @@ static void early_console_write(struct console *con, const char *s, unsigned n)
 	}
 }
 
-static struct console early_console = {
+static struct console early_console_prom = {
 	.name	= "early",
 	.write	= early_console_write,
 	.flags	= CON_PRINTBUFFER | CON_BOOT,
 	.index	= -1
 };
 
-static int early_console_initialized __initdata;
-
 void __init setup_early_printk(void)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return;
-	early_console_initialized = 1;
+	early_console = &early_console_prom;
 
-	register_console(&early_console);
+	register_console(&early_console_prom);
 }
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index f974849..13b8670 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -156,15 +156,13 @@ static struct console udbg_console = {
 	.index	= 0,
 };
 
-static int early_console_initialized;
-
 /*
  * Called by setup_system after ppc_md->probe and ppc_md->early_init.
  * Call it again after setting udbg_putc in ppc_md->setup_arch.
  */
 void __init register_early_udbg_console(void)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return;
 
 	if (!udbg_putc)
@@ -174,7 +172,7 @@ void __init register_early_udbg_console(void)
 		printk(KERN_INFO "early console immortal !\n");
 		udbg_console.flags &= ~CON_BOOT;
 	}
-	early_console_initialized = 1;
+	early_console = &udbg_console;
 	register_console(&udbg_console);
 }
 
diff --git a/arch/sh/kernel/sh_bios.c b/arch/sh/kernel/sh_bios.c
index 47475cc..a5b51b9 100644
--- a/arch/sh/kernel/sh_bios.c
+++ b/arch/sh/kernel/sh_bios.c
@@ -144,8 +144,6 @@ static struct console bios_console = {
 	.index		= -1,
 };
 
-static struct console *early_console;
-
 static int __init setup_early_printk(char *buf)
 {
 	int keep_early = 0;
diff --git a/arch/tile/kernel/early_printk.c b/arch/tile/kernel/early_printk.c
index afb9c9a..34d72a1 100644
--- a/arch/tile/kernel/early_printk.c
+++ b/arch/tile/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/irqflags.h>
+#include <linux/printk.h>
 #include <asm/setup.h>
 #include <hv/hypervisor.h>
 
@@ -33,25 +34,8 @@ static struct console early_hv_console = {
 };
 
 /* Direct interface for emergencies */
-static struct console *early_console = &early_hv_console;
-static int early_console_initialized;
 static int early_console_complete;
 
-static void early_vprintk(const char *fmt, va_list ap)
-{
-	char buf[512];
-	int n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_console->write(early_console, buf, n);
-}
-
-void early_printk(const char *fmt, ...)
-{
-	va_list ap;
-	va_start(ap, fmt);
-	early_vprintk(fmt, ap);
-	va_end(ap);
-}
-
 void early_panic(const char *fmt, ...)
 {
 	va_list ap;
@@ -69,14 +53,13 @@ static int __initdata keep_early;
 
 static int __init setup_early_printk(char *str)
 {
-	if (early_console_initialized)
+	if (early_console)
 		return 1;
 
 	if (str != NULL && strncmp(str, "keep", 4) == 0)
 		keep_early = 1;
 
 	early_console = &early_hv_console;
-	early_console_initialized = 1;
 	register_console(early_console);
 
 	return 0;
@@ -85,12 +68,12 @@ static int __init setup_early_printk(char *str)
 void __init disable_early_printk(void)
 {
 	early_console_complete = 1;
-	if (!early_console_initialized || !early_console)
+	if (!early_console)
 		return;
 	if (!keep_early) {
 		early_printk("disabling early console\n");
 		unregister_console(early_console);
-		early_console_initialized = 0;
+		early_console = NULL;
 	} else {
 		early_printk("keeping early console\n");
 	}
@@ -98,7 +81,7 @@ void __init disable_early_printk(void)
 
 void warn_early_printk(void)
 {
-	if (early_console_complete || early_console_initialized)
+	if (early_console_complete || early_console)
 		return;
 	early_printk("\
 Machine shutting down before console output is fully initialized.\n\
diff --git a/arch/um/kernel/early_printk.c b/arch/um/kernel/early_printk.c
index 49480f0..4a0800b 100644
--- a/arch/um/kernel/early_printk.c
+++ b/arch/um/kernel/early_printk.c
@@ -16,7 +16,7 @@ static void early_console_write(struct console *con, const char *s, unsigned int
 	um_early_printk(s, n);
 }
 
-static struct console early_console = {
+static struct console early_console_dev = {
 	.name = "earlycon",
 	.write = early_console_write,
 	.flags = CON_BOOT,
@@ -25,8 +25,10 @@ static struct console early_console = {
 
 static int __init setup_early_printk(char *buf)
 {
-	register_console(&early_console);
-
+	if (!early_console) {
+		early_console = &early_console_dev;
+		register_console(&early_console_dev);
+	}
 	return 0;
 }
 
diff --git a/arch/unicore32/kernel/early_printk.c b/arch/unicore32/kernel/early_printk.c
index 3922255..9be0d5d 100644
--- a/arch/unicore32/kernel/early_printk.c
+++ b/arch/unicore32/kernel/early_printk.c
@@ -33,21 +33,17 @@ static struct console early_ocd_console = {
 	.index =	-1,
 };
 
-/* Direct interface for emergencies */
-static struct console *early_console = &early_ocd_console;
-
-static int __initdata keep_early;
-
 static int __init setup_early_printk(char *buf)
 {
-	if (!buf)
+	int keep_early;
+
+	if (!buf || early_console)
 		return 0;
 
 	if (strstr(buf, "keep"))
 		keep_early = 1;
 
-	if (!strncmp(buf, "ocd", 3))
-		early_console = &early_ocd_console;
+	early_console = &early_ocd_console;
 
 	if (keep_early)
 		early_console->flags &= ~CON_BOOT;
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 9b9f18b..d15f575 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -169,25 +169,9 @@ static struct console early_serial_console = {
 	.index =	-1,
 };
 
-/* Direct interface for emergencies */
-static struct console *early_console = &early_vga_console;
-static int __initdata early_console_initialized;
-
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	char buf[512];
-	int n;
-	va_list ap;
-
-	va_start(ap, fmt);
-	n = vscnprintf(buf, sizeof(buf), fmt, ap);
-	early_console->write(early_console, buf, n);
-	va_end(ap);
-}
-
 static inline void early_console_register(struct console *con, int keep_early)
 {
-	if (early_console->index != -1) {
+	if (con->index != -1) {
 		printk(KERN_CRIT "ERROR: earlyprintk= %s already used\n",
 		       con->name);
 		return;
@@ -207,9 +191,8 @@ static int __init setup_early_printk(char *buf)
 	if (!buf)
 		return 0;
 
-	if (early_console_initialized)
+	if (early_console)
 		return 0;
-	early_console_initialized = 1;
 
 	keep = (strstr(buf, "keep") != NULL);
 
diff --git a/include/linux/console.h b/include/linux/console.h
index 29680a8..73bab0f 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -141,6 +141,7 @@ struct console {
 	for (con = console_drivers; con != NULL; con = con->next)
 
 extern int console_set_on_cmdline;
+extern struct console *early_console;
 
 extern int add_preferred_console(char *name, int idx, char *options);
 extern int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options);
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1249a54..b846afd 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -95,8 +95,14 @@ int no_printk(const char *fmt, ...)
 	return 0;
 }
 
+#ifdef CONFIG_EARLY_PRINTK
 extern asmlinkage __printf(1, 2)
 void early_printk(const char *fmt, ...);
+void early_vprintk(const char *fmt, va_list ap);
+#else
+static inline __printf(1, 2) __cold
+void early_printk(const char *s, ...) { }
+#endif
 
 #ifdef CONFIG_PRINTK
 asmlinkage __printf(5, 0)
diff --git a/kernel/printk.c b/kernel/printk.c
index 0b31715..7664e49 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -49,13 +49,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/printk.h>
 
-/*
- * Architectures can override it:
- */
-void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
-{
-}
-
 /* printk's without a loglevel use this.. */
 #define DEFAULT_MESSAGE_LOGLEVEL CONFIG_DEFAULT_MESSAGE_LOGLEVEL
 
@@ -1724,6 +1717,29 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
 
 #endif /* CONFIG_PRINTK */
 
+#ifdef CONFIG_EARLY_PRINTK
+struct console *early_console;
+
+void early_vprintk(const char *fmt, va_list ap)
+{
+	if (early_console) {
+		char buf[512];
+		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
+
+		early_console->write(early_console, buf, n);
+	}
+}
+
+asmlinkage void early_printk(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	early_vprintk(fmt, ap);
+	va_end(ap);
+}
+#endif
+
 static int __add_preferred_console(char *name, int idx, char *options,
 				   char *brl_options)
 {
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] early_printk: consolidate random copies of identical code
  2013-03-08 16:11           ` [PATCH v3] " Paul Gortmaker
@ 2013-03-08 21:46             ` Andrew Morton
  2013-03-23 21:23             ` Geert Uytterhoeven
  1 sibling, 0 replies; 19+ messages in thread
From: Andrew Morton @ 2013-03-08 21:46 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Mike Frysinger, Ingo Molnar, Randy Dunlap, linux-kernel,
	Thomas Gleixner, Russell King, Michal Simek, Ralf Baechle,
	Benjamin Herrenschmidt, Paul Mundt, David S. Miller,
	Chris Metcalf, Richard Weinberger

On Fri, 8 Mar 2013 11:11:20 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> The early console implementations are the same all over the place.  Move
> the print function to kernel/printk and get rid of the copies.
> 
> ...
> 
> [v3: drop sparc bits as suggested by tglx, redo build tests on sparc
>  sparc32, Randy's randconfig, ppc, mips, arm...]
> 
> [v2: essentially unchanged since v1, so I've left the acked/reviewed
>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
>  and PRINTK=n, because the early_console struct and early_printk calls
>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
>  and still works for everyday sane configs too.]
>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2   

I queued the below delta.

I suspect arch/powerpc/kernel/udbg.c is still broken with
CONFIG_EARLY_PRINTK=n?  My powerpc crosscompiler broke and I need to
buy a new one :(

 arch/sparc/kernel/setup_32.c |    1 
 arch/sparc/kernel/setup_64.c |    8 -----
 kernel/printk.c              |   46 ++++++++++++++++-----------------
 3 files changed, 24 insertions(+), 31 deletions(-)

diff -puN arch/sparc/kernel/setup_32.c~early_printk-consolidate-random-copies-of-identical-code-v3 arch/sparc/kernel/setup_32.c
--- a/arch/sparc/kernel/setup_32.c~early_printk-consolidate-random-copies-of-identical-code-v3
+++ a/arch/sparc/kernel/setup_32.c
@@ -309,7 +309,6 @@ void __init setup_arch(char **cmdline_p)
 
 	boot_flags_init(*cmdline_p);
 
-	early_console = &prom_early_console;
 	register_console(&prom_early_console);
 
 	printk("ARCH: ");
diff -puN arch/sparc/kernel/setup_64.c~early_printk-consolidate-random-copies-of-identical-code-v3 arch/sparc/kernel/setup_64.c
--- a/arch/sparc/kernel/setup_64.c~early_printk-consolidate-random-copies-of-identical-code-v3
+++ a/arch/sparc/kernel/setup_64.c
@@ -551,12 +551,6 @@ static void __init init_sparc64_elf_hwca
 		pause_patch();
 }
 
-static inline void register_prom_console(void)
-{
-	early_console = &prom_early_console;
-	register_console(&prom_early_console);
-}
-
 void __init setup_arch(char **cmdline_p)
 {
 	/* Initialize PROM console and command line. */
@@ -568,7 +562,7 @@ void __init setup_arch(char **cmdline_p)
 #ifdef CONFIG_EARLYFB
 	if (btext_find_display())
 #endif
-		register_prom_console();
+		register_console(&prom_early_console);
 
 	if (tlb_type == hypervisor)
 		printk("ARCH: SUN4V\n");
diff -puN kernel/printk.c~early_printk-consolidate-random-copies-of-identical-code-v3 kernel/printk.c
--- a/kernel/printk.c~early_printk-consolidate-random-copies-of-identical-code-v3
+++ a/kernel/printk.c
@@ -759,29 +759,6 @@ module_param(ignore_loglevel, bool, S_IR
 MODULE_PARM_DESC(ignore_loglevel, "ignore loglevel setting, to"
 	"print all kernel messages to the console.");
 
-#ifdef CONFIG_EARLY_PRINTK
-struct console *early_console;
-
-void early_vprintk(const char *fmt, va_list ap)
-{
-	if (early_console) {
-		char buf[512];
-		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
-
-		early_console->write(early_console, buf, n);
-	}
-}
-
-asmlinkage void early_printk(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	early_vprintk(fmt, ap);
-	va_end(ap);
-}
-#endif
-
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 
 static int boot_delay; /* msecs delay after each printk during bootup */
@@ -1743,6 +1720,29 @@ static size_t cont_print_text(char *text
 
 #endif /* CONFIG_PRINTK */
 
+#ifdef CONFIG_EARLY_PRINTK
+struct console *early_console;
+
+void early_vprintk(const char *fmt, va_list ap)
+{
+	if (early_console) {
+		char buf[512];
+		int n = vscnprintf(buf, sizeof(buf), fmt, ap);
+
+		early_console->write(early_console, buf, n);
+	}
+}
+
+asmlinkage void early_printk(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	early_vprintk(fmt, ap);
+	va_end(ap);
+}
+#endif
+
 static int __add_preferred_console(char *name, int idx, char *options,
 				   char *brl_options)
 {
_


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3] early_printk: consolidate random copies of identical code
  2013-03-08 16:11           ` [PATCH v3] " Paul Gortmaker
  2013-03-08 21:46             ` Andrew Morton
@ 2013-03-23 21:23             ` Geert Uytterhoeven
  1 sibling, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2013-03-23 21:23 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Andrew Morton, Mike Frysinger, Ingo Molnar, Randy Dunlap,
	linux-kernel, Thomas Gleixner, Russell King, Michal Simek,
	Ralf Baechle, Benjamin Herrenschmidt, Paul Mundt,
	David S. Miller, Chris Metcalf, Richard Weinberger

On Fri, Mar 8, 2013 at 5:11 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The early console implementations are the same all over the place.  Move
> the print function to kernel/printk and get rid of the copies.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Russell King <linux@arm.linux.org.uk>
> Acked-by: Mike Frysinger <vapier@gentoo.org>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Richard Weinberger <richard@nod.at>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>
> [v3: drop sparc bits as suggested by tglx, redo build tests on sparc
>  sparc32, Randy's randconfig, ppc, mips, arm...]
>
> [v2: essentially unchanged since v1, so I've left the acked/reviewed
>  tags.  There was a compile fail[1] for a randconfig with EARLY_PRINTK=y
>  and PRINTK=n, because the early_console struct and early_printk calls
>  were nested within an #ifdef CONFIG_PRINTK -- moving that whole block
>  exactly as-is to be outside the #ifdef CONFIG_PRINTK fixes the randconfig
>  and still works for everyday sane configs too.]
>  [1] http://marc.info/?l=linux-next&m=136219350914998&w=2

> --- a/arch/sh/kernel/sh_bios.c
> +++ b/arch/sh/kernel/sh_bios.c
> @@ -144,8 +144,6 @@ static struct console bios_console = {
>         .index          = -1,
>  };
>
> -static struct console *early_console;

This one wasn't protected by #ifdef CONFIG_EARLY_PRINTK...

> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -1724,6 +1717,29 @@ static size_t cont_print_text(char *text, size_t size) { return 0; }
>
>  #endif /* CONFIG_PRINTK */
>
> +#ifdef CONFIG_EARLY_PRINTK
> +struct console *early_console;

... but this one is, causing

arch/sh/kernel/built-in.o: In function `setup_early_printk':
sh_bios.c:(.init.text+0xc80): undefined reference to `early_console'

Cfr. e.g. http://kisskb.ellerman.id.au/kisskb/buildresult/8433720/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-03-23 21:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-07 19:15 [PATCH v2] early_printk: consolidate random copies of identical code Paul Gortmaker
2013-03-07 19:25 ` Andrew Morton
2013-03-07 19:50   ` Paul Gortmaker
2013-03-07 20:05     ` Joe Perches
2013-03-07 21:35     ` Andrew Morton
2013-03-07 21:41       ` Thomas Gleixner
2013-03-07 22:47       ` Rob Landley
2013-03-08  0:49       ` Paul Gortmaker
2013-03-08  0:56         ` Andrew Morton
2013-03-08  1:15           ` Paul Gortmaker
2013-03-08  1:10         ` Steven Rostedt
2013-03-08 15:29           ` Guenter Roeck
2013-03-07 20:20   ` Paul Gortmaker
2013-03-07 21:25     ` Thomas Gleixner
2013-03-07 21:43       ` Andrew Morton
2013-03-07 22:34         ` Thomas Gleixner
2013-03-08 16:11           ` [PATCH v3] " Paul Gortmaker
2013-03-08 21:46             ` Andrew Morton
2013-03-23 21:23             ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).