Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 18 Apr 2007 21:06:41 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> 
> This patch contains the following changes for supporting multiple logging
>  agents.
> 
> 1. extend netconsole to multiple netpolls
>    To send kernel messages to multiple logging agents, extend netcosnole
>     to be able to use multiple netpolls. Each netpoll sends kernel messages
>     to its own logging agent.
> 
> 2. change config parameter format
>    We change config parameter format from single configuration to multiple 
>    configurations separated by ';'.
> 
>    ex) sending kernel messages to destination1 and destination2 using eth0.
>     modprobe netconsole \
>             netconsole="@/eth0,@[destination1]/;@/eth0,@[destination2]/"
> 
> 3. introduce CONFIG_NETCONSOLE_DYNCON config to change between 
>    existing netconsole and netconsole applying the above function.
> 
> Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
> Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
> ---
> Index: mm/drivers/net/netconsole.c
> ===================================================================
> --- mm.orig/drivers/net/netconsole.c
> +++ mm/drivers/net/netconsole.c
> @@ -61,15 +61,102 @@ static struct netpoll np = {
>  	.remote_port = 6666,
>  	.remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
>  };
> -static int configured = 0;
>  
>  #define MAX_PRINT_CHUNK 1000
>  
> +#ifdef CONFIG_NETCONSOLE_DYNCON
> +struct netconsole_target {
> +	struct list_head list;
> +	int id;
> +	struct netpoll np;
> +};
> +
> +static LIST_HEAD(target_list);
> +static DEFINE_SPINLOCK(target_list_lock);
> +
> +static int add_target(char* target_config);
> +static void remove_target(struct netconsole_target *nt);
> +static void cleanup_netconsole(void);

I started to do some cleanups and fixups here, but abandoned it when it was
all getting a bit large.

Here are some fixes against this patch:


- Fix Kconfig

- Avoid need for forward-declared statics

- Coding style:

	fix `if' layout
	unneeded braces

- use spin_lock_irqsave() and _restore()



diff -puN drivers/net/Kconfig~netconsole-support-multiple-logging-fix drivers/net/Kconfig
--- a/drivers/net/Kconfig~netconsole-support-multiple-logging-fix
+++ a/drivers/net/Kconfig
@@ -2965,9 +2965,11 @@ config NETCONSOLE_DYNCON
 	bool "Support for multiple logging and UI for netconsole"
 	depends on NETCONSOLE
 	---help---
-	This option enables multiple logging and changing dynamically
-	 configurations (e.g. IP adderss, port number and so on)
-	 by using sysfs and ioctl.
+	  This option enables multiple logging and changing dynamically
+	  configurations (e.g. IP address, port number and so on)
+	  by using sysfs and ioctl.
+	  See <file:Documentation/networking/netconsole.txt> for details.
+
 endif #NETDEVICES
 
 config NETPOLL
diff -puN drivers/net/netconsole.c~netconsole-support-multiple-logging-fix drivers/net/netconsole.c
--- a/drivers/net/netconsole.c~netconsole-support-multiple-logging-fix
+++ a/drivers/net/netconsole.c
@@ -74,10 +74,6 @@ struct netconsole_target {
 static LIST_HEAD(target_list);
 static DEFINE_SPINLOCK(target_list_lock);
 
-static int add_target(char* target_config);
-static void remove_target(struct netconsole_target *nt);
-static void cleanup_netconsole(void);
-
 static int add_target(char* target_config)
 {
 	int retval = 0;
@@ -142,27 +138,24 @@ static void write_msg(struct console *co
 	if (list_empty(&target_list))
 		return;
 
-	local_irq_save(flags);
-	spin_lock(&target_list_lock);
+	spin_lock_irqsave(&target_list_lock, flags);
 
-	for(left = len; left; ) {
+	for (left = len; left; ) {
 		frag = min(left, MAX_PRINT_CHUNK);
-		list_for_each_entry(target, &target_list, list) {
+		list_for_each_entry(target, &target_list, list)
 			netpoll_send_udp(&target->np, msg, frag);
-		}
 		msg += frag;
 		left -= frag;
 	}
 
-	spin_unlock(&target_list_lock);
-	local_irq_restore(flags);
+	spin_unlock_irqrestore(&target_list_lock, flags);
 #else
 	if (!np.dev)
 		return;
 
 	local_irq_save(flags);
 
-	for(left = len; left; ) {
+	for (left = len; left; ) {
 		frag = min(left, MAX_PRINT_CHUNK);
 		netpoll_send_udp(&np, msg, frag);
 		msg += frag;
@@ -189,6 +182,20 @@ static int __init option_setup(char *opt
 __setup("netconsole=", option_setup);
 #endif
 
+static void cleanup_netconsole(void)
+{
+#ifdef CONFIG_NETCONSOLE_DYNCON
+	struct netconsole_target *nt, *tmp;
+
+	unregister_console(&netconsole);
+	list_for_each_entry_safe(nt, tmp, &target_list, list)
+		remove_target(nt);
+#else
+	unregister_console(&netconsole);
+	netpoll_cleanup(&np);
+#endif /* CONFIG_NETCONSOLE_DYNCON */
+}
+
 static int __init init_netconsole(void)
 {
 	char *tmp = config;
@@ -196,7 +203,7 @@ static int __init init_netconsole(void)
 	char *p;
 
 	register_console(&netconsole);
-	if(!strlen(config)) {
+	if (!strlen(config)) {
 		printk(KERN_ERR "netconsole: not configured\n");
 		return 0;
 	}
@@ -220,20 +227,5 @@ static int __init init_netconsole(void)
 	return 0;
 }
 
-static void cleanup_netconsole(void)
-{
-#ifdef CONFIG_NETCONSOLE_DYNCON
-	struct netconsole_target *nt, *tmp;
-
-	unregister_console(&netconsole);
-	list_for_each_entry_safe(nt, tmp, &target_list, list) {
-		remove_target(nt);
-	}
-#else
-	unregister_console(&netconsole);
-	netpoll_cleanup(&np);
-#endif /* CONFIG_NETCONSOLE_DYNCON */
-}
-
 module_init(init_netconsole);
 module_exit(cleanup_netconsole);
_