Skip to contentSkip to conversation

Provide console_device()

PATCH In review Russell King 3 participants 6 messages Jun 14, 2004 – Jun 30, 2004

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

PATCH 1/3 Provide console_device()

[This patch series has also been separately sent to the architecture
maintainers]

Add console_device() to return the console tty driver structure and the index. Acquire the console lock while scanning the list of console drivers to protect us against console driver list manipulations.

drivers/char/tty_io.c +2 -7
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/drivers/char/tty_io.c linux/drivers/char/tty_io.c
@@ -1344,13 +1344,8 @@ retry_open:
 	}
 #endif
 	if (device == MKDEV(TTYAUX_MAJOR,1)) {
-		struct console *c = console_drivers;
-		for (c = console_drivers; c; c = c->next) {
-			if (!c->device)
-				continue;
-			driver = c->device(c, &index);
-			if (!driver)
-				continue;
+		driver = console_device(&index);
+		if (driver) {
 			/* Don't let /dev/console block */
 			filp->f_flags |= O_NONBLOCK;
 			noctty = 1;
kernel/printk.c +17 -0
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/kernel/printk.c linux/kernel/printk.c
@@ -683,6 +683,23 @@ void console_unblank(void)
 }
 EXPORT_SYMBOL(console_unblank);
 
+struct tty_driver *console_device(int *index)
+{
+	struct console *c;
+	struct tty_driver *driver = NULL;
+
+	acquire_console_sem();
+	for (c = console_drivers; c != NULL; c = c->next) {
+		if (!c->device)
+			continue;
+		driver = c->device(c, index);
+		if (driver)
+			break;
+	}
+	release_console_sem();
+	return driver;
+}
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
include/linux/console.h +1 -0
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/include/linux/console.h linux/include/linux/console.h
@@ -104,6 +104,7 @@ extern void acquire_console_sem(void);
 extern void release_console_sem(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
+extern struct tty_driver *console_device(int *);
 extern int is_console_locked(void);
 
 /* Some debug stub to catch some of the obvious races in the VT code */
Signature · 4 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core
#1 lore raw

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

Add console_suspend() and console_resume() methods so the serial drivers can disable console output before suspending a port, and re-enable output afterwards.

We also add locking to ensure that we synchronise with any in-progress printk.

drivers/serial/serial_core.c +2 -2
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/drivers/serial/serial_core.c linux/drivers/serial/serial_core.c
@@ -1923,7 +1923,7 @@ int uart_suspend_port(struct uart_driver
 	 * Disable the console device before suspending.
 	 */
 	if (uart_console(port))
-		port->cons->flags &= ~CON_ENABLED;
+		console_suspend(port->cons);
 
 	uart_change_pm(state, 3);
 
@@ -1945,7 +1945,7 @@ int uart_resume_port(struct uart_driver 
 	 */
 	if (uart_console(port)) {
 		uart_change_speed(state, NULL);
-		port->cons->flags |= CON_ENABLED;
+		console_resume(port->cons);
 	}
 
 	if (state->info && state->info->flags & UIF_INITIALIZED) {
kernel/printk.c +16 -0
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/kernel/printk.c linux/kernel/printk.c
@@ -700,6 +700,22 @@ struct tty_driver *console_device(int *i
 	return driver;
 }
 
+void console_suspend(struct console *console)
+{
+	acquire_console_sem();
+	console->flags &= ~CON_ENABLED;
+	release_console_sem();
+}
+EXPORT_SYMBOL(console_suspend);
+
+void console_resume(struct console *console)
+{
+	acquire_console_sem();
+	console->flags |= CON_ENABLED;
+	release_console_sem();
+}
+EXPORT_SYMBOL(console_resume);
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
include/linux/console.h +2 -0
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/include/linux/console.h linux/include/linux/console.h
@@ -105,6 +105,8 @@ extern void release_console_sem(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
 extern struct tty_driver *console_device(int *);
+extern void console_suspend(struct console *);
+extern void console_resume(struct console *);
 extern int is_console_locked(void);
 
 /* Some debug stub to catch some of the obvious races in the VT code */
Signature · 4 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core
#2 lore raw

Re: [RFC][PATCH 3/3] Convert console list to list_head

Convert console drivers to use list_head rather than its own private list implementation. Unfortunately, several other places reference "console_drivers" directly, all of them without locking. PPC/PPC64 even seems to export the list for no apparant reason.

arch/ia64/hp/sim/simserial.c:extern struct console *console_drivers; /* from kernel/printk.c */
arch/ia64/hp/sim/simserial.c: console = console_drivers;

This is interesting - it appears to search the console driver list for the first console, hold a reference to it, and call its write method. If a console were to be unregistered via unregister_console() this would potentially oops.

arch/parisc/kernel/pdc_cons.c:  while ((console = console_drivers) != NULL)
arch/parisc/kernel/pdc_cons.c:          unregister_console(console_drivers);
arch/parisc/kernel/traps.c:     if (!console_drivers)
arch/parisc/kernel/traps.c:     if (!console_drivers)

PARISC seems to like to unregister all consoles, and then re- registers its own console driver. IOW, it assumes it can safely override the state of the console subsystem independent of whatever drivers may be registered.

arch/ppc/kernel/ppc_ksyms.c:EXPORT_SYMBOL(console_drivers); arch/ppc64/kernel/ppc_ksyms.c:EXPORT_SYMBOL(console_drivers);

Random exports of this kernel symbol...

arch/sparc64/kernel/setup.c:    cons = console_drivers;
arch/sparc64/kernel/setup.c:            cons = console_drivers;

Sparc64 appears to want to temporarily override the console subsystem for debugging purposes. Maybe there's a better way?

Since "console_drivers" vanishes as a result of this, all the above break, so this needs further work before it can be merged into 2.6. Can people consider the above and come up with both a cleaner and saner solution?

kernel/printk.c renamed from kernel/printk.c.console +14 -29
@@ -61,7 +61,8 @@
  * driver system.
  */
 static DECLARE_MUTEX(console_sem);
-struct console *console_drivers;
+static LIST_HEAD(console_driver_list);
+
 /*
  * This is used for debugging the mess that is the VT code by
  * keeping track if we have the console semaphore held. It's
@@ -383,7 +384,7 @@
 {
 	struct console *con;
 
-	for (con = console_drivers; con; con = con->next) {
+	list_for_each_entry(con, &console_driver_list, list) {
 		if ((con->flags & CON_ENABLED) && con->write)
 			con->write(con, &LOG_BUF(start), end - start);
 	}
@@ -396,7 +397,7 @@
 				unsigned long end, int msg_log_level)
 {
 	if (msg_log_level < console_loglevel &&
-			console_drivers && start != end) {
+	    !list_empty(&console_driver_list) && start != end) {
 		if ((start & LOG_BUF_MASK) > (end & LOG_BUF_MASK)) {
 			/* wrapped write */
 			__call_console_drivers(start & LOG_BUF_MASK,
@@ -580,7 +581,7 @@
  * acquire_console_sem - lock the console system for exclusive use.
  *
  * Acquires a semaphore which guarantees that the caller has
- * exclusive access to the console system and the console_drivers list.
+ * exclusive access to the console system and the console_driver_list.
  *
  * Can sleep, returns nothing.
  */
@@ -676,7 +677,7 @@
 		return;
 	console_locked = 1;
 	console_may_schedule = 0;
-	for (c = console_drivers; c != NULL; c = c->next)
+	list_for_each_entry(c, &console_driver_list, list)
 		if ((c->flags & CON_ENABLED) && c->unblank)
 			c->unblank();
 	release_console_sem();
@@ -689,7 +690,7 @@
 	struct tty_driver *driver = NULL;
 
 	acquire_console_sem();
-	for (c = console_drivers; c != NULL; c = c->next) {
+	list_for_each_entry(c, &console_driver_list, list) {
 		if (!c->device)
 			continue;
 		driver = c->device(c, index);
@@ -772,13 +773,11 @@
 	 *	preferred driver at the head of the list.
 	 */
 	acquire_console_sem();
-	if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
-		console->next = console_drivers;
-		console_drivers = console;
-	} else {
-		console->next = console_drivers->next;
-		console_drivers->next = console;
-	}
+	if ((console->flags & CON_CONSDEV) || list_empty(&console_driver_list)) 
+		list_add(&console->list, &console_driver_list);
+	else
+		list_add(&console->list, console_driver_list.next);
+
 	if (console->flags & CON_PRINTBUFFER) {
 		/*
 		 * release_console_sem() will print out the buffered messages
@@ -794,31 +793,17 @@
 
 int unregister_console(struct console * console)
 {
-        struct console *a,*b;
 	int res = 1;
 
 	acquire_console_sem();
-	if (console_drivers == console) {
-		console_drivers=console->next;
-		res = 0;
-	} else {
-		for (a=console_drivers->next, b=console_drivers ;
-		     a; b=a, a=b->next) {
-			if (a == console) {
-				b->next = a->next;
-				res = 0;
-				break;
-			}  
-		}
-	}
+	list_del(&console->list);
 	
 	/* If last console is removed, we re-enable picking the first
 	 * one that gets registered. Without that, pmac early boot console
 	 * would prevent fbcon from taking over.
 	 */
-	if (console_drivers == NULL)
+	if (list_empty(&console_driver_list))
 		preferred_console = -1;
-		
 
 	release_console_sem();
 	return res;
include/linux/console.h renamed from include/linux/console.h.console +2 -2
@@ -16,6 +16,7 @@
 
 #include <linux/types.h>
 #include <linux/spinlock.h>
+#include <linux/list.h>
 
 struct vc_data;
 struct console_font_op;
@@ -93,13 +94,12 @@
 	short	index;
 	int	cflag;
 	void	*data;
-	struct	 console *next;
+	struct list_head list;
 };
 
 extern int add_preferred_console(char *name, int idx, char *options);
 extern void register_console(struct console *);
 extern int unregister_console(struct console *);
-extern struct console *console_drivers;
 extern void acquire_console_sem(void);
 extern void release_console_sem(void);
 extern void console_conditional_schedule(void);
Signature · 4 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core
#3 lore raw

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

In reply to Russell King

Hi!

Quoted 3 lines

Add console_suspend() and console_resume() methods so the serial drivers can disable console output before suspending a port, and re-enable output afterwards.

Could it be called console_stop()/console_start()? suspend/resume sounds like power managment, and it is unrelated....
Pavel

Signature · 2 lines
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
#4 lore raw

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

Quoted 9 lines · Pavel Machek

Hi!

Quoted 3 lines

Add console_suspend() and console_resume() methods so the serial drivers can disable console output before suspending a port, and re-enable output afterwards.

Could it be called console_stop()/console_start()? suspend/resume sounds like power managment, and it is unrelated....

That's exactly what we ended up doing.

#5 lore raw

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

In reply to Pavel Machek
Quoted 8 lines · Pavel Machek

Hi!

Quoted 3 lines

Add console_suspend() and console_resume() methods so the serial drivers can disable console output before suspending a port, and re-enable output afterwards.

Could it be called console_stop()/console_start()? suspend/resume sounds like power managment, and it is unrelated....

It is power management related! However, we decided that console_start/ console_stop was a better name for it.

That said, it is _NOT_ intended for general use since there is NO counting of how many times console_stop() is called. It is merely used by the serial driver to prevent further serial console output before shutting the console port down for power management.

This avoids the problem where, on resume, we haven't even enabled the UART - if we were to attempt to write to the UART in this circumstance, it would not be setup in any way - no baud rate, and in fact, since it is disabled, we'd spin for ever waiting for the port to say its completed transmission of the first byte.

So yes, it most definitely _is_ power management related.

Signature · 4 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core