[PATCH v3 00/22] netoops support

[PATCH v3 00/22] netoops support

[Mike Waychison]

This patchset applies to v2.6.37-rc5.  It also applies cleanly to net-next as
of this morning.

The following series implements support for 'netoops', a simple driver that
will deliver kmsg logs together with machine specifics over the network.

This driver is based on code used in Google's production server environment.
We internally call the driver 'netdump', but are planning on changing the name
to 'netoops' to follow the convention set by both the mtdoops and ramoops
drivers.  We use these facilities to gather crash data from our entire fleet of
machines in a light-weight manner.  We do things this way because it
simply isn't feasible to gather full crash data off of every machine in
the wild when they decide it is time to die.

Currently, this driver only supports UDP over ipv4.

In order to handle configuration, the target support in netconsole is
fixed, seperated out, and re-used by netoops.

I'm posting these patches in an effort to eventually get this sort of
functionality mainlined.  Issues that I had personal concerns about have
been addressed, as well have several from others.  These changes are
documented below.

There is some contention as to whether or not the transmission of
structured data is useful in a crash situation.  I have documented why
we prefer to have structured date below in the comparison to netconsole.


Patchset summary
================

Patches 1 through 4 inclusive are fixes to the existing netconsole code,
adding locking consistency, fixing races and deadlocks.  These are probably
ready to be merged as they fix real problems with the netconsole driver.

Patches 5 through 14 inclusive splits the target configuration portion
of netconsole out into a new component in net/core/netpoll_targets.c.  These
are ready to merge as a cleanup series.

Patches 15 through 18 inclusive are core changes to support functionality in
the netoops driver.  These are required for the netoops driver itself, but are
independent of all prior patches.

Patches 19 through 22 represent the netoops driver itself, with different
functional aspects broken out.

 1 - netconsole: Remove unneeded reference counting
 2 - netconsole: Introduce locking over the netpoll fields
 3 - netconsole: Introduce 'enabled' state-machine
 4 - netconsole: Call netpoll_cleanup() in process context

 5 - netconsole: Wrap the list and locking in a structure
 6 - netconsole: Push configfs_subsystem into netpoll_targets
 7 - netconsole: Move netdev_notifier into netpoll_targets
 8 - netconsole: Split out netpoll_targets init/exit
 9 - netconsole: Add pointer to netpoll_targets
10 - netconsole: Rename netconsole_target -> netpoll_target
11 - netconsole: Abstract away the subsystem name
12 - netconsole: Move setting of default ports.
13 - netpoll: Introduce netpoll_target configs
14 - netpoll: Move target code into netpoll_targets.c

15 - Oops: Pass regs to oops_exit()
16 - kmsg_dumper: Pass pt_regs along to dumpers.
17 - kmsg_dumper: Introduce a new 'SOFT' dump reason
18 - sys-rq: Add option to soft dump

19 - netoops: add core functionality
20 - netoops: Add x86 specific bits to packet headers
21 - netoops: Add user-programmable boot_id
22 - netoops: Add a user programmable blob to the netoops packet.

Diffstat
========
 Documentation/sysrq.txt         |    4 
 arch/arm/kernel/traps.c         |    2 
 arch/parisc/kernel/traps.c      |    2 
 arch/powerpc/kernel/traps.c     |    2 
 arch/s390/kernel/traps.c        |    2 
 arch/sh/kernel/traps_32.c       |    2 
 arch/x86/kernel/dumpstack.c     |    2 
 drivers/char/ramoops.c          |    4 
 drivers/mtd/mtdoops.c           |    4 
 drivers/net/Kconfig             |   26 +
 drivers/net/Makefile            |    1 
 drivers/net/netconsole.c        |  735 +--------------------------------------
 drivers/net/netoops.c           |  346 ++++++++++++++++++
 drivers/tty/sysrq.c             |   14 
 include/linux/kernel.h          |    2 
 include/linux/kmsg_dump.h       |    9 
 include/linux/netpoll_targets.h |   76 ++++
 kernel/kexec.c                  |    5 
 kernel/panic.c                  |    6 
 kernel/printk.c                 |    4 
 net/core/Makefile               |    1 
 net/core/netpoll_targets.c      |  749 ++++++++++++++++++++++++++++++++++++++++
 22 files changed, 1260 insertions(+), 738 deletions(-)

Comparison to netconsole
========================

This driver differs from netconsole in a couple different ways.

* Network overheads:
     With the number of machines we have, streaming large amounts of consoles
     within the data center can really add up.  This gets worse when you take
     into account how reliant we are on kernel logging like OOM conditions
     (which are very regular and very verbose).  Events in the data center
     (such as application growth) tend to be temporally correlated, which
     causes large bursts of logging when we are OOM.  We aren't so interested
     in this kernel verbosity from a global collection standpoint though, and
     haven't been keen on the amount of extra un-regulated UDP traffic it would
     generate.  We are however interested in kernel oopses which occur far less
     often.

* Structured data:
     In terms of the data received, we've really benefited by having structured
     data in the payload.  We've been collecting kernel oopses since sometime
     in 2006 and have a _vast_ collection of crashes that we have indexed by
     just about anything you could ever want (registers, full dmesg text,
     backtraces, motherboards, CPU types, kernel versions, bios versions, etc).
     This has allowed us to quickly find 'big bugs' vs 'rare bugs' (similar to
     kerneloops.org) in a data center environment.

     This structured data also allows for automated labeling of oopses/panics
     using a variety of criteria.  Netconsole only provides unstructured
     streaming data, and the bits that we care about are either not present in
     the dmesg logs or they are, but is extremely difficult to parse them out
     (especially across kernel versions).  Other bits of information, like
     firmware version, are also difficult to associate with crashes with
     post-processing due to gaps in global sampling and the churn that occurs
     in the lab where versions can change quickly.

* Network reliability:
     Another area where the two approaches have differed has been in handling
     of network reliability.  Historically (though less and less now), we found
     that we had to transmit data several times.  We also used to explicitly
     space out packets with delays to handle switch chip buffer overruns.  Both
     of these functions I presume could be added to netconsole without too much
     of a problem.

ChangeLog:
==========
- v3
   - Dropped 'net_dump_now' interface as we already have CONFIG_LKDTM to
     trigger crashes.
   - Updated to also cancel any pending workers and clean the target up
     if needed when removing being dropped from configfs.  Issue
     identified by Neil Horman.
   - The user-programmable boot_id was been split out into its own patch.
   - Other userland programmable entries have been removed and replaced
     by a 'netoops_user_blob' field that is programmable to anything
     less than or equal to 128 bytes in length.
   - Support for 'one-shot' has been removed completely.
   - Now that one-shot support and net_dump_now support are removed from
     the patchset, we no longer have any interfaces in procfs.
   - x86 vendor is now specified in the packet headers.
   - 'netpoll_targets' can now be compiled as a module if neither
     netconsole nor netoops require it to be built in. This property
     also extends to CONFIGFS_FS.
   - All fields for packets are now encoded in little-endian.

- v2
   - Now uses the same mechanism that netconsole uses for configuring
     targets, which is also now abstracted out to
     net/core/netpoll_targets.c.

[PATCH v3 01/22] netconsole: Remove unneeded reference counting

[Mike Waychison]

The loops that iterate through the list of targets for emitting console
messages in the netconsole driver operating with interrupts disabled and
the list-protecting lock held.  There is no way for the elements of the
list to disappear, so we don't need to grab references to them.

This patch keeps the definitions however of netconsole_target_get() and
netconsole_target_put() as they are used in a subsequent patch.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 94255f0..c87a49e 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -671,7 +671,6 @@ static int netconsole_netdev_event(struct notifier_block *this,
 
 	spin_lock_irqsave(&target_list_lock, flags);
 	list_for_each_entry(nt, &target_list, list) {
-		netconsole_target_get(nt);
 		if (nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
@@ -693,7 +692,6 @@ static int netconsole_netdev_event(struct notifier_block *this,
 				break;
 			}
 		}
-		netconsole_target_put(nt);
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
 	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
@@ -722,7 +720,6 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 
 	spin_lock_irqsave(&target_list_lock, flags);
 	list_for_each_entry(nt, &target_list, list) {
-		netconsole_target_get(nt);
 		if (nt->enabled && netif_running(nt->np.dev)) {
 			/*
 			 * We nest this inside the for-each-target loop above
@@ -738,7 +735,6 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 				left -= frag;
 			}
 		}
-		netconsole_target_put(nt);
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
 }

[PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields

[Mike Waychison]

The netconsole driver currently doesn't do any locking over its
configuration fields.  This can cause problems if we were to ever have
concurrent writing to fields while somebody is enabling the service.

For simplicity, this patch extends targets_list_lock to cover all
configuration fields within the targets.  Macros are also added here to
wrap accessors so that we check whether the target has been enabled with
locking handled.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |  114 ++++++++++++++++++++++++++--------------------
 1 files changed, 64 insertions(+), 50 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index c87a49e..6e16888 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 			     const char *buf,
 			     size_t count)
 {
+	unsigned long flags;
 	int err;
 	long enabled;
 
@@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		return enabled;
 
 	if (enabled) {	/* 1 */
+		spin_lock_irqsave(&target_list_lock, flags);
+		if (nt->enabled)
+			goto busy;
+		spin_unlock_irqrestore(&target_list_lock, flags);
 
 		/*
 		 * Skip netpoll_parse_options() -- all the attributes are
@@ -343,18 +348,28 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		netpoll_print_options(&nt->np);
 
 		err = netpoll_setup(&nt->np);
+		spin_lock_irqsave(&target_list_lock, flags);
+		if (err)
+			nt->enabled = 0;
+		else
+			nt->enabled = 1;
+		spin_unlock_irqrestore(&target_list_lock, flags);
 		if (err)
 			return err;
 
 		printk(KERN_INFO "netconsole: network logging started\n");
-
 	} else {	/* 0 */
+		spin_lock_irqsave(&target_list_lock, flags);
+		nt->enabled = 0;
+		spin_unlock_irqrestore(&target_list_lock, flags);
+
 		netpoll_cleanup(&nt->np);
 	}
 
-	nt->enabled = enabled;
-
 	return strnlen(buf, count);
+busy:
+	spin_unlock_irqrestore(&target_list_lock, flags);
+	return -EBUSY;
 }
 
 static ssize_t store_dev_name(struct netconsole_target *nt,
@@ -363,13 +378,6 @@ static ssize_t store_dev_name(struct netconsole_target *nt,
 {
 	size_t len;
 
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	strlcpy(nt->np.dev_name, buf, IFNAMSIZ);
 
 	/* Get rid of possible trailing newline from echo(1) */
@@ -387,13 +395,6 @@ static ssize_t store_local_port(struct netconsole_target *nt,
 	long local_port;
 #define __U16_MAX	((__u16) ~0U)
 
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	local_port = strtol10_check_range(buf, 0, __U16_MAX);
 	if (local_port < 0)
 		return local_port;
@@ -410,13 +411,6 @@ static ssize_t store_remote_port(struct netconsole_target *nt,
 	long remote_port;
 #define __U16_MAX	((__u16) ~0U)
 
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	remote_port = strtol10_check_range(buf, 0, __U16_MAX);
 	if (remote_port < 0)
 		return remote_port;
@@ -430,13 +424,6 @@ static ssize_t store_local_ip(struct netconsole_target *nt,
 			      const char *buf,
 			      size_t count)
 {
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	nt->np.local_ip = in_aton(buf);
 
 	return strnlen(buf, count);
@@ -446,13 +433,6 @@ static ssize_t store_remote_ip(struct netconsole_target *nt,
 			       const char *buf,
 			       size_t count)
 {
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	nt->np.remote_ip = in_aton(buf);
 
 	return strnlen(buf, count);
@@ -466,13 +446,6 @@ static ssize_t store_remote_mac(struct netconsole_target *nt,
 	char *p = (char *) buf;
 	int i;
 
-	if (nt->enabled) {
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "
-				"disable to update parameters\n",
-				config_item_name(&nt->item));
-		return -EINVAL;
-	}
-
 	for (i = 0; i < ETH_ALEN - 1; i++) {
 		remote_mac[i] = simple_strtoul(p, &p, 16);
 		if (*p != ':')
@@ -496,15 +469,56 @@ invalid:
  * Attribute definitions for netconsole_target.
  */
 
-#define NETCONSOLE_TARGET_ATTR_RO(_name)				\
+#define __NETCONSOLE_TARGET_ATTR_RO(_name, _prefix_...)			\
 static struct netconsole_target_attr netconsole_target_##_name =	\
-	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_name, NULL)
+	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_prefix_##_name, NULL)
 
-#define NETCONSOLE_TARGET_ATTR_RW(_name)				\
+#define __NETCONSOLE_TARGET_ATTR_RW(_name, _prefix_...)			\
 static struct netconsole_target_attr netconsole_target_##_name =	\
-	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR, show_##_name, store_##_name)
+	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR,			\
+			show_##_prefix_##_name, store_##_prefix_##_name)
+
+#define NETCONSOLE_WRAP_ATTR_STORE(_name)				\
+static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
+				    const char *buf,			\
+				    size_t count)			\
+{									\
+	unsigned long flags;						\
+	ssize_t ret;							\
+	spin_lock_irqsave(&target_list_lock, flags);			\
+	if (nt->enabled) {						\
+		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
+				"disable to update parameters\n",	\
+				config_item_name(&nt->item));		\
+		spin_unlock_irqrestore(&target_list_lock, flags);	\
+		return -EBUSY;						\
+	}								\
+	ret = store_##_name(nt, buf, count);				\
+	spin_unlock_irqrestore(&target_list_lock, flags);		\
+	return ret;							\
+}
+
+#define NETCONSOLE_WRAP_ATTR_SHOW(_name)				\
+static ssize_t show_locked_##_name(struct netconsole_target *nt, char *buf) \
+{									\
+	unsigned long flags;						\
+	ssize_t ret;							\
+	spin_lock_irqsave(&target_list_lock, flags);			\
+	ret = show_##_name(nt, buf);					\
+	spin_unlock_irqrestore(&target_list_lock, flags);		\
+	return ret;							\
+}
+
+#define NETCONSOLE_TARGET_ATTR_RW(_name)				\
+		NETCONSOLE_WRAP_ATTR_STORE(_name)			\
+		NETCONSOLE_WRAP_ATTR_SHOW(_name)			\
+		__NETCONSOLE_TARGET_ATTR_RW(_name, locked_)
+
+#define NETCONSOLE_TARGET_ATTR_RO(_name)				\
+		NETCONSOLE_WRAP_ATTR_SHOW(_name)			\
+		__NETCONSOLE_TARGET_ATTR_RO(_name, locked_)
 
-NETCONSOLE_TARGET_ATTR_RW(enabled);
+__NETCONSOLE_TARGET_ATTR_RW(enabled);
 NETCONSOLE_TARGET_ATTR_RW(dev_name);
 NETCONSOLE_TARGET_ATTR_RW(local_port);
 NETCONSOLE_TARGET_ATTR_RW(remote_port);

Re: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields

[Michał Mirosław]

2010/12/14 Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>:
> The netconsole driver currently doesn't do any locking over its
> configuration fields.  This can cause problems if we were to ever have
> concurrent writing to fields while somebody is enabling the service.
>
> For simplicity, this patch extends targets_list_lock to cover all
> configuration fields within the targets.  Macros are also added here to
> wrap accessors so that we check whether the target has been enabled with
> locking handled.
>
> Signed-off-by: Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Acked-by: Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/net/netconsole.c |  114 ++++++++++++++++++++++++++--------------------
>  1 files changed, 64 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index c87a49e..6e16888 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>                             const char *buf,
>                             size_t count)
>  {
> +       unsigned long flags;
>        int err;
>        long enabled;
>
> @@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>                return enabled;
>
>        if (enabled) {  /* 1 */
> +               spin_lock_irqsave(&target_list_lock, flags);
> +               if (nt->enabled)
> +                       goto busy;
> +               spin_unlock_irqrestore(&target_list_lock, flags);
>

This looks wrong. Unless there is another lock or mutex covering this
function, at this point (after spin_unlock_irqrestore()) another
thread might set nt->enabled = 1.

Best Regards,
Michał Mirosław

Re: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields

[Mike Waychison]

2010/12/14 Michał Mirosław <mirqus@gmail.com>:
> 2010/12/14 Mike Waychison <mikew@google.com>:
>> The netconsole driver currently doesn't do any locking over its
>> configuration fields.  This can cause problems if we were to ever have
>> concurrent writing to fields while somebody is enabling the service.
>>
>> For simplicity, this patch extends targets_list_lock to cover all
>> configuration fields within the targets.  Macros are also added here to
>> wrap accessors so that we check whether the target has been enabled with
>> locking handled.
>>
>> Signed-off-by: Mike Waychison <mikew@google.com>
>> Acked-by: Matt Mackall <mpm@selenic.com>
>> ---
>>  drivers/net/netconsole.c |  114 ++++++++++++++++++++++++++--------------------
>>  1 files changed, 64 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
>> index c87a49e..6e16888 100644
>> --- a/drivers/net/netconsole.c
>> +++ b/drivers/net/netconsole.c
>> @@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>>                             const char *buf,
>>                             size_t count)
>>  {
>> +       unsigned long flags;
>>        int err;
>>        long enabled;
>>
>> @@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>>                return enabled;
>>
>>        if (enabled) {  /* 1 */
>> +               spin_lock_irqsave(&target_list_lock, flags);
>> +               if (nt->enabled)
>> +                       goto busy;
>> +               spin_unlock_irqrestore(&target_list_lock, flags);
>>
>
> This looks wrong. Unless there is another lock or mutex covering this
> function, at this point (after spin_unlock_irqrestore()) another
> thread might set nt->enabled = 1.
>

Agreed that this looks wrong :)

It is fixed in the next patch where a state machine is introduced to
replace the binary flag nt->enabled.  The code before this patch had
the a very similar problem in that a target could be enabled twice.
store_enabled() would call netpoll_setup() the second time without
checking to see if it was already enabled.

[PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Mike Waychison]

Representing the internal state within netconsole isn't really a boolean
value, but rather a state machine with transitions.

This patch introduces 4 states that the netconsole multi-target can
handle.  These states are:
- NETPOLL_DISABLED:
    The netpoll structure hasn't been setup.
- NETPOLL_SETTINGUP:
    The netpoll structure is being setup, and only whoever set this
    state can transition out of it.  Must come from the NETPOLL_DISABLED
    state.
- NETPOLL_ENABLED:
    The netpoll structure has been setup and we are good to emit
    packets.
- NETPOLL_CLEANING:
    Somebody is queued to call netpoll_clean.  Whoever does so must
    transition out of this state.  Must come from the NETPOLL_CLEANING
    state.

The SETTINGUP state specifically targets the window where
netpoll_setup() can take a while (for example, waiting on RTNL).
During this window, we want to exclude console messages from being
emitted to this netpoll target.  We also want to exclude any subsequent
user thread that tries to simultaneously enable or disable the target.

The CLEANING state will be used in a subsequent patch to safely defer
netpoll_cleanup to scheduled work in process context (to fix the
deadlock that will occur whenever NETDEV_UNREGISTER is seen).

When introducing these new transition states, it no longer makes sense
to 'disable' the target if the interface is going down, only when it is
being removed completely (or deslaved).  In fact, prior to this change,
we would leak a reference to the network device if an interface was
brought down, the target removed, and netconsole unloaded.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   62 +++++++++++++++++++++++++++++-----------------
 1 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 6e16888..288a025 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -71,16 +71,24 @@ static LIST_HEAD(target_list);
 /* This needs to be a spinlock because write_msg() cannot sleep */
 static DEFINE_SPINLOCK(target_list_lock);
 
+#define NETPOLL_DISABLED	0
+#define NETPOLL_SETTINGUP	1
+#define NETPOLL_ENABLED		2
+#define NETPOLL_CLEANING	3
+
 /**
  * struct netconsole_target - Represents a configured netconsole target.
  * @list:	Links this target into the target_list.
  * @item:	Links us into the configfs subsystem hierarchy.
- * @enabled:	On / off knob to enable / disable target.
- *		Visible from userspace (read-write).
- *		We maintain a strict 1:1 correspondence between this and
- *		whether the corresponding netpoll is active or inactive.
+ * @np_state:	Enabled / Disabled / SettingUp / Cleaning
+ *		Visible from userspace (read-write) as "enabled".
+ *		We maintain a state machine here of the valid states.  Either a
+ *		target is enabled or disabled, but it may also be in a
+ *		transitional state whereby nobody is allowed to act on the
+ *		target other than whoever owns the transition.
+ *
  *		Also, other parameters of a target may be modified at
- *		runtime only when it is disabled (enabled == 0).
+ *		runtime only when it is disabled (np_state == NETPOLL_ENABLED).
  * @np:		The netpoll structure for this target.
  *		Contains the other userspace visible parameters:
  *		dev_name	(read-write)
@@ -96,7 +104,7 @@ struct netconsole_target {
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
 	struct config_item	item;
 #endif
-	int			enabled;
+	int			np_state;
 	struct netpoll		np;
 };
 
@@ -189,7 +197,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	if (err)
 		goto fail;
 
-	nt->enabled = 1;
+	nt->np_state = NETPOLL_ENABLED;
 
 	return nt;
 
@@ -275,7 +283,8 @@ static long strtol10_check_range(const char *cp, long min, long max)
 
 static ssize_t show_enabled(struct netconsole_target *nt, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", nt->enabled);
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			nt->np_state == NETPOLL_ENABLED);
 }
 
 static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
@@ -337,9 +346,12 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 
 	if (enabled) {	/* 1 */
 		spin_lock_irqsave(&target_list_lock, flags);
-		if (nt->enabled)
+		if (nt->np_state != NETPOLL_DISABLED)
 			goto busy;
-		spin_unlock_irqrestore(&target_list_lock, flags);
+		else {
+			nt->np_state = NETPOLL_SETTINGUP;
+			spin_unlock_irqrestore(&target_list_lock, flags);
+		}
 
 		/*
 		 * Skip netpoll_parse_options() -- all the attributes are
@@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		err = netpoll_setup(&nt->np);
 		spin_lock_irqsave(&target_list_lock, flags);
 		if (err)
-			nt->enabled = 0;
+			nt->np_state = NETPOLL_DISABLED;
 		else
-			nt->enabled = 1;
+			nt->np_state = NETPOLL_ENABLED;
 		spin_unlock_irqrestore(&target_list_lock, flags);
 		if (err)
 			return err;
@@ -360,10 +372,17 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		printk(KERN_INFO "netconsole: network logging started\n");
 	} else {	/* 0 */
 		spin_lock_irqsave(&target_list_lock, flags);
-		nt->enabled = 0;
+		if (nt->np_state == NETPOLL_ENABLED)
+			nt->np_state = NETPOLL_CLEANING;
+		else if (nt->np_state != NETPOLL_DISABLED)
+			goto busy;
 		spin_unlock_irqrestore(&target_list_lock, flags);
 
 		netpoll_cleanup(&nt->np);
+
+		spin_lock_irqsave(&target_list_lock, flags);
+		nt->np_state = NETPOLL_DISABLED;
+		spin_unlock_irqrestore(&target_list_lock, flags);
 	}
 
 	return strnlen(buf, count);
@@ -486,7 +505,7 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
 	unsigned long flags;						\
 	ssize_t ret;							\
 	spin_lock_irqsave(&target_list_lock, flags);			\
-	if (nt->enabled) {						\
+	if (nt->np_state != NETPOLL_DISABLED) {				\
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
 				"disable to update parameters\n",	\
 				config_item_name(&nt->item));		\
@@ -642,7 +661,7 @@ static void drop_netconsole_target(struct config_group *group,
 	 * The target may have never been enabled, or was manually disabled
 	 * before being removed so netpoll may have already been cleaned up.
 	 */
-	if (nt->enabled)
+	if (nt->np_state == NETPOLL_ENABLED)
 		netpoll_cleanup(&nt->np);
 
 	config_item_put(&nt->item);
@@ -680,16 +699,17 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	struct net_device *dev = ptr;
 
 	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
-	      event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
+	      event == NETDEV_BONDING_DESLAVE))
 		goto done;
 
 	spin_lock_irqsave(&target_list_lock, flags);
 	list_for_each_entry(nt, &target_list, list) {
-		if (nt->np.dev == dev) {
+		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
 				strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
 				break;
+			case NETDEV_BONDING_DESLAVE:
 			case NETDEV_UNREGISTER:
 				/*
 				 * rtnl_lock already held
@@ -699,11 +719,6 @@ static int netconsole_netdev_event(struct notifier_block *this,
 					dev_put(nt->np.dev);
 					nt->np.dev = NULL;
 				}
-				/* Fall through */
-			case NETDEV_GOING_DOWN:
-			case NETDEV_BONDING_DESLAVE:
-				nt->enabled = 0;
-				break;
 			}
 		}
 	}
@@ -734,7 +749,8 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 
 	spin_lock_irqsave(&target_list_lock, flags);
 	list_for_each_entry(nt, &target_list, list) {
-		if (nt->enabled && netif_running(nt->np.dev)) {
+		if (nt->np_state == NETPOLL_ENABLED
+		    && netif_running(nt->np.dev)) {
 			/*
 			 * We nest this inside the for-each-target loop above
 			 * so that we're able to get as much logging out to

Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Michał Mirosław]

2010/12/14 Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>:
> Representing the internal state within netconsole isn't really a boolean
> value, but rather a state machine with transitions.
[...]
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 6e16888..288a025 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
[...]
> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>                err = netpoll_setup(&nt->np);
>                spin_lock_irqsave(&target_list_lock, flags);
>                if (err)
> -                       nt->enabled = 0;
> +                       nt->np_state = NETPOLL_DISABLED;
>                else
> -                       nt->enabled = 1;
> +                       nt->np_state = NETPOLL_ENABLED;
>                spin_unlock_irqrestore(&target_list_lock, flags);
>                if (err)
>                        return err;
[...]

Since the spinlock protects only nt->np_state setting, you might be
able to remove it altogether and use cmpxchg() where nt->np_state
transitions from enabled or disabled state.

Maybe the locking scheme just needs more thought altogether?

Best Regards,
Michał Mirosław

Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Mike Waychison]

2010/12/14 Michał Mirosław <mirqus@gmail.com>:
> 2010/12/14 Mike Waychison <mikew@google.com>:
>> Representing the internal state within netconsole isn't really a boolean
>> value, but rather a state machine with transitions.
> [...]
>> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
>> index 6e16888..288a025 100644
>> --- a/drivers/net/netconsole.c
>> +++ b/drivers/net/netconsole.c
> [...]
>> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>>                err = netpoll_setup(&nt->np);
>>                spin_lock_irqsave(&target_list_lock, flags);
>>                if (err)
>> -                       nt->enabled = 0;
>> +                       nt->np_state = NETPOLL_DISABLED;
>>                else
>> -                       nt->enabled = 1;
>> +                       nt->np_state = NETPOLL_ENABLED;
>>                spin_unlock_irqrestore(&target_list_lock, flags);
>>                if (err)
>>                        return err;
> [...]
>
> Since the spinlock protects only nt->np_state setting, you might be
> able to remove it altogether and use cmpxchg() where nt->np_state
> transitions from enabled or disabled state.
>
> Maybe the locking scheme just needs more thought altogether?

The target_list_lock protects the list, as well as the state
transitions.  This makes iterating through the list and getting a
consistent view of the targets a lot easier when it comes time to
transmitting packets we are guaranteed that nobody is changing the
target state underneath us if nt->np_state == NETPOLL_ENABLED while we
hold the lock.

Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Neil Horman]

On Tue, Dec 14, 2010 at 01:29:10PM -0800, Mike Waychison wrote:
> Representing the internal state within netconsole isn't really a boolean
> value, but rather a state machine with transitions.
> 
> This patch introduces 4 states that the netconsole multi-target can
> handle.  These states are:
> - NETPOLL_DISABLED:
>     The netpoll structure hasn't been setup.
> - NETPOLL_SETTINGUP:
>     The netpoll structure is being setup, and only whoever set this
>     state can transition out of it.  Must come from the NETPOLL_DISABLED
>     state.
> - NETPOLL_ENABLED:
>     The netpoll structure has been setup and we are good to emit
>     packets.
> - NETPOLL_CLEANING:
>     Somebody is queued to call netpoll_clean.  Whoever does so must
>     transition out of this state.  Must come from the NETPOLL_CLEANING
>     state.
> 
> The SETTINGUP state specifically targets the window where
> netpoll_setup() can take a while (for example, waiting on RTNL).
> During this window, we want to exclude console messages from being
> emitted to this netpoll target.  We also want to exclude any subsequent
> user thread that tries to simultaneously enable or disable the target.
> 
> The CLEANING state will be used in a subsequent patch to safely defer
> netpoll_cleanup to scheduled work in process context (to fix the
> deadlock that will occur whenever NETDEV_UNREGISTER is seen).
> 
> When introducing these new transition states, it no longer makes sense
> to 'disable' the target if the interface is going down, only when it is
> being removed completely (or deslaved).  In fact, prior to this change,
> we would leak a reference to the network device if an interface was
> brought down, the target removed, and netconsole unloaded.
> 
> Signed-off-by: Mike Waychison <mikew@google.com>
> Acked-by: Matt Mackall <mpm@selenic.com>
> ---
>  drivers/net/netconsole.c |   62 +++++++++++++++++++++++++++++-----------------
>  1 files changed, 39 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 6e16888..288a025 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -71,16 +71,24 @@ static LIST_HEAD(target_list);
>  /* This needs to be a spinlock because write_msg() cannot sleep */
>  static DEFINE_SPINLOCK(target_list_lock);
>  
> +#define NETPOLL_DISABLED	0
> +#define NETPOLL_SETTINGUP	1
> +#define NETPOLL_ENABLED		2
> +#define NETPOLL_CLEANING	3
> +
>  /**
>   * struct netconsole_target - Represents a configured netconsole target.
>   * @list:	Links this target into the target_list.
>   * @item:	Links us into the configfs subsystem hierarchy.
> - * @enabled:	On / off knob to enable / disable target.
> - *		Visible from userspace (read-write).
> - *		We maintain a strict 1:1 correspondence between this and
> - *		whether the corresponding netpoll is active or inactive.
> + * @np_state:	Enabled / Disabled / SettingUp / Cleaning
> + *		Visible from userspace (read-write) as "enabled".
> + *		We maintain a state machine here of the valid states.  Either a
> + *		target is enabled or disabled, but it may also be in a
> + *		transitional state whereby nobody is allowed to act on the
> + *		target other than whoever owns the transition.
> + *
>   *		Also, other parameters of a target may be modified at
> - *		runtime only when it is disabled (enabled == 0).
> + *		runtime only when it is disabled (np_state == NETPOLL_ENABLED).
>   * @np:		The netpoll structure for this target.
>   *		Contains the other userspace visible parameters:
>   *		dev_name	(read-write)
> @@ -96,7 +104,7 @@ struct netconsole_target {
>  #ifdef	CONFIG_NETCONSOLE_DYNAMIC
>  	struct config_item	item;
>  #endif
> -	int			enabled;
> +	int			np_state;
>  	struct netpoll		np;
>  };
>  
> @@ -189,7 +197,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
>  	if (err)
>  		goto fail;
>  
> -	nt->enabled = 1;
> +	nt->np_state = NETPOLL_ENABLED;
>  
>  	return nt;
>  
> @@ -275,7 +283,8 @@ static long strtol10_check_range(const char *cp, long min, long max)
>  
>  static ssize_t show_enabled(struct netconsole_target *nt, char *buf)
>  {
> -	return snprintf(buf, PAGE_SIZE, "%d\n", nt->enabled);
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			nt->np_state == NETPOLL_ENABLED);
>  }
>  
>  static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
> @@ -337,9 +346,12 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  
>  	if (enabled) {	/* 1 */
>  		spin_lock_irqsave(&target_list_lock, flags);
> -		if (nt->enabled)
> +		if (nt->np_state != NETPOLL_DISABLED)
>  			goto busy;
> -		spin_unlock_irqrestore(&target_list_lock, flags);
> +		else {
> +			nt->np_state = NETPOLL_SETTINGUP;
> +			spin_unlock_irqrestore(&target_list_lock, flags);
> +		}
>  
>  		/*
>  		 * Skip netpoll_parse_options() -- all the attributes are
> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  		err = netpoll_setup(&nt->np);
>  		spin_lock_irqsave(&target_list_lock, flags);
>  		if (err)
> -			nt->enabled = 0;
> +			nt->np_state = NETPOLL_DISABLED;
>  		else
> -			nt->enabled = 1;
> +			nt->np_state = NETPOLL_ENABLED;
>  		spin_unlock_irqrestore(&target_list_lock, flags);
>  		if (err)
>  			return err;
> @@ -360,10 +372,17 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>  		printk(KERN_INFO "netconsole: network logging started\n");
>  	} else {	/* 0 */
>  		spin_lock_irqsave(&target_list_lock, flags);
> -		nt->enabled = 0;
> +		if (nt->np_state == NETPOLL_ENABLED)
> +			nt->np_state = NETPOLL_CLEANING;
> +		else if (nt->np_state != NETPOLL_DISABLED)
> +			goto busy;
>  		spin_unlock_irqrestore(&target_list_lock, flags);
>  
>  		netpoll_cleanup(&nt->np);
> +
> +		spin_lock_irqsave(&target_list_lock, flags);
> +		nt->np_state = NETPOLL_DISABLED;
> +		spin_unlock_irqrestore(&target_list_lock, flags);
>  	}
>  
>  	return strnlen(buf, count);
> @@ -486,7 +505,7 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
>  	unsigned long flags;						\
>  	ssize_t ret;							\
>  	spin_lock_irqsave(&target_list_lock, flags);			\
> -	if (nt->enabled) {						\
> +	if (nt->np_state != NETPOLL_DISABLED) {				\
>  		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
>  				"disable to update parameters\n",	\
>  				config_item_name(&nt->item));		\
> @@ -642,7 +661,7 @@ static void drop_netconsole_target(struct config_group *group,
>  	 * The target may have never been enabled, or was manually disabled
>  	 * before being removed so netpoll may have already been cleaned up.
>  	 */
> -	if (nt->enabled)
> +	if (nt->np_state == NETPOLL_ENABLED)
>  		netpoll_cleanup(&nt->np);
>  
>  	config_item_put(&nt->item);
> @@ -680,16 +699,17 @@ static int netconsole_netdev_event(struct notifier_block *this,
>  	struct net_device *dev = ptr;
>  
>  	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
> -	      event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
> +	      event == NETDEV_BONDING_DESLAVE))
>  		goto done;
>  
>  	spin_lock_irqsave(&target_list_lock, flags);
>  	list_for_each_entry(nt, &target_list, list) {
> -		if (nt->np.dev == dev) {
> +		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
>  			switch (event) {
>  			case NETDEV_CHANGENAME:
>  				strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
>  				break;
> +			case NETDEV_BONDING_DESLAVE:
>  			case NETDEV_UNREGISTER:
I don't follow what you're doing here.  Previously NETDEV_BONDING_DESLAVE events
simply caused the netconsole interface to get deactivated, and now we're doing a
dev_put on the bonded interface bacause of the above move.  Given that
NETDEV_BONDING_DESLAVE events are generated when a slave leaves the bond, this
doesn't seem right, or am I missing something

Regards
Neil

Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine

[Mike Waychison]

On Fri, Dec 17, 2010 at 12:52 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Tue, Dec 14, 2010 at 01:29:10PM -0800, Mike Waychison wrote:

>> @@ -680,16 +699,17 @@ static int netconsole_netdev_event(struct notifier_block *this,
>>       struct net_device *dev = ptr;
>>
>>       if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
>> -           event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
>> +           event == NETDEV_BONDING_DESLAVE))
>>               goto done;
>>
>>       spin_lock_irqsave(&target_list_lock, flags);
>>       list_for_each_entry(nt, &target_list, list) {
>> -             if (nt->np.dev == dev) {
>> +             if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
>>                       switch (event) {
>>                       case NETDEV_CHANGENAME:
>>                               strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
>>                               break;
>> +                     case NETDEV_BONDING_DESLAVE:
>>                       case NETDEV_UNREGISTER:
> I don't follow what you're doing here.  Previously NETDEV_BONDING_DESLAVE events
> simply caused the netconsole interface to get deactivated, and now we're doing a
> dev_put on the bonded interface bacause of the above move.  Given that
> NETDEV_BONDING_DESLAVE events are generated when a slave leaves the bond, this
> doesn't seem right, or am I missing something

Ya, I did a poor job of explaining this bit.  Let me try now:

- NETDEV_GOING_DOWN is unneccesary because we will always see a
NETDEV_UNREGISTER event, which is why it is removed.
- I wasn't sure how to handle the NETDEV_BONDING_DESLAVE event in a
consistent way.  Originally we would disable the target, but this
actually leaks nt->np as it will never get cleaned up properly.  To
the user, it looks as if the target gets disabled.

I don't think you have any contention with the first comment above
(though I need to document it).

You are right in that with this patch, the target is left enabled,
when we should probably mark it as disabled.  I'd be happy to add the
transition from NETPOLL_ENABLED -> NETPOLL_DISABLED here if you'd
like, but it'd move again in the next patch anyway (which already
re-introduces the state transition in defer_netpoll_cleanup()).  Sorry
this is confusing :(   I'll make it clearer in the next series
version.

[PATCH v3 04/22] netconsole: Call netpoll_cleanup() in process context

[Mike Waychison]

The netconsole driver currently deadlocks if a NETDEV_UNREGISTER event
is received while netconsole is in use, which in turn causes it to pin a
reference to the network device.  The first deadlock was dealt with in
3b410a31 so that we wouldn't recursively grab RTNL, but even calling
__netpoll_cleanup isn't safe to do considering that we are in atomic
context.  __netpoll_cleanup assumes it can sleep and has several
sleeping calls, such as synchronize_rcu_bh and
cancel_rearming_delayed_work.

Fix this by deferring netpoll_cleanup using scheduling work that
operates in process context.  We have to grab a reference to the
config_item in this case as we need to pin the item in place until it is
operated on.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
Changelog:
- v3
  - Updated to also cancel any pending workers and clean the target up
    if needed when removing being dropped from configfs.  Issue
    identified by Neil Horman.
---
 drivers/net/netconsole.c |   64 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 288a025..68700c1 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -106,6 +106,7 @@ struct netconsole_target {
 #endif
 	int			np_state;
 	struct netpoll		np;
+	struct work_struct	cleanup_work;
 };
 
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
@@ -166,6 +167,22 @@ static void netconsole_target_put(struct netconsole_target *nt)
 
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
 
+static void deferred_netpoll_cleanup(struct work_struct *work)
+{
+	struct netconsole_target *nt;
+	unsigned long flags;
+
+	nt = container_of(work, struct netconsole_target, cleanup_work);
+	netpoll_cleanup(&nt->np);
+
+	spin_lock_irqsave(&target_list_lock, flags);
+	BUG_ON(nt->np_state != NETPOLL_CLEANING);
+	nt->np_state = NETPOLL_DISABLED;
+	spin_unlock_irqrestore(&target_list_lock, flags);
+
+	netconsole_target_put(nt);
+}
+
 /* Allocate new target (from boot/module param) and setup netpoll for it */
 static struct netconsole_target *alloc_param_target(char *target_config)
 {
@@ -187,6 +204,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
 
 	/* Parse parameters and setup netpoll */
 	err = netpoll_parse_options(&nt->np, target_config);
@@ -209,7 +227,9 @@ fail:
 /* Cleanup netpoll for given target (from boot/module param) and free it */
 static void free_param_target(struct netconsole_target *nt)
 {
-	netpoll_cleanup(&nt->np);
+	cancel_work_sync(&nt->cleanup_work);
+	if (nt->np_state == NETPOLL_CLEANING || nt->np_state == NETPOLL_ENABLED)
+		netpoll_cleanup(&nt->np);
 	kfree(nt);
 }
 
@@ -350,6 +370,13 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 			goto busy;
 		else {
 			nt->np_state = NETPOLL_SETTINGUP;
+			/*
+			 * Nominally, we would grab an extra reference on the
+			 * config_item here for dynamic targets while we let go
+			 * of the lock, but this isn't required in this case
+			 * because there is a reference implicitly held by the
+			 * caller of the store operation.
+			 */
 			spin_unlock_irqrestore(&target_list_lock, flags);
 		}
 
@@ -635,6 +662,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
 
 	/* Initialize the config_item member */
 	config_item_init_type_name(&nt->item, name, &netconsole_target_type);
@@ -658,10 +686,16 @@ static void drop_netconsole_target(struct config_group *group,
 	spin_unlock_irqrestore(&target_list_lock, flags);
 
 	/*
-	 * The target may have never been enabled, or was manually disabled
-	 * before being removed so netpoll may have already been cleaned up.
+	 * The target may have never been disabled, or was disabled due
+	 * to a netdev event, but we haven't had the chance to clean
+	 * things up yet.
+	 *
+	 * We can't wait for the target to be cleaned up by its
+	 * scheduled work however, as that work doesn't pin this module
+	 * in place.
 	 */
-	if (nt->np_state == NETPOLL_ENABLED)
+	cancel_work_sync(&nt->cleanup_work);
+	if (nt->np_state == NETPOLL_ENABLED || nt->np_state == NETPOLL_CLEANING)
 		netpoll_cleanup(&nt->np);
 
 	config_item_put(&nt->item);
@@ -689,6 +723,19 @@ static struct configfs_subsystem netconsole_subsys = {
 
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
 
+/*
+ * Call netpoll_cleanup on this target asynchronously.
+ * target_list_lock is required.
+ */
+static void defer_netpoll_cleanup(struct netconsole_target *nt)
+{
+	if (nt->np_state != NETPOLL_ENABLED)
+		return;
+	netconsole_target_get(nt);
+	nt->np_state = NETPOLL_CLEANING;
+	schedule_work(&nt->cleanup_work);
+}
+
 /* Handle network interface device notifications */
 static int netconsole_netdev_event(struct notifier_block *this,
 				   unsigned long event,
@@ -712,13 +759,10 @@ static int netconsole_netdev_event(struct notifier_block *this,
 			case NETDEV_BONDING_DESLAVE:
 			case NETDEV_UNREGISTER:
 				/*
-				 * rtnl_lock already held
+				 * We can't cleanup netpoll in atomic context.
+				 * Kick it off as deferred work.
 				 */
-				if (nt->np.dev) {
-					__netpoll_cleanup(&nt->np);
-					dev_put(nt->np.dev);
-					nt->np.dev = NULL;
-				}
+				defer_netpoll_cleanup(nt);
 			}
 		}
 	}

[PATCH v3 05/22] netconsole: Wrap the list and locking in a structure

[Mike Waychison]

In preparation for moving all the netpoll target handling code out of
netconsole proper and into netpoll, wrap the list and lock in a new
structure, netpoll_targets.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   85 ++++++++++++++++++++++++----------------------
 1 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 68700c1..3aa7151 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -65,11 +65,14 @@ static int __init option_setup(char *opt)
 __setup("netconsole=", option_setup);
 #endif	/* MODULE */
 
-/* Linked list of all configured targets */
-static LIST_HEAD(target_list);
-
-/* This needs to be a spinlock because write_msg() cannot sleep */
-static DEFINE_SPINLOCK(target_list_lock);
+struct netpoll_targets {
+	struct list_head list;
+	spinlock_t lock;
+};
+#define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
+	{ .list = LIST_HEAD_INIT(x.list), \
+	  .lock = __SPIN_LOCK_UNLOCKED(x.lock) }
+static DEFINE_NETPOLL_TARGETS(targets);
 
 #define NETPOLL_DISABLED	0
 #define NETPOLL_SETTINGUP	1
@@ -78,7 +81,7 @@ static DEFINE_SPINLOCK(target_list_lock);
 
 /**
  * struct netconsole_target - Represents a configured netconsole target.
- * @list:	Links this target into the target_list.
+ * @list:	Links this target into the netpoll_targets.list.
  * @item:	Links us into the configfs subsystem hierarchy.
  * @np_state:	Enabled / Disabled / SettingUp / Cleaning
  *		Visible from userspace (read-write) as "enabled".
@@ -175,10 +178,10 @@ static void deferred_netpoll_cleanup(struct work_struct *work)
 	nt = container_of(work, struct netconsole_target, cleanup_work);
 	netpoll_cleanup(&nt->np);
 
-	spin_lock_irqsave(&target_list_lock, flags);
+	spin_lock_irqsave(&targets.lock, flags);
 	BUG_ON(nt->np_state != NETPOLL_CLEANING);
 	nt->np_state = NETPOLL_DISABLED;
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_unlock_irqrestore(&targets.lock, flags);
 
 	netconsole_target_put(nt);
 }
@@ -365,7 +368,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		return enabled;
 
 	if (enabled) {	/* 1 */
-		spin_lock_irqsave(&target_list_lock, flags);
+		spin_lock_irqsave(&targets.lock, flags);
 		if (nt->np_state != NETPOLL_DISABLED)
 			goto busy;
 		else {
@@ -377,7 +380,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 			 * because there is a reference implicitly held by the
 			 * caller of the store operation.
 			 */
-			spin_unlock_irqrestore(&target_list_lock, flags);
+			spin_unlock_irqrestore(&targets.lock, flags);
 		}
 
 		/*
@@ -387,34 +390,34 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		netpoll_print_options(&nt->np);
 
 		err = netpoll_setup(&nt->np);
-		spin_lock_irqsave(&target_list_lock, flags);
+		spin_lock_irqsave(&targets.lock, flags);
 		if (err)
 			nt->np_state = NETPOLL_DISABLED;
 		else
 			nt->np_state = NETPOLL_ENABLED;
-		spin_unlock_irqrestore(&target_list_lock, flags);
+		spin_unlock_irqrestore(&targets.lock, flags);
 		if (err)
 			return err;
 
 		printk(KERN_INFO "netconsole: network logging started\n");
 	} else {	/* 0 */
-		spin_lock_irqsave(&target_list_lock, flags);
+		spin_lock_irqsave(&targets.lock, flags);
 		if (nt->np_state == NETPOLL_ENABLED)
 			nt->np_state = NETPOLL_CLEANING;
 		else if (nt->np_state != NETPOLL_DISABLED)
 			goto busy;
-		spin_unlock_irqrestore(&target_list_lock, flags);
+		spin_unlock_irqrestore(&targets.lock, flags);
 
 		netpoll_cleanup(&nt->np);
 
-		spin_lock_irqsave(&target_list_lock, flags);
+		spin_lock_irqsave(&targets.lock, flags);
 		nt->np_state = NETPOLL_DISABLED;
-		spin_unlock_irqrestore(&target_list_lock, flags);
+		spin_unlock_irqrestore(&targets.lock, flags);
 	}
 
 	return strnlen(buf, count);
 busy:
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_unlock_irqrestore(&targets.lock, flags);
 	return -EBUSY;
 }
 
@@ -531,16 +534,16 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
 {									\
 	unsigned long flags;						\
 	ssize_t ret;							\
-	spin_lock_irqsave(&target_list_lock, flags);			\
+	spin_lock_irqsave(&targets.lock, flags);			\
 	if (nt->np_state != NETPOLL_DISABLED) {				\
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
 				"disable to update parameters\n",	\
 				config_item_name(&nt->item));		\
-		spin_unlock_irqrestore(&target_list_lock, flags);	\
+		spin_unlock_irqrestore(&targets.lock, flags);		\
 		return -EBUSY;						\
 	}								\
 	ret = store_##_name(nt, buf, count);				\
-	spin_unlock_irqrestore(&target_list_lock, flags);		\
+	spin_unlock_irqrestore(&targets.lock, flags);			\
 	return ret;							\
 }
 
@@ -549,9 +552,9 @@ static ssize_t show_locked_##_name(struct netconsole_target *nt, char *buf) \
 {									\
 	unsigned long flags;						\
 	ssize_t ret;							\
-	spin_lock_irqsave(&target_list_lock, flags);			\
+	spin_lock_irqsave(&targets.lock, flags);			\
 	ret = show_##_name(nt, buf);					\
-	spin_unlock_irqrestore(&target_list_lock, flags);		\
+	spin_unlock_irqrestore(&targets.lock, flags);			\
 	return ret;							\
 }
 
@@ -668,9 +671,9 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	config_item_init_type_name(&nt->item, name, &netconsole_target_type);
 
 	/* Adding, but it is disabled */
-	spin_lock_irqsave(&target_list_lock, flags);
-	list_add(&nt->list, &target_list);
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_lock_irqsave(&targets.lock, flags);
+	list_add(&nt->list, &targets.list);
+	spin_unlock_irqrestore(&targets.lock, flags);
 
 	return &nt->item;
 }
@@ -681,9 +684,9 @@ static void drop_netconsole_target(struct config_group *group,
 	unsigned long flags;
 	struct netconsole_target *nt = to_target(item);
 
-	spin_lock_irqsave(&target_list_lock, flags);
+	spin_lock_irqsave(&targets.lock, flags);
 	list_del(&nt->list);
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_unlock_irqrestore(&targets.lock, flags);
 
 	/*
 	 * The target may have never been disabled, or was disabled due
@@ -698,7 +701,7 @@ static void drop_netconsole_target(struct config_group *group,
 	if (nt->np_state == NETPOLL_ENABLED || nt->np_state == NETPOLL_CLEANING)
 		netpoll_cleanup(&nt->np);
 
-	config_item_put(&nt->item);
+	netconsole_target_put(nt);
 }
 
 static struct configfs_group_operations netconsole_subsys_group_ops = {
@@ -725,7 +728,7 @@ static struct configfs_subsystem netconsole_subsys = {
 
 /*
  * Call netpoll_cleanup on this target asynchronously.
- * target_list_lock is required.
+ * targets.lock is required.
  */
 static void defer_netpoll_cleanup(struct netconsole_target *nt)
 {
@@ -749,8 +752,8 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	      event == NETDEV_BONDING_DESLAVE))
 		goto done;
 
-	spin_lock_irqsave(&target_list_lock, flags);
-	list_for_each_entry(nt, &target_list, list) {
+	spin_lock_irqsave(&targets.lock, flags);
+	list_for_each_entry(nt, &targets.list, list) {
 		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
@@ -766,7 +769,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
 			}
 		}
 	}
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_unlock_irqrestore(&targets.lock, flags);
 	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
 		printk(KERN_INFO "netconsole: network logging stopped, "
 			"interface %s %s\n",  dev->name,
@@ -788,11 +791,11 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 	const char *tmp;
 
 	/* Avoid taking lock and disabling interrupts unnecessarily */
-	if (list_empty(&target_list))
+	if (list_empty(&targets.list))
 		return;
 
-	spin_lock_irqsave(&target_list_lock, flags);
-	list_for_each_entry(nt, &target_list, list) {
+	spin_lock_irqsave(&targets.lock, flags);
+	list_for_each_entry(nt, &targets.list, list) {
 		if (nt->np_state == NETPOLL_ENABLED
 		    && netif_running(nt->np.dev)) {
 			/*
@@ -810,7 +813,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 			}
 		}
 	}
-	spin_unlock_irqrestore(&target_list_lock, flags);
+	spin_unlock_irqrestore(&targets.lock, flags);
 }
 
 static struct console netconsole = {
@@ -837,9 +840,9 @@ static int __init init_netconsole(void)
 			/* Dump existing printks when we register */
 			netconsole.flags |= CON_PRINTBUFFER;
 
-			spin_lock_irqsave(&target_list_lock, flags);
-			list_add(&nt->list, &target_list);
-			spin_unlock_irqrestore(&target_list_lock, flags);
+			spin_lock_irqsave(&targets.lock, flags);
+			list_add(&nt->list, &targets.list);
+			spin_unlock_irqrestore(&targets.lock, flags);
 		}
 	}
 
@@ -867,7 +870,7 @@ fail:
 	 * from the boot/module option exist here). Skipping the list
 	 * lock is safe here, and netpoll_cleanup() will sleep.
 	 */
-	list_for_each_entry_safe(nt, tmp, &target_list, list) {
+	list_for_each_entry_safe(nt, tmp, &targets.list, list) {
 		list_del(&nt->list);
 		free_param_target(nt);
 	}
@@ -891,7 +894,7 @@ static void __exit cleanup_netconsole(void)
 	 * destroy them. Skipping the list lock is safe here, and
 	 * netpoll_cleanup() will sleep.
 	 */
-	list_for_each_entry_safe(nt, tmp, &target_list, list) {
+	list_for_each_entry_safe(nt, tmp, &targets.list, list) {
 		list_del(&nt->list);
 		free_param_target(nt);
 	}

[PATCH v3 06/22] netconsole: Push configfs_subsystem into netpoll_targets

[Mike Waychison]

This patch is a step in the preparation of pushing the target handling
code out of netconsole and into netpoll proper where it can be used by
other clients.

Here, we move the configfs_subsystem bits out of global scope and
package it in the netpoll_targets structure that will maintain a set of
targets.  There is a bit of code that is rearranged due to needing to
provide netconsole_subsys_type in scope much earlier in the file.  This
has the added benefit of folding all of the dynamic target handling code
under a single #ifdef CONFIG_NETCONSOLE_DYNAMIC block.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |  132 ++++++++++++++++++++++------------------------
 1 files changed, 64 insertions(+), 68 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 3aa7151..aa95b91 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -68,6 +68,9 @@ __setup("netconsole=", option_setup);
 struct netpoll_targets {
 	struct list_head list;
 	spinlock_t lock;
+#ifdef	CONFIG_NETCONSOLE_DYNAMIC
+	struct configfs_subsystem configfs_subsys;
+#endif
 };
 #define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
 	{ .list = LIST_HEAD_INIT(x.list), \
@@ -112,63 +115,8 @@ struct netconsole_target {
 	struct work_struct	cleanup_work;
 };
 
-#ifdef	CONFIG_NETCONSOLE_DYNAMIC
-
-static struct configfs_subsystem netconsole_subsys;
-
-static int __init dynamic_netconsole_init(void)
-{
-	config_group_init(&netconsole_subsys.su_group);
-	mutex_init(&netconsole_subsys.su_mutex);
-	return configfs_register_subsystem(&netconsole_subsys);
-}
-
-static void __exit dynamic_netconsole_exit(void)
-{
-	configfs_unregister_subsystem(&netconsole_subsys);
-}
-
-/*
- * Targets that were created by parsing the boot/module option string
- * do not exist in the configfs hierarchy (and have NULL names) and will
- * never go away, so make these a no-op for them.
- */
-static void netconsole_target_get(struct netconsole_target *nt)
-{
-	if (config_item_name(&nt->item))
-		config_item_get(&nt->item);
-}
-
-static void netconsole_target_put(struct netconsole_target *nt)
-{
-	if (config_item_name(&nt->item))
-		config_item_put(&nt->item);
-}
-
-#else	/* !CONFIG_NETCONSOLE_DYNAMIC */
-
-static int __init dynamic_netconsole_init(void)
-{
-	return 0;
-}
-
-static void __exit dynamic_netconsole_exit(void)
-{
-}
-
-/*
- * No danger of targets going away from under us when dynamic
- * reconfigurability is off.
- */
-static void netconsole_target_get(struct netconsole_target *nt)
-{
-}
-
-static void netconsole_target_put(struct netconsole_target *nt)
-{
-}
-
-#endif	/* CONFIG_NETCONSOLE_DYNAMIC */
+static void netconsole_target_get(struct netconsole_target *nt);
+static void netconsole_target_put(struct netconsole_target *nt);
 
 static void deferred_netpoll_cleanup(struct work_struct *work)
 {
@@ -714,15 +662,63 @@ static struct config_item_type netconsole_subsys_type = {
 	.ct_owner	= THIS_MODULE,
 };
 
-/* The netconsole configfs subsystem */
-static struct configfs_subsystem netconsole_subsys = {
-	.su_group	= {
-		.cg_item	= {
-			.ci_namebuf	= "netconsole",
-			.ci_type	= &netconsole_subsys_type,
-		},
-	},
-};
+static int __init dynamic_netpoll_targets_init(struct netpoll_targets *nts)
+{
+	struct configfs_subsystem *subsys = &nts->configfs_subsys;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, "netconsole",
+		CONFIGFS_ITEM_NAME_LEN);
+	subsys->su_group.cg_item.ci_type = &netconsole_subsys_type;
+	return configfs_register_subsystem(subsys);
+}
+
+static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
+{
+	configfs_unregister_subsystem(&nts->configfs_subsys);
+}
+
+/*
+ * Targets that were created by parsing the boot/module option string
+ * do not exist in the configfs hierarchy (and have NULL names) and will
+ * never go away, so make these a no-op for them.
+ */
+static void netconsole_target_get(struct netconsole_target *nt)
+{
+	if (config_item_name(&nt->item))
+		config_item_get(&nt->item);
+}
+
+static void netconsole_target_put(struct netconsole_target *nt)
+{
+	if (config_item_name(&nt->item))
+		config_item_put(&nt->item);
+}
+
+#else	/* !CONFIG_NETCONSOLE_DYNAMIC */
+
+static int __init dynamic_netpoll_targets_init(const char *subsys_name,
+					       struct netpoll_targets *nts)
+{
+	return 0;
+}
+
+static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
+{
+}
+
+/*
+ * No danger of targets going away from under us when dynamic
+ * reconfigurability is off.
+ */
+static void netconsole_target_get(struct netconsole_target *nt)
+{
+}
+
+static void netconsole_target_put(struct netconsole_target *nt)
+{
+}
 
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
 
@@ -850,7 +846,7 @@ static int __init init_netconsole(void)
 	if (err)
 		goto fail;
 
-	err = dynamic_netconsole_init();
+	err = dynamic_netpoll_targets_init(&targets);
 	if (err)
 		goto undonotifier;
 
@@ -883,7 +879,7 @@ static void __exit cleanup_netconsole(void)
 	struct netconsole_target *nt, *tmp;
 
 	unregister_console(&netconsole);
-	dynamic_netconsole_exit();
+	dynamic_netpoll_targets_exit(&targets);
 	unregister_netdevice_notifier(&netconsole_netdev_notifier);
 
 	/*

[PATCH v3 07/22] netconsole: Move netdev_notifier into netpoll_targets

[Mike Waychison]

Move the notifier chain node up into struct netpoll_targets, and use it
to get at the netpoll_targets struct whenever we get a NETDEV event.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index aa95b91..fed427d 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -71,6 +71,7 @@ struct netpoll_targets {
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
 	struct configfs_subsystem configfs_subsys;
 #endif
+	struct notifier_block netdev_notifier;
 };
 #define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
 	{ .list = LIST_HEAD_INIT(x.list), \
@@ -740,6 +741,8 @@ static int netconsole_netdev_event(struct notifier_block *this,
 				   unsigned long event,
 				   void *ptr)
 {
+	struct netpoll_targets *nts = container_of(this, struct netpoll_targets,
+						   netdev_notifier);
 	unsigned long flags;
 	struct netconsole_target *nt;
 	struct net_device *dev = ptr;
@@ -748,8 +751,8 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	      event == NETDEV_BONDING_DESLAVE))
 		goto done;
 
-	spin_lock_irqsave(&targets.lock, flags);
-	list_for_each_entry(nt, &targets.list, list) {
+	spin_lock_irqsave(&nts->lock, flags);
+	list_for_each_entry(nt, &nts->list, list) {
 		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
@@ -765,7 +768,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
 			}
 		}
 	}
-	spin_unlock_irqrestore(&targets.lock, flags);
+	spin_unlock_irqrestore(&nts->lock, flags);
 	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
 		printk(KERN_INFO "netconsole: network logging stopped, "
 			"interface %s %s\n",  dev->name,
@@ -775,10 +778,6 @@ done:
 	return NOTIFY_DONE;
 }
 
-static struct notifier_block netconsole_netdev_notifier = {
-	.notifier_call  = netconsole_netdev_event,
-};
-
 static void write_msg(struct console *con, const char *msg, unsigned int len)
 {
 	int frag, left;
@@ -842,7 +841,8 @@ static int __init init_netconsole(void)
 		}
 	}
 
-	err = register_netdevice_notifier(&netconsole_netdev_notifier);
+	targets.netdev_notifier.notifier_call = netconsole_netdev_event;
+	err = register_netdevice_notifier(&targets.netdev_notifier);
 	if (err)
 		goto fail;
 
@@ -856,7 +856,7 @@ static int __init init_netconsole(void)
 	return err;
 
 undonotifier:
-	unregister_netdevice_notifier(&netconsole_netdev_notifier);
+	unregister_netdevice_notifier(&targets.netdev_notifier);
 
 fail:
 	printk(KERN_ERR "netconsole: cleaning up\n");
@@ -880,7 +880,7 @@ static void __exit cleanup_netconsole(void)
 
 	unregister_console(&netconsole);
 	dynamic_netpoll_targets_exit(&targets);
-	unregister_netdevice_notifier(&netconsole_netdev_notifier);
+	unregister_netdevice_notifier(&targets.netdev_notifier);
 
 	/*
 	 * Targets created via configfs pin references on our module

[PATCH v3 09/22] netconsole: Add pointer to netpoll_targets

[Mike Waychison]

For each netconsole_target, we'd like to get back at the parenting
netpoll_targets structure so that we can access the locking and list
fields.  Add a pointer that points to the netpoll_targets structure.  We
don't have to maintain reference counting because a netpoll_targets will
always out-live their children netconsole_target structures.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   71 +++++++++++++++++++++++++++++-----------------
 1 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 57451a7..6ab41f8 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -107,6 +107,7 @@ static DEFINE_NETPOLL_TARGETS(targets);
  *		remote_mac	(read-write)
  */
 struct netconsole_target {
+	struct netpoll_targets *nts;
 	struct list_head	list;
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
 	struct config_item	item;
@@ -122,21 +123,25 @@ static void netconsole_target_put(struct netconsole_target *nt);
 static void deferred_netpoll_cleanup(struct work_struct *work)
 {
 	struct netconsole_target *nt;
+	struct netpoll_targets *nts;
 	unsigned long flags;
 
 	nt = container_of(work, struct netconsole_target, cleanup_work);
+	nts = nt->nts;
+
 	netpoll_cleanup(&nt->np);
 
-	spin_lock_irqsave(&targets.lock, flags);
+	spin_lock_irqsave(&nts->lock, flags);
 	BUG_ON(nt->np_state != NETPOLL_CLEANING);
 	nt->np_state = NETPOLL_DISABLED;
-	spin_unlock_irqrestore(&targets.lock, flags);
+	spin_unlock_irqrestore(&nts->lock, flags);
 
 	netconsole_target_put(nt);
 }
 
 /* Allocate new target (from boot/module param) and setup netpoll for it */
-static struct netconsole_target *alloc_param_target(char *target_config)
+static struct netconsole_target *alloc_param_target(struct netpoll_targets *nts,
+						    char *target_config)
 {
 	int err = -ENOMEM;
 	struct netconsole_target *nt;
@@ -151,6 +156,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 		goto fail;
 	}
 
+	nt->nts = nts;
 	nt->np.name = "netconsole";
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
@@ -308,6 +314,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 			     const char *buf,
 			     size_t count)
 {
+	struct netpoll_targets *nts = nt->nts;
 	unsigned long flags;
 	int err;
 	long enabled;
@@ -317,7 +324,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		return enabled;
 
 	if (enabled) {	/* 1 */
-		spin_lock_irqsave(&targets.lock, flags);
+		spin_lock_irqsave(&nts->lock, flags);
 		if (nt->np_state != NETPOLL_DISABLED)
 			goto busy;
 		else {
@@ -329,7 +336,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 			 * because there is a reference implicitly held by the
 			 * caller of the store operation.
 			 */
-			spin_unlock_irqrestore(&targets.lock, flags);
+			spin_unlock_irqrestore(&nts->lock, flags);
 		}
 
 		/*
@@ -339,34 +346,34 @@ static ssize_t store_enabled(struct netconsole_target *nt,
 		netpoll_print_options(&nt->np);
 
 		err = netpoll_setup(&nt->np);
-		spin_lock_irqsave(&targets.lock, flags);
+		spin_lock_irqsave(&nts->lock, flags);
 		if (err)
 			nt->np_state = NETPOLL_DISABLED;
 		else
 			nt->np_state = NETPOLL_ENABLED;
-		spin_unlock_irqrestore(&targets.lock, flags);
+		spin_unlock_irqrestore(&nts->lock, flags);
 		if (err)
 			return err;
 
 		printk(KERN_INFO "netconsole: network logging started\n");
 	} else {	/* 0 */
-		spin_lock_irqsave(&targets.lock, flags);
+		spin_lock_irqsave(&nts->lock, flags);
 		if (nt->np_state == NETPOLL_ENABLED)
 			nt->np_state = NETPOLL_CLEANING;
 		else if (nt->np_state != NETPOLL_DISABLED)
 			goto busy;
-		spin_unlock_irqrestore(&targets.lock, flags);
+		spin_unlock_irqrestore(&nts->lock, flags);
 
 		netpoll_cleanup(&nt->np);
 
-		spin_lock_irqsave(&targets.lock, flags);
+		spin_lock_irqsave(&nts->lock, flags);
 		nt->np_state = NETPOLL_DISABLED;
-		spin_unlock_irqrestore(&targets.lock, flags);
+		spin_unlock_irqrestore(&nts->lock, flags);
 	}
 
 	return strnlen(buf, count);
 busy:
-	spin_unlock_irqrestore(&targets.lock, flags);
+	spin_unlock_irqrestore(&nts->lock, flags);
 	return -EBUSY;
 }
 
@@ -481,29 +488,31 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
 				    const char *buf,			\
 				    size_t count)			\
 {									\
+	struct netpoll_targets *nts = nt->nts;				\
 	unsigned long flags;						\
 	ssize_t ret;							\
-	spin_lock_irqsave(&targets.lock, flags);			\
+	spin_lock_irqsave(&nts->lock, flags);				\
 	if (nt->np_state != NETPOLL_DISABLED) {				\
 		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
 				"disable to update parameters\n",	\
 				config_item_name(&nt->item));		\
-		spin_unlock_irqrestore(&targets.lock, flags);		\
+		spin_unlock_irqrestore(&nts->lock, flags);		\
 		return -EBUSY;						\
 	}								\
 	ret = store_##_name(nt, buf, count);				\
-	spin_unlock_irqrestore(&targets.lock, flags);			\
+	spin_unlock_irqrestore(&nts->lock, flags);			\
 	return ret;							\
 }
 
 #define NETCONSOLE_WRAP_ATTR_SHOW(_name)				\
 static ssize_t show_locked_##_name(struct netconsole_target *nt, char *buf) \
 {									\
+	struct netpoll_targets *nts = nt->nts;				\
 	unsigned long flags;						\
 	ssize_t ret;							\
-	spin_lock_irqsave(&targets.lock, flags);			\
+	spin_lock_irqsave(&nts->lock, flags);				\
 	ret = show_##_name(nt, buf);					\
-	spin_unlock_irqrestore(&targets.lock, flags);			\
+	spin_unlock_irqrestore(&nts->lock, flags);			\
 	return ret;							\
 }
 
@@ -589,6 +598,13 @@ static struct config_item_type netconsole_target_type = {
 	.ct_owner		= THIS_MODULE,
 };
 
+static struct netpoll_targets *group_to_targets(struct config_group *group)
+{
+	struct configfs_subsystem *subsys;
+	subsys = container_of(group, struct configfs_subsystem, su_group);
+	return container_of(subsys, struct netpoll_targets, configfs_subsys);
+}
+
 /*
  * Group operations and type for netconsole_subsys.
  */
@@ -596,8 +612,9 @@ static struct config_item_type netconsole_target_type = {
 static struct config_item *make_netconsole_target(struct config_group *group,
 						  const char *name)
 {
-	unsigned long flags;
+	struct netpoll_targets *nts = group_to_targets(group);
 	struct netconsole_target *nt;
+	unsigned long flags;
 
 	/*
 	 * Allocate and initialize with defaults.
@@ -609,6 +626,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	nt->nts = nts;
 	nt->np.name = "netconsole";
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
@@ -620,9 +638,9 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	config_item_init_type_name(&nt->item, name, &netconsole_target_type);
 
 	/* Adding, but it is disabled */
-	spin_lock_irqsave(&targets.lock, flags);
-	list_add(&nt->list, &targets.list);
-	spin_unlock_irqrestore(&targets.lock, flags);
+	spin_lock_irqsave(&nts->lock, flags);
+	list_add(&nt->list, &nts->list);
+	spin_unlock_irqrestore(&nts->lock, flags);
 
 	return &nt->item;
 }
@@ -630,12 +648,13 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 static void drop_netconsole_target(struct config_group *group,
 				   struct config_item *item)
 {
-	unsigned long flags;
+	struct netpoll_targets *nts = group_to_targets(group);
 	struct netconsole_target *nt = to_target(item);
+	unsigned long flags;
 
-	spin_lock_irqsave(&targets.lock, flags);
+	spin_lock_irqsave(&nts->lock, flags);
 	list_del(&nt->list);
-	spin_unlock_irqrestore(&targets.lock, flags);
+	spin_unlock_irqrestore(&nts->lock, flags);
 
 	/*
 	 * The target may have never been disabled, or was disabled due
@@ -726,7 +745,7 @@ static void netconsole_target_put(struct netconsole_target *nt)
 
 /*
  * Call netpoll_cleanup on this target asynchronously.
- * targets.lock is required.
+ * nts->lock is required.
  */
 static void defer_netpoll_cleanup(struct netconsole_target *nt)
 {
@@ -830,7 +849,7 @@ static int __init register_netpoll_targets(const char *subsys_name,
 
 	if (strnlen(input, MAX_PARAM_LENGTH)) {
 		while ((target_config = strsep(&input, ";"))) {
-			nt = alloc_param_target(target_config);
+			nt = alloc_param_target(nts, target_config);
 			if (IS_ERR(nt)) {
 				err = PTR_ERR(nt);
 				goto fail;

[PATCH v3 10/22] netconsole: Rename netconsole_target -> netpoll_target

[Mike Waychison]

Rename targets from "struct netconsole_target" to "struct
netpoll_target" as they will soon no longer be netconsole specific.

While here, also rename the configfs related types.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |  238 +++++++++++++++++++++++-----------------------
 1 files changed, 119 insertions(+), 119 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 6ab41f8..2f6282d 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -84,7 +84,7 @@ static DEFINE_NETPOLL_TARGETS(targets);
 #define NETPOLL_CLEANING	3
 
 /**
- * struct netconsole_target - Represents a configured netconsole target.
+ * struct netpoll_target - Represents a configured netpoll target.
  * @list:	Links this target into the netpoll_targets.list.
  * @item:	Links us into the configfs subsystem hierarchy.
  * @np_state:	Enabled / Disabled / SettingUp / Cleaning
@@ -106,7 +106,7 @@ static DEFINE_NETPOLL_TARGETS(targets);
  *		local_mac	(read-only)
  *		remote_mac	(read-write)
  */
-struct netconsole_target {
+struct netpoll_target {
 	struct netpoll_targets *nts;
 	struct list_head	list;
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
@@ -117,16 +117,16 @@ struct netconsole_target {
 	struct work_struct	cleanup_work;
 };
 
-static void netconsole_target_get(struct netconsole_target *nt);
-static void netconsole_target_put(struct netconsole_target *nt);
+static void netpoll_target_get(struct netpoll_target *nt);
+static void netpoll_target_put(struct netpoll_target *nt);
 
 static void deferred_netpoll_cleanup(struct work_struct *work)
 {
-	struct netconsole_target *nt;
+	struct netpoll_target *nt;
 	struct netpoll_targets *nts;
 	unsigned long flags;
 
-	nt = container_of(work, struct netconsole_target, cleanup_work);
+	nt = container_of(work, struct netpoll_target, cleanup_work);
 	nts = nt->nts;
 
 	netpoll_cleanup(&nt->np);
@@ -136,15 +136,15 @@ static void deferred_netpoll_cleanup(struct work_struct *work)
 	nt->np_state = NETPOLL_DISABLED;
 	spin_unlock_irqrestore(&nts->lock, flags);
 
-	netconsole_target_put(nt);
+	netpoll_target_put(nt);
 }
 
 /* Allocate new target (from boot/module param) and setup netpoll for it */
-static struct netconsole_target *alloc_param_target(struct netpoll_targets *nts,
-						    char *target_config)
+static struct netpoll_target *alloc_param_target(struct netpoll_targets *nts,
+						 char *target_config)
 {
 	int err = -ENOMEM;
-	struct netconsole_target *nt;
+	struct netpoll_target *nt;
 
 	/*
 	 * Allocate and initialize with defaults.
@@ -183,7 +183,7 @@ fail:
 }
 
 /* Cleanup netpoll for given target (from boot/module param) and free it */
-static void free_param_target(struct netconsole_target *nt)
+static void free_param_target(struct netpoll_target *nt)
 {
 	cancel_work_sync(&nt->cleanup_work);
 	if (nt->np_state == NETPOLL_CLEANING || nt->np_state == NETPOLL_ENABLED)
@@ -211,19 +211,19 @@ static void free_param_target(struct netconsole_target *nt)
  *				<target>/...
  */
 
-struct netconsole_target_attr {
+struct netpoll_target_attr {
 	struct configfs_attribute	attr;
-	ssize_t				(*show)(struct netconsole_target *nt,
+	ssize_t				(*show)(struct netpoll_target *nt,
 						char *buf);
-	ssize_t				(*store)(struct netconsole_target *nt,
+	ssize_t				(*store)(struct netpoll_target *nt,
 						 const char *buf,
 						 size_t count);
 };
 
-static struct netconsole_target *to_target(struct config_item *item)
+static struct netpoll_target *to_target(struct config_item *item)
 {
 	return item ?
-		container_of(item, struct netconsole_target, item) :
+		container_of(item, struct netpoll_target, item) :
 		NULL;
 }
 
@@ -256,41 +256,41 @@ static long strtol10_check_range(const char *cp, long min, long max)
 }
 
 /*
- * Attribute operations for netconsole_target.
+ * Attribute operations for netpoll_target.
  */
 
-static ssize_t show_enabled(struct netconsole_target *nt, char *buf)
+static ssize_t show_enabled(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%d\n",
 			nt->np_state == NETPOLL_ENABLED);
 }
 
-static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
+static ssize_t show_dev_name(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%s\n", nt->np.dev_name);
 }
 
-static ssize_t show_local_port(struct netconsole_target *nt, char *buf)
+static ssize_t show_local_port(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.local_port);
 }
 
-static ssize_t show_remote_port(struct netconsole_target *nt, char *buf)
+static ssize_t show_remote_port(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.remote_port);
 }
 
-static ssize_t show_local_ip(struct netconsole_target *nt, char *buf)
+static ssize_t show_local_ip(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.local_ip);
 }
 
-static ssize_t show_remote_ip(struct netconsole_target *nt, char *buf)
+static ssize_t show_remote_ip(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.remote_ip);
 }
 
-static ssize_t show_local_mac(struct netconsole_target *nt, char *buf)
+static ssize_t show_local_mac(struct netpoll_target *nt, char *buf)
 {
 	struct net_device *dev = nt->np.dev;
 	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -298,7 +298,7 @@ static ssize_t show_local_mac(struct netconsole_target *nt, char *buf)
 	return snprintf(buf, PAGE_SIZE, "%pM\n", dev ? dev->dev_addr : bcast);
 }
 
-static ssize_t show_remote_mac(struct netconsole_target *nt, char *buf)
+static ssize_t show_remote_mac(struct netpoll_target *nt, char *buf)
 {
 	return snprintf(buf, PAGE_SIZE, "%pM\n", nt->np.remote_mac);
 }
@@ -310,7 +310,7 @@ static ssize_t show_remote_mac(struct netconsole_target *nt, char *buf)
  * would enable him to dynamically add new netpoll targets for new
  * network interfaces as and when they come up).
  */
-static ssize_t store_enabled(struct netconsole_target *nt,
+static ssize_t store_enabled(struct netpoll_target *nt,
 			     const char *buf,
 			     size_t count)
 {
@@ -377,7 +377,7 @@ busy:
 	return -EBUSY;
 }
 
-static ssize_t store_dev_name(struct netconsole_target *nt,
+static ssize_t store_dev_name(struct netpoll_target *nt,
 			      const char *buf,
 			      size_t count)
 {
@@ -393,7 +393,7 @@ static ssize_t store_dev_name(struct netconsole_target *nt,
 	return strnlen(buf, count);
 }
 
-static ssize_t store_local_port(struct netconsole_target *nt,
+static ssize_t store_local_port(struct netpoll_target *nt,
 				const char *buf,
 				size_t count)
 {
@@ -409,7 +409,7 @@ static ssize_t store_local_port(struct netconsole_target *nt,
 	return strnlen(buf, count);
 }
 
-static ssize_t store_remote_port(struct netconsole_target *nt,
+static ssize_t store_remote_port(struct netpoll_target *nt,
 				 const char *buf,
 				 size_t count)
 {
@@ -425,7 +425,7 @@ static ssize_t store_remote_port(struct netconsole_target *nt,
 	return strnlen(buf, count);
 }
 
-static ssize_t store_local_ip(struct netconsole_target *nt,
+static ssize_t store_local_ip(struct netpoll_target *nt,
 			      const char *buf,
 			      size_t count)
 {
@@ -434,7 +434,7 @@ static ssize_t store_local_ip(struct netconsole_target *nt,
 	return strnlen(buf, count);
 }
 
-static ssize_t store_remote_ip(struct netconsole_target *nt,
+static ssize_t store_remote_ip(struct netpoll_target *nt,
 			       const char *buf,
 			       size_t count)
 {
@@ -443,7 +443,7 @@ static ssize_t store_remote_ip(struct netconsole_target *nt,
 	return strnlen(buf, count);
 }
 
-static ssize_t store_remote_mac(struct netconsole_target *nt,
+static ssize_t store_remote_mac(struct netpoll_target *nt,
 				const char *buf,
 				size_t count)
 {
@@ -471,20 +471,20 @@ invalid:
 }
 
 /*
- * Attribute definitions for netconsole_target.
+ * Attribute definitions for netpoll_target.
  */
 
-#define __NETCONSOLE_TARGET_ATTR_RO(_name, _prefix_...)			\
-static struct netconsole_target_attr netconsole_target_##_name =	\
+#define __NETPOLL_TARGET_ATTR_RO(_name, _prefix_...)			\
+static struct netpoll_target_attr netpoll_target_##_name =	\
 	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_prefix_##_name, NULL)
 
-#define __NETCONSOLE_TARGET_ATTR_RW(_name, _prefix_...)			\
-static struct netconsole_target_attr netconsole_target_##_name =	\
+#define __NETPOLL_TARGET_ATTR_RW(_name, _prefix_...)			\
+static struct netpoll_target_attr netpoll_target_##_name =	\
 	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR,			\
 			show_##_prefix_##_name, store_##_prefix_##_name)
 
-#define NETCONSOLE_WRAP_ATTR_STORE(_name)				\
-static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
+#define NETPOLL_WRAP_ATTR_STORE(_name)					\
+static ssize_t store_locked_##_name(struct netpoll_target *nt,		\
 				    const char *buf,			\
 				    size_t count)			\
 {									\
@@ -504,8 +504,8 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt,	\
 	return ret;							\
 }
 
-#define NETCONSOLE_WRAP_ATTR_SHOW(_name)				\
-static ssize_t show_locked_##_name(struct netconsole_target *nt, char *buf) \
+#define NETPOLL_WRAP_ATTR_SHOW(_name)					\
+static ssize_t show_locked_##_name(struct netpoll_target *nt, char *buf) \
 {									\
 	struct netpoll_targets *nts = nt->nts;				\
 	unsigned long flags;						\
@@ -516,53 +516,53 @@ static ssize_t show_locked_##_name(struct netconsole_target *nt, char *buf) \
 	return ret;							\
 }
 
-#define NETCONSOLE_TARGET_ATTR_RW(_name)				\
-		NETCONSOLE_WRAP_ATTR_STORE(_name)			\
-		NETCONSOLE_WRAP_ATTR_SHOW(_name)			\
-		__NETCONSOLE_TARGET_ATTR_RW(_name, locked_)
-
-#define NETCONSOLE_TARGET_ATTR_RO(_name)				\
-		NETCONSOLE_WRAP_ATTR_SHOW(_name)			\
-		__NETCONSOLE_TARGET_ATTR_RO(_name, locked_)
-
-__NETCONSOLE_TARGET_ATTR_RW(enabled);
-NETCONSOLE_TARGET_ATTR_RW(dev_name);
-NETCONSOLE_TARGET_ATTR_RW(local_port);
-NETCONSOLE_TARGET_ATTR_RW(remote_port);
-NETCONSOLE_TARGET_ATTR_RW(local_ip);
-NETCONSOLE_TARGET_ATTR_RW(remote_ip);
-NETCONSOLE_TARGET_ATTR_RO(local_mac);
-NETCONSOLE_TARGET_ATTR_RW(remote_mac);
-
-static struct configfs_attribute *netconsole_target_attrs[] = {
-	&netconsole_target_enabled.attr,
-	&netconsole_target_dev_name.attr,
-	&netconsole_target_local_port.attr,
-	&netconsole_target_remote_port.attr,
-	&netconsole_target_local_ip.attr,
-	&netconsole_target_remote_ip.attr,
-	&netconsole_target_local_mac.attr,
-	&netconsole_target_remote_mac.attr,
+#define NETPOLL_TARGET_ATTR_RW(_name)				\
+		NETPOLL_WRAP_ATTR_STORE(_name)			\
+		NETPOLL_WRAP_ATTR_SHOW(_name)			\
+		__NETPOLL_TARGET_ATTR_RW(_name, locked_)
+
+#define NETPOLL_TARGET_ATTR_RO(_name)				\
+		NETPOLL_WRAP_ATTR_SHOW(_name)			\
+		__NETPOLL_TARGET_ATTR_RO(_name, locked_)
+
+__NETPOLL_TARGET_ATTR_RW(enabled);
+NETPOLL_TARGET_ATTR_RW(dev_name);
+NETPOLL_TARGET_ATTR_RW(local_port);
+NETPOLL_TARGET_ATTR_RW(remote_port);
+NETPOLL_TARGET_ATTR_RW(local_ip);
+NETPOLL_TARGET_ATTR_RW(remote_ip);
+NETPOLL_TARGET_ATTR_RO(local_mac);
+NETPOLL_TARGET_ATTR_RW(remote_mac);
+
+static struct configfs_attribute *netpoll_target_attrs[] = {
+	&netpoll_target_enabled.attr,
+	&netpoll_target_dev_name.attr,
+	&netpoll_target_local_port.attr,
+	&netpoll_target_remote_port.attr,
+	&netpoll_target_local_ip.attr,
+	&netpoll_target_remote_ip.attr,
+	&netpoll_target_local_mac.attr,
+	&netpoll_target_remote_mac.attr,
 	NULL,
 };
 
 /*
- * Item operations and type for netconsole_target.
+ * Item operations and type for netpoll_target.
  */
 
-static void netconsole_target_release(struct config_item *item)
+static void netpoll_target_release(struct config_item *item)
 {
 	kfree(to_target(item));
 }
 
-static ssize_t netconsole_target_attr_show(struct config_item *item,
-					   struct configfs_attribute *attr,
-					   char *buf)
+static ssize_t netpoll_target_attr_show(struct config_item *item,
+					struct configfs_attribute *attr,
+					char *buf)
 {
 	ssize_t ret = -EINVAL;
-	struct netconsole_target *nt = to_target(item);
-	struct netconsole_target_attr *na =
-		container_of(attr, struct netconsole_target_attr, attr);
+	struct netpoll_target *nt = to_target(item);
+	struct netpoll_target_attr *na =
+		container_of(attr, struct netpoll_target_attr, attr);
 
 	if (na->show)
 		ret = na->show(nt, buf);
@@ -570,15 +570,15 @@ static ssize_t netconsole_target_attr_show(struct config_item *item,
 	return ret;
 }
 
-static ssize_t netconsole_target_attr_store(struct config_item *item,
-					    struct configfs_attribute *attr,
-					    const char *buf,
-					    size_t count)
+static ssize_t netpoll_target_attr_store(struct config_item *item,
+					 struct configfs_attribute *attr,
+					 const char *buf,
+					 size_t count)
 {
 	ssize_t ret = -EINVAL;
-	struct netconsole_target *nt = to_target(item);
-	struct netconsole_target_attr *na =
-		container_of(attr, struct netconsole_target_attr, attr);
+	struct netpoll_target *nt = to_target(item);
+	struct netpoll_target_attr *na =
+		container_of(attr, struct netpoll_target_attr, attr);
 
 	if (na->store)
 		ret = na->store(nt, buf, count);
@@ -586,15 +586,15 @@ static ssize_t netconsole_target_attr_store(struct config_item *item,
 	return ret;
 }
 
-static struct configfs_item_operations netconsole_target_item_ops = {
-	.release		= netconsole_target_release,
-	.show_attribute		= netconsole_target_attr_show,
-	.store_attribute	= netconsole_target_attr_store,
+static struct configfs_item_operations netpoll_target_item_ops = {
+	.release		= netpoll_target_release,
+	.show_attribute		= netpoll_target_attr_show,
+	.store_attribute	= netpoll_target_attr_store,
 };
 
-static struct config_item_type netconsole_target_type = {
-	.ct_attrs		= netconsole_target_attrs,
-	.ct_item_ops		= &netconsole_target_item_ops,
+static struct config_item_type netpoll_target_type = {
+	.ct_attrs		= netpoll_target_attrs,
+	.ct_item_ops		= &netpoll_target_item_ops,
 	.ct_owner		= THIS_MODULE,
 };
 
@@ -606,14 +606,14 @@ static struct netpoll_targets *group_to_targets(struct config_group *group)
 }
 
 /*
- * Group operations and type for netconsole_subsys.
+ * Group operations and type for netpoll_target_subsys.
  */
 
-static struct config_item *make_netconsole_target(struct config_group *group,
-						  const char *name)
+static struct config_item *make_netpoll_target(struct config_group *group,
+					       const char *name)
 {
 	struct netpoll_targets *nts = group_to_targets(group);
-	struct netconsole_target *nt;
+	struct netpoll_target *nt;
 	unsigned long flags;
 
 	/*
@@ -635,7 +635,7 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
 
 	/* Initialize the config_item member */
-	config_item_init_type_name(&nt->item, name, &netconsole_target_type);
+	config_item_init_type_name(&nt->item, name, &netpoll_target_type);
 
 	/* Adding, but it is disabled */
 	spin_lock_irqsave(&nts->lock, flags);
@@ -645,11 +645,11 @@ static struct config_item *make_netconsole_target(struct config_group *group,
 	return &nt->item;
 }
 
-static void drop_netconsole_target(struct config_group *group,
-				   struct config_item *item)
+static void drop_netpoll_target(struct config_group *group,
+				struct config_item *item)
 {
 	struct netpoll_targets *nts = group_to_targets(group);
-	struct netconsole_target *nt = to_target(item);
+	struct netpoll_target *nt = to_target(item);
 	unsigned long flags;
 
 	spin_lock_irqsave(&nts->lock, flags);
@@ -669,16 +669,16 @@ static void drop_netconsole_target(struct config_group *group,
 	if (nt->np_state == NETPOLL_ENABLED || nt->np_state == NETPOLL_CLEANING)
 		netpoll_cleanup(&nt->np);
 
-	netconsole_target_put(nt);
+	netpoll_target_put(nt);
 }
 
-static struct configfs_group_operations netconsole_subsys_group_ops = {
-	.make_item	= make_netconsole_target,
-	.drop_item	= drop_netconsole_target,
+static struct configfs_group_operations netpoll_subsys_group_ops = {
+	.make_item	= make_netpoll_target,
+	.drop_item	= drop_netpoll_target,
 };
 
-static struct config_item_type netconsole_subsys_type = {
-	.ct_group_ops	= &netconsole_subsys_group_ops,
+static struct config_item_type netpoll_subsys_type = {
+	.ct_group_ops	= &netpoll_subsys_group_ops,
 	.ct_owner	= THIS_MODULE,
 };
 
@@ -691,7 +691,7 @@ static int __init dynamic_netpoll_targets_init(const char *subsys_name,
 	mutex_init(&subsys->su_mutex);
 	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, subsys_name,
 		CONFIGFS_ITEM_NAME_LEN);
-	subsys->su_group.cg_item.ci_type = &netconsole_subsys_type;
+	subsys->su_group.cg_item.ci_type = &netpoll_subsys_type;
 	return configfs_register_subsystem(subsys);
 }
 
@@ -705,13 +705,13 @@ static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
  * do not exist in the configfs hierarchy (and have NULL names) and will
  * never go away, so make these a no-op for them.
  */
-static void netconsole_target_get(struct netconsole_target *nt)
+static void netpoll_target_get(struct netpoll_target *nt)
 {
 	if (config_item_name(&nt->item))
 		config_item_get(&nt->item);
 }
 
-static void netconsole_target_put(struct netconsole_target *nt)
+static void netpoll_target_put(struct netpoll_target *nt)
 {
 	if (config_item_name(&nt->item))
 		config_item_put(&nt->item);
@@ -733,11 +733,11 @@ static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
  * No danger of targets going away from under us when dynamic
  * reconfigurability is off.
  */
-static void netconsole_target_get(struct netconsole_target *nt)
+static void netpoll_target_get(struct netpoll_target *nt)
 {
 }
 
-static void netconsole_target_put(struct netconsole_target *nt)
+static void netpoll_target_put(struct netpoll_target *nt)
 {
 }
 
@@ -747,24 +747,24 @@ static void netconsole_target_put(struct netconsole_target *nt)
  * Call netpoll_cleanup on this target asynchronously.
  * nts->lock is required.
  */
-static void defer_netpoll_cleanup(struct netconsole_target *nt)
+static void defer_netpoll_cleanup(struct netpoll_target *nt)
 {
 	if (nt->np_state != NETPOLL_ENABLED)
 		return;
-	netconsole_target_get(nt);
+	netpoll_target_get(nt);
 	nt->np_state = NETPOLL_CLEANING;
 	schedule_work(&nt->cleanup_work);
 }
 
 /* Handle network interface device notifications */
-static int netconsole_netdev_event(struct notifier_block *this,
-				   unsigned long event,
-				   void *ptr)
+static int netpoll_targets_netdev_event(struct notifier_block *this,
+					unsigned long event,
+					void *ptr)
 {
 	struct netpoll_targets *nts = container_of(this, struct netpoll_targets,
 						   netdev_notifier);
 	unsigned long flags;
-	struct netconsole_target *nt;
+	struct netpoll_target *nt;
 	struct net_device *dev = ptr;
 
 	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
@@ -802,7 +802,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 {
 	int frag, left;
 	unsigned long flags;
-	struct netconsole_target *nt;
+	struct netpoll_target *nt;
 	const char *tmp;
 
 	/* Avoid taking lock and disabling interrupts unnecessarily */
@@ -842,7 +842,7 @@ static int __init register_netpoll_targets(const char *subsys_name,
 					   char *static_targets)
 {
 	int err;
-	struct netconsole_target *nt, *tmp;
+	struct netpoll_target *nt, *tmp;
 	char *target_config;
 	char *input = static_targets;
 	unsigned long flags;
@@ -861,7 +861,7 @@ static int __init register_netpoll_targets(const char *subsys_name,
 		}
 	}
 
-	nts->netdev_notifier.notifier_call = netconsole_netdev_event;
+	nts->netdev_notifier.notifier_call = netpoll_targets_netdev_event;
 	err = register_netdevice_notifier(&nts->netdev_notifier);
 	if (err)
 		goto fail;
@@ -890,7 +890,7 @@ fail:
 
 static void __exit unregister_netpoll_targets(struct netpoll_targets *nts)
 {
-	struct netconsole_target *nt, *tmp;
+	struct netpoll_target *nt, *tmp;
 
 	dynamic_netpoll_targets_exit(nts);
 	unregister_netdevice_notifier(&nts->netdev_notifier);

[PATCH v3 11/22] netconsole: Abstract away the subsystem name

[Mike Waychison]

Remove any hardcoded instances of "netconsole" and keep a new copy in
netpoll_targets.  We could perhaps use the copy that is stored in the
configfs group's name, but that isn't always available depending on
build config options,and maintaining the extra string is just easier.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   50 +++++++++++++++++++++++++++++-----------------
 1 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 2f6282d..5c8701f 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -72,6 +72,7 @@ struct netpoll_targets {
 	struct configfs_subsystem configfs_subsys;
 #endif
 	struct notifier_block netdev_notifier;
+	char *subsys_name;
 };
 #define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
 	{ .list = LIST_HEAD_INIT(x.list), \
@@ -152,12 +153,13 @@ static struct netpoll_target *alloc_param_target(struct netpoll_targets *nts,
 	 */
 	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
 	if (!nt) {
-		printk(KERN_ERR "netconsole: failed to allocate memory\n");
+		printk(KERN_ERR "%s: failed to allocate memory\n",
+		       nts->subsys_name);
 		goto fail;
 	}
 
 	nt->nts = nts;
-	nt->np.name = "netconsole";
+	nt->np.name = nts->subsys_name;
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
@@ -196,7 +198,7 @@ static void free_param_target(struct netpoll_target *nt)
 /*
  * Our subsystem hierarchy is:
  *
- * /sys/kernel/config/netconsole/
+ * /sys/kernel/config/<subsys_name>/
  *				|
  *				<target>/
  *				|	enabled
@@ -232,7 +234,8 @@ static struct netpoll_target *to_target(struct config_item *item)
  * We return (signed) long only because we may want to return errors.
  * Do not use this to convert numbers that are allowed to be negative.
  */
-static long strtol10_check_range(const char *cp, long min, long max)
+static long strtol10_check_range(struct netpoll_targets *nts,
+				 const char *cp, long min, long max)
 {
 	long ret;
 	char *p = (char *) cp;
@@ -243,12 +246,12 @@ static long strtol10_check_range(const char *cp, long min, long max)
 	ret = simple_strtol(p, &p, 10);
 
 	if (*p && (*p != '\n')) {
-		printk(KERN_ERR "netconsole: invalid input\n");
+		printk(KERN_ERR "%s: invalid input\n", nts->subsys_name);
 		return -EINVAL;
 	}
 	if ((ret < min) || (ret > max)) {
-		printk(KERN_ERR "netconsole: input %ld must be between "
-				"%ld and %ld\n", ret, min, max);
+		printk(KERN_ERR "%s: input %ld must be between %ld and %ld\n",
+		       nts->subsys_name, ret, min, max);
 		return -EINVAL;
 	}
 
@@ -319,7 +322,7 @@ static ssize_t store_enabled(struct netpoll_target *nt,
 	int err;
 	long enabled;
 
-	enabled = strtol10_check_range(buf, 0, 1);
+	enabled = strtol10_check_range(nts, buf, 0, 1);
 	if (enabled < 0)
 		return enabled;
 
@@ -355,7 +358,8 @@ static ssize_t store_enabled(struct netpoll_target *nt,
 		if (err)
 			return err;
 
-		printk(KERN_INFO "netconsole: network logging started\n");
+		printk(KERN_INFO "%s: network logging started\n",
+		       nts->subsys_name);
 	} else {	/* 0 */
 		spin_lock_irqsave(&nts->lock, flags);
 		if (nt->np_state == NETPOLL_ENABLED)
@@ -400,7 +404,7 @@ static ssize_t store_local_port(struct netpoll_target *nt,
 	long local_port;
 #define __U16_MAX	((__u16) ~0U)
 
-	local_port = strtol10_check_range(buf, 0, __U16_MAX);
+	local_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
 	if (local_port < 0)
 		return local_port;
 
@@ -416,7 +420,7 @@ static ssize_t store_remote_port(struct netpoll_target *nt,
 	long remote_port;
 #define __U16_MAX	((__u16) ~0U)
 
-	remote_port = strtol10_check_range(buf, 0, __U16_MAX);
+	remote_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
 	if (remote_port < 0)
 		return remote_port;
 
@@ -466,7 +470,7 @@ static ssize_t store_remote_mac(struct netpoll_target *nt,
 	return strnlen(buf, count);
 
 invalid:
-	printk(KERN_ERR "netconsole: invalid input\n");
+	printk(KERN_ERR "%s: invalid input\n", nt->nts->subsys_name);
 	return -EINVAL;
 }
 
@@ -493,8 +497,9 @@ static ssize_t store_locked_##_name(struct netpoll_target *nt,		\
 	ssize_t ret;							\
 	spin_lock_irqsave(&nts->lock, flags);				\
 	if (nt->np_state != NETPOLL_DISABLED) {				\
-		printk(KERN_ERR "netconsole: target (%s) is enabled, "	\
+		printk(KERN_ERR "%s: target (%s) is enabled, "		\
 				"disable to update parameters\n",	\
+				nts->subsys_name,			\
 				config_item_name(&nt->item));		\
 		spin_unlock_irqrestore(&nts->lock, flags);		\
 		return -EBUSY;						\
@@ -622,12 +627,13 @@ static struct config_item *make_netpoll_target(struct config_group *group,
 	 */
 	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
 	if (!nt) {
-		printk(KERN_ERR "netconsole: failed to allocate memory\n");
+		printk(KERN_ERR "%s: failed to allocate memory\n",
+		       nts->subsys_name);
 		return ERR_PTR(-ENOMEM);
 	}
 
 	nt->nts = nts;
-	nt->np.name = "netconsole";
+	nt->np.name = nts->subsys_name;
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
@@ -790,8 +796,8 @@ static int netpoll_targets_netdev_event(struct notifier_block *this,
 	}
 	spin_unlock_irqrestore(&nts->lock, flags);
 	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
-		printk(KERN_INFO "netconsole: network logging stopped, "
-			"interface %s %s\n",  dev->name,
+		printk(KERN_INFO "%s: network logging stopped, "
+			"interface %s %s\n", nts->subsys_name, dev->name,
 			event == NETDEV_UNREGISTER ? "unregistered" : "released slaves");
 
 done:
@@ -866,12 +872,19 @@ static int __init register_netpoll_targets(const char *subsys_name,
 	if (err)
 		goto fail;
 
+	nts->subsys_name = kstrdup(subsys_name, GFP_KERNEL);
+	err = -ENOMEM;
+	if (!nts->subsys_name)
+		goto undonotifier;
+
 	err = dynamic_netpoll_targets_init(subsys_name, nts);
 	if (err)
-		goto undonotifier;
+		goto free_subsys_name;
 
 	return 0;
 
+free_subsys_name:
+	kfree(nts->subsys_name);
 undonotifier:
 	unregister_netdevice_notifier(&nts->netdev_notifier);
 fail:
@@ -907,6 +920,7 @@ static void __exit unregister_netpoll_targets(struct netpoll_targets *nts)
 		list_del(&nt->list);
 		free_param_target(nt);
 	}
+	kfree(nts->subsys_name);
 }
 
 static int __init init_netconsole(void)

[PATCH v3 12/22] netconsole: Move setting of default ports.

[Mike Waychison]

Clients of netpoll_targets need to be able to specify the default port
numbers.  Do this by specifying the default port numbers in struct
netpoll_targets directly.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 5c8701f..fbef723 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -68,6 +68,7 @@ __setup("netconsole=", option_setup);
 struct netpoll_targets {
 	struct list_head list;
 	spinlock_t lock;
+	u16 default_local_port, default_remote_port;
 #ifdef	CONFIG_NETCONSOLE_DYNAMIC
 	struct configfs_subsystem configfs_subsys;
 #endif
@@ -161,8 +162,8 @@ static struct netpoll_target *alloc_param_target(struct netpoll_targets *nts,
 	nt->nts = nts;
 	nt->np.name = nts->subsys_name;
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
-	nt->np.local_port = 6665;
-	nt->np.remote_port = 6666;
+	nt->np.local_port = nts->default_local_port;
+	nt->np.remote_port = nts->default_remote_port;
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
 
@@ -635,8 +636,8 @@ static struct config_item *make_netpoll_target(struct config_group *group,
 	nt->nts = nts;
 	nt->np.name = nts->subsys_name;
 	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
-	nt->np.local_port = 6665;
-	nt->np.remote_port = 6666;
+	nt->np.local_port = nts->default_local_port;
+	nt->np.remote_port = nts->default_remote_port;
 	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
 	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
 
@@ -926,6 +927,10 @@ static void __exit unregister_netpoll_targets(struct netpoll_targets *nts)
 static int __init init_netconsole(void)
 {
 	int err;
+
+	targets.default_local_port = 6665;
+	targets.default_remote_port = 6666;
+
 	err = register_netpoll_targets("netconsole", &targets, config);
 	if (err)
 		return err;

[PATCH v3 13/22] netpoll: Introduce netpoll_target configs

[Mike Waychison]

As preparation for moving netpoll_targets out of netconsole and making
them available to other clients, introduce new Kconfig options.

CONFIG_NETPOLL_TARGETS

Access to targets APIs.  Only supports parameter based parsing of
targets (via the kernel command line or module paramters).

CONFIG_NETPOLL_TARGETS_DYNAMIC

Extends the support that netpoll_targets provides by allowing for the
dynamic creation of targets in configfs on a per client basis.  This is
boolean flag used to enable dynamic support.  Specifying it without
also selecting NETPOLL_TARGETS is meaningless.

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>

---
Changelog:
- v3
  - Fixed to support building netpoll_targets as a module.  It depends
    on netpoll, but netpoll itself doesn't depend on it.
    netpoll_targets will be built as a module if it is needed by either
    NETCONSOLE or NETOOPS are enabled as modules (and neither are built
    in).  CONFIGFS_FS is also only selected to be a module if that is
    possible.
---
 drivers/net/Kconfig      |   15 ++++++++++++---
 drivers/net/netconsole.c |   10 +++++-----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 4f1755b..2ae9818 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3379,14 +3379,15 @@ config NET_FC
 
 config NETCONSOLE
 	tristate "Network console logging support"
+	select NETPOLL_TARGETS
 	---help---
 	If you want to log kernel messages over the network, enable this.
 	See <file:Documentation/networking/netconsole.txt> for details.
 
 config NETCONSOLE_DYNAMIC
 	bool "Dynamic reconfiguration of logging targets"
-	depends on NETCONSOLE && SYSFS
-	select CONFIGFS_FS
+	depends on NETCONSOLE
+	select NETPOLL_TARGETS_DYNAMIC
 	help
 	  This option enables the ability to dynamically reconfigure target
 	  parameters (interface, IP addresses, port numbers, MAC addresses)
@@ -3394,7 +3395,15 @@ config NETCONSOLE_DYNAMIC
 	  See <file:Documentation/networking/netconsole.txt> for details.
 
 config NETPOLL
-	def_bool NETCONSOLE
+	def_bool false
+
+config NETPOLL_TARGETS
+	tristate
+	select NETPOLL
+	select CONFIGFS_FS if NETPOLL_TARGETS_DYNAMIC
+
+config NETPOLL_TARGETS_DYNAMIC
+	bool
 
 config NETPOLL_TRAP
 	bool "Netpoll traffic trapping"
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index fbef723..1970be3 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -69,7 +69,7 @@ struct netpoll_targets {
 	struct list_head list;
 	spinlock_t lock;
 	u16 default_local_port, default_remote_port;
-#ifdef	CONFIG_NETCONSOLE_DYNAMIC
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
 	struct configfs_subsystem configfs_subsys;
 #endif
 	struct notifier_block netdev_notifier;
@@ -111,7 +111,7 @@ static DEFINE_NETPOLL_TARGETS(targets);
 struct netpoll_target {
 	struct netpoll_targets *nts;
 	struct list_head	list;
-#ifdef	CONFIG_NETCONSOLE_DYNAMIC
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
 	struct config_item	item;
 #endif
 	int			np_state;
@@ -194,7 +194,7 @@ static void free_param_target(struct netpoll_target *nt)
 	kfree(nt);
 }
 
-#ifdef	CONFIG_NETCONSOLE_DYNAMIC
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
 
 /*
  * Our subsystem hierarchy is:
@@ -724,7 +724,7 @@ static void netpoll_target_put(struct netpoll_target *nt)
 		config_item_put(&nt->item);
 }
 
-#else	/* !CONFIG_NETCONSOLE_DYNAMIC */
+#else	/* !CONFIG_NETPOLL_TARGETS_DYNAMIC */
 
 static int __init dynamic_netpoll_targets_init(const char *subsys_name,
 					       struct netpoll_targets *nts)
@@ -748,7 +748,7 @@ static void netpoll_target_put(struct netpoll_target *nt)
 {
 }
 
-#endif	/* CONFIG_NETCONSOLE_DYNAMIC */
+#endif	/* CONFIG_NETPOLL_TARGETS_DYNAMIC */
 
 /*
  * Call netpoll_cleanup on this target asynchronously.

[PATCH v3 14/22] netpoll: Move target code into netpoll_targets.c

[Mike Waychison]

Move all the target handling code out of netconsole.c and into a new
file, net/core/netpoll_targets.c.

In doing so, we have to remove the __exit and __init attributes on the
constructors/desctructors, as well as export the needed symbols to
modules.  Reference counting also becomes a no-op when dynamic targets
are disabled (!CONFIG_NETPOLL_TARGETS_DYNAMIC).

>From netpoll_targets.c's perspective, MAX_PARAM_LENGTH isn't available,
so we ensure that the string being passed in is zero-terminated and do
an open strlen() instead of strnlen().

Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
 drivers/net/netconsole.c        |  823 ---------------------------------------
 include/linux/netpoll_targets.h |   76 ++++
 net/core/Makefile               |    1 
 net/core/netpoll_targets.c      |  749 +++++++++++++++++++++++++++++++++++
 4 files changed, 828 insertions(+), 821 deletions(-)
 create mode 100644 include/linux/netpoll_targets.h
 create mode 100644 net/core/netpoll_targets.c

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 1970be3..64b0e4f 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -37,13 +37,11 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/module.h>
-#include <linux/slab.h>
 #include <linux/console.h>
 #include <linux/moduleparam.h>
-#include <linux/string.h>
 #include <linux/netpoll.h>
+#include <linux/netpoll_targets.h>
 #include <linux/inet.h>
-#include <linux/configfs.h>
 
 MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
 MODULE_DESCRIPTION("Console driver for network interfaces");
@@ -65,746 +63,8 @@ static int __init option_setup(char *opt)
 __setup("netconsole=", option_setup);
 #endif	/* MODULE */
 
-struct netpoll_targets {
-	struct list_head list;
-	spinlock_t lock;
-	u16 default_local_port, default_remote_port;
-#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
-	struct configfs_subsystem configfs_subsys;
-#endif
-	struct notifier_block netdev_notifier;
-	char *subsys_name;
-};
-#define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
-	{ .list = LIST_HEAD_INIT(x.list), \
-	  .lock = __SPIN_LOCK_UNLOCKED(x.lock) }
 static DEFINE_NETPOLL_TARGETS(targets);
 
-#define NETPOLL_DISABLED	0
-#define NETPOLL_SETTINGUP	1
-#define NETPOLL_ENABLED		2
-#define NETPOLL_CLEANING	3
-
-/**
- * struct netpoll_target - Represents a configured netpoll target.
- * @list:	Links this target into the netpoll_targets.list.
- * @item:	Links us into the configfs subsystem hierarchy.
- * @np_state:	Enabled / Disabled / SettingUp / Cleaning
- *		Visible from userspace (read-write) as "enabled".
- *		We maintain a state machine here of the valid states.  Either a
- *		target is enabled or disabled, but it may also be in a
- *		transitional state whereby nobody is allowed to act on the
- *		target other than whoever owns the transition.
- *
- *		Also, other parameters of a target may be modified at
- *		runtime only when it is disabled (np_state == NETPOLL_ENABLED).
- * @np:		The netpoll structure for this target.
- *		Contains the other userspace visible parameters:
- *		dev_name	(read-write)
- *		local_port	(read-write)
- *		remote_port	(read-write)
- *		local_ip	(read-write)
- *		remote_ip	(read-write)
- *		local_mac	(read-only)
- *		remote_mac	(read-write)
- */
-struct netpoll_target {
-	struct netpoll_targets *nts;
-	struct list_head	list;
-#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
-	struct config_item	item;
-#endif
-	int			np_state;
-	struct netpoll		np;
-	struct work_struct	cleanup_work;
-};
-
-static void netpoll_target_get(struct netpoll_target *nt);
-static void netpoll_target_put(struct netpoll_target *nt);
-
-static void deferred_netpoll_cleanup(struct work_struct *work)
-{
-	struct netpoll_target *nt;
-	struct netpoll_targets *nts;
-	unsigned long flags;
-
-	nt = container_of(work, struct netpoll_target, cleanup_work);
-	nts = nt->nts;
-
-	netpoll_cleanup(&nt->np);
-
-	spin_lock_irqsave(&nts->lock, flags);
-	BUG_ON(nt->np_state != NETPOLL_CLEANING);
-	nt->np_state = NETPOLL_DISABLED;
-	spin_unlock_irqrestore(&nts->lock, flags);
-
-	netpoll_target_put(nt);
-}
-
-/* Allocate new target (from boot/module param) and setup netpoll for it */
-static struct netpoll_target *alloc_param_target(struct netpoll_targets *nts,
-						 char *target_config)
-{
-	int err = -ENOMEM;
-	struct netpoll_target *nt;
-
-	/*
-	 * Allocate and initialize with defaults.
-	 * Note that these targets get their config_item fields zeroed-out.
-	 */
-	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
-	if (!nt) {
-		printk(KERN_ERR "%s: failed to allocate memory\n",
-		       nts->subsys_name);
-		goto fail;
-	}
-
-	nt->nts = nts;
-	nt->np.name = nts->subsys_name;
-	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
-	nt->np.local_port = nts->default_local_port;
-	nt->np.remote_port = nts->default_remote_port;
-	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
-	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
-
-	/* Parse parameters and setup netpoll */
-	err = netpoll_parse_options(&nt->np, target_config);
-	if (err)
-		goto fail;
-
-	err = netpoll_setup(&nt->np);
-	if (err)
-		goto fail;
-
-	nt->np_state = NETPOLL_ENABLED;
-
-	return nt;
-
-fail:
-	kfree(nt);
-	return ERR_PTR(err);
-}
-
-/* Cleanup netpoll for given target (from boot/module param) and free it */
-static void free_param_target(struct netpoll_target *nt)
-{
-	cancel_work_sync(&nt->cleanup_work);
-	if (nt->np_state == NETPOLL_CLEANING || nt->np_state == NETPOLL_ENABLED)
-		netpoll_cleanup(&nt->np);
-	kfree(nt);
-}
-
-#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
-
-/*
- * Our subsystem hierarchy is:
- *
- * /sys/kernel/config/<subsys_name>/
- *				|
- *				<target>/
- *				|	enabled
- *				|	dev_name
- *				|	local_port
- *				|	remote_port
- *				|	local_ip
- *				|	remote_ip
- *				|	local_mac
- *				|	remote_mac
- *				|
- *				<target>/...
- */
-
-struct netpoll_target_attr {
-	struct configfs_attribute	attr;
-	ssize_t				(*show)(struct netpoll_target *nt,
-						char *buf);
-	ssize_t				(*store)(struct netpoll_target *nt,
-						 const char *buf,
-						 size_t count);
-};
-
-static struct netpoll_target *to_target(struct config_item *item)
-{
-	return item ?
-		container_of(item, struct netpoll_target, item) :
-		NULL;
-}
-
-/*
- * Wrapper over simple_strtol (base 10) with sanity and range checking.
- * We return (signed) long only because we may want to return errors.
- * Do not use this to convert numbers that are allowed to be negative.
- */
-static long strtol10_check_range(struct netpoll_targets *nts,
-				 const char *cp, long min, long max)
-{
-	long ret;
-	char *p = (char *) cp;
-
-	WARN_ON(min < 0);
-	WARN_ON(max < min);
-
-	ret = simple_strtol(p, &p, 10);
-
-	if (*p && (*p != '\n')) {
-		printk(KERN_ERR "%s: invalid input\n", nts->subsys_name);
-		return -EINVAL;
-	}
-	if ((ret < min) || (ret > max)) {
-		printk(KERN_ERR "%s: input %ld must be between %ld and %ld\n",
-		       nts->subsys_name, ret, min, max);
-		return -EINVAL;
-	}
-
-	return ret;
-}
-
-/*
- * Attribute operations for netpoll_target.
- */
-
-static ssize_t show_enabled(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%d\n",
-			nt->np_state == NETPOLL_ENABLED);
-}
-
-static ssize_t show_dev_name(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%s\n", nt->np.dev_name);
-}
-
-static ssize_t show_local_port(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.local_port);
-}
-
-static ssize_t show_remote_port(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.remote_port);
-}
-
-static ssize_t show_local_ip(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.local_ip);
-}
-
-static ssize_t show_remote_ip(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.remote_ip);
-}
-
-static ssize_t show_local_mac(struct netpoll_target *nt, char *buf)
-{
-	struct net_device *dev = nt->np.dev;
-	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
-	return snprintf(buf, PAGE_SIZE, "%pM\n", dev ? dev->dev_addr : bcast);
-}
-
-static ssize_t show_remote_mac(struct netpoll_target *nt, char *buf)
-{
-	return snprintf(buf, PAGE_SIZE, "%pM\n", nt->np.remote_mac);
-}
-
-/*
- * This one is special -- targets created through the configfs interface
- * are not enabled (and the corresponding netpoll activated) by default.
- * The user is expected to set the desired parameters first (which
- * would enable him to dynamically add new netpoll targets for new
- * network interfaces as and when they come up).
- */
-static ssize_t store_enabled(struct netpoll_target *nt,
-			     const char *buf,
-			     size_t count)
-{
-	struct netpoll_targets *nts = nt->nts;
-	unsigned long flags;
-	int err;
-	long enabled;
-
-	enabled = strtol10_check_range(nts, buf, 0, 1);
-	if (enabled < 0)
-		return enabled;
-
-	if (enabled) {	/* 1 */
-		spin_lock_irqsave(&nts->lock, flags);
-		if (nt->np_state != NETPOLL_DISABLED)
-			goto busy;
-		else {
-			nt->np_state = NETPOLL_SETTINGUP;
-			/*
-			 * Nominally, we would grab an extra reference on the
-			 * config_item here for dynamic targets while we let go
-			 * of the lock, but this isn't required in this case
-			 * because there is a reference implicitly held by the
-			 * caller of the store operation.
-			 */
-			spin_unlock_irqrestore(&nts->lock, flags);
-		}
-
-		/*
-		 * Skip netpoll_parse_options() -- all the attributes are
-		 * already configured via configfs. Just print them out.
-		 */
-		netpoll_print_options(&nt->np);
-
-		err = netpoll_setup(&nt->np);
-		spin_lock_irqsave(&nts->lock, flags);
-		if (err)
-			nt->np_state = NETPOLL_DISABLED;
-		else
-			nt->np_state = NETPOLL_ENABLED;
-		spin_unlock_irqrestore(&nts->lock, flags);
-		if (err)
-			return err;
-
-		printk(KERN_INFO "%s: network logging started\n",
-		       nts->subsys_name);
-	} else {	/* 0 */
-		spin_lock_irqsave(&nts->lock, flags);
-		if (nt->np_state == NETPOLL_ENABLED)
-			nt->np_state = NETPOLL_CLEANING;
-		else if (nt->np_state != NETPOLL_DISABLED)
-			goto busy;
-		spin_unlock_irqrestore(&nts->lock, flags);
-
-		netpoll_cleanup(&nt->np);
-
-		spin_lock_irqsave(&nts->lock, flags);
-		nt->np_state = NETPOLL_DISABLED;
-		spin_unlock_irqrestore(&nts->lock, flags);
-	}
-
-	return strnlen(buf, count);
-busy:
-	spin_unlock_irqrestore(&nts->lock, flags);
-	return -EBUSY;
-}
-
-static ssize_t store_dev_name(struct netpoll_target *nt,
-			      const char *buf,
-			      size_t count)
-{
-	size_t len;
-
-	strlcpy(nt->np.dev_name, buf, IFNAMSIZ);
-
-	/* Get rid of possible trailing newline from echo(1) */
-	len = strnlen(nt->np.dev_name, IFNAMSIZ);
-	if (nt->np.dev_name[len - 1] == '\n')
-		nt->np.dev_name[len - 1] = '\0';
-
-	return strnlen(buf, count);
-}
-
-static ssize_t store_local_port(struct netpoll_target *nt,
-				const char *buf,
-				size_t count)
-{
-	long local_port;
-#define __U16_MAX	((__u16) ~0U)
-
-	local_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
-	if (local_port < 0)
-		return local_port;
-
-	nt->np.local_port = local_port;
-
-	return strnlen(buf, count);
-}
-
-static ssize_t store_remote_port(struct netpoll_target *nt,
-				 const char *buf,
-				 size_t count)
-{
-	long remote_port;
-#define __U16_MAX	((__u16) ~0U)
-
-	remote_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
-	if (remote_port < 0)
-		return remote_port;
-
-	nt->np.remote_port = remote_port;
-
-	return strnlen(buf, count);
-}
-
-static ssize_t store_local_ip(struct netpoll_target *nt,
-			      const char *buf,
-			      size_t count)
-{
-	nt->np.local_ip = in_aton(buf);
-
-	return strnlen(buf, count);
-}
-
-static ssize_t store_remote_ip(struct netpoll_target *nt,
-			       const char *buf,
-			       size_t count)
-{
-	nt->np.remote_ip = in_aton(buf);
-
-	return strnlen(buf, count);
-}
-
-static ssize_t store_remote_mac(struct netpoll_target *nt,
-				const char *buf,
-				size_t count)
-{
-	u8 remote_mac[ETH_ALEN];
-	char *p = (char *) buf;
-	int i;
-
-	for (i = 0; i < ETH_ALEN - 1; i++) {
-		remote_mac[i] = simple_strtoul(p, &p, 16);
-		if (*p != ':')
-			goto invalid;
-		p++;
-	}
-	remote_mac[ETH_ALEN - 1] = simple_strtoul(p, &p, 16);
-	if (*p && (*p != '\n'))
-		goto invalid;
-
-	memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN);
-
-	return strnlen(buf, count);
-
-invalid:
-	printk(KERN_ERR "%s: invalid input\n", nt->nts->subsys_name);
-	return -EINVAL;
-}
-
-/*
- * Attribute definitions for netpoll_target.
- */
-
-#define __NETPOLL_TARGET_ATTR_RO(_name, _prefix_...)			\
-static struct netpoll_target_attr netpoll_target_##_name =	\
-	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_prefix_##_name, NULL)
-
-#define __NETPOLL_TARGET_ATTR_RW(_name, _prefix_...)			\
-static struct netpoll_target_attr netpoll_target_##_name =	\
-	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR,			\
-			show_##_prefix_##_name, store_##_prefix_##_name)
-
-#define NETPOLL_WRAP_ATTR_STORE(_name)					\
-static ssize_t store_locked_##_name(struct netpoll_target *nt,		\
-				    const char *buf,			\
-				    size_t count)			\
-{									\
-	struct netpoll_targets *nts = nt->nts;				\
-	unsigned long flags;						\
-	ssize_t ret;							\
-	spin_lock_irqsave(&nts->lock, flags);				\
-	if (nt->np_state != NETPOLL_DISABLED) {				\
-		printk(KERN_ERR "%s: target (%s) is enabled, "		\
-				"disable to update parameters\n",	\
-				nts->subsys_name,			\
-				config_item_name(&nt->item));		\
-		spin_unlock_irqrestore(&nts->lock, flags);		\
-		return -EBUSY;						\
-	}								\
-	ret = store_##_name(nt, buf, count);				\
-	spin_unlock_irqrestore(&nts->lock, flags);			\
-	return ret;							\
-}
-
-#define NETPOLL_WRAP_ATTR_SHOW(_name)					\
-static ssize_t show_locked_##_name(struct netpoll_target *nt, char *buf) \
-{									\
-	struct netpoll_targets *nts = nt->nts;				\
-	unsigned long flags;						\
-	ssize_t ret;							\
-	spin_lock_irqsave(&nts->lock, flags);				\
-	ret = show_##_name(nt, buf);					\
-	spin_unlock_irqrestore(&nts->lock, flags);			\
-	return ret;							\
-}
-
-#define NETPOLL_TARGET_ATTR_RW(_name)				\
-		NETPOLL_WRAP_ATTR_STORE(_name)			\
-		NETPOLL_WRAP_ATTR_SHOW(_name)			\
-		__NETPOLL_TARGET_ATTR_RW(_name, locked_)
-
-#define NETPOLL_TARGET_ATTR_RO(_name)				\
-		NETPOLL_WRAP_ATTR_SHOW(_name)			\
-		__NETPOLL_TARGET_ATTR_RO(_name, locked_)
-
-__NETPOLL_TARGET_ATTR_RW(enabled);
-NETPOLL_TARGET_ATTR_RW(dev_name);
-NETPOLL_TARGET_ATTR_RW(local_port);
-NETPOLL_TARGET_ATTR_RW(remote_port);
-NETPOLL_TARGET_ATTR_RW(local_ip);
-NETPOLL_TARGET_ATTR_RW(remote_ip);
-NETPOLL_TARGET_ATTR_RO(local_mac);
-NETPOLL_TARGET_ATTR_RW(remote_mac);
-
-static struct configfs_attribute *netpoll_target_attrs[] = {
-	&netpoll_target_enabled.attr,
-	&netpoll_target_dev_name.attr,
-	&netpoll_target_local_port.attr,
-	&netpoll_target_remote_port.attr,
-	&netpoll_target_local_ip.attr,
-	&netpoll_target_remote_ip.attr,
-	&netpoll_target_local_mac.attr,
-	&netpoll_target_remote_mac.attr,
-	NULL,
-};
-
-/*
- * Item operations and type for netpoll_target.
- */
-
-static void netpoll_target_release(struct config_item *item)
-{
-	kfree(to_target(item));
-}
-
-static ssize_t netpoll_target_attr_show(struct config_item *item,
-					struct configfs_attribute *attr,
-					char *buf)
-{
-	ssize_t ret = -EINVAL;
-	struct netpoll_target *nt = to_target(item);
-	struct netpoll_target_attr *na =
-		container_of(attr, struct netpoll_target_attr, attr);
-
-	if (na->show)
-		ret = na->show(nt, buf);
-
-	return ret;
-}
-
-static ssize_t netpoll_target_attr_store(struct config_item *item,
-					 struct configfs_attribute *attr,
-					 const char *buf,
-					 size_t count)
-{
-	ssize_t ret = -EINVAL;
-	struct netpoll_target *nt = to_target(item);
-	struct netpoll_target_attr *na =
-		container_of(attr, struct netpoll_target_attr, attr);
-
-	if (na->store)
-		ret = na->store(nt, buf, count);
-
-	return ret;
-}
-
-static struct configfs_item_operations netpoll_target_item_ops = {
-	.release		= netpoll_target_release,
-	.show_attribute		= netpoll_target_attr_show,
-	.store_attribute	= netpoll_target_attr_store,
-};
-
-static struct config_item_type netpoll_target_type = {
-	.ct_attrs		= netpoll_target_attrs,
-	.ct_item_ops		= &netpoll_target_item_ops,
-	.ct_owner		= THIS_MODULE,
-};
-
-static struct netpoll_targets *group_to_targets(struct config_group *group)
-{
-	struct configfs_subsystem *subsys;
-	subsys = container_of(group, struct configfs_subsystem, su_group);
-	return container_of(subsys, struct netpoll_targets, configfs_subsys);
-}
-
-/*
- * Group operations and type for netpoll_target_subsys.
- */
-
-static struct config_item *make_netpoll_target(struct config_group *group,
-					       const char *name)
-{
-	struct netpoll_targets *nts = group_to_targets(group);
-	struct netpoll_target *nt;
-	unsigned long flags;
-
-	/*
-	 * Allocate and initialize with defaults.
-	 * Target is disabled at creation (enabled == 0).
-	 */
-	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
-	if (!nt) {
-		printk(KERN_ERR "%s: failed to allocate memory\n",
-		       nts->subsys_name);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	nt->nts = nts;
-	nt->np.name = nts->subsys_name;
-	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
-	nt->np.local_port = nts->default_local_port;
-	nt->np.remote_port = nts->default_remote_port;
-	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
-	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
-
-	/* Initialize the config_item member */
-	config_item_init_type_name(&nt->item, name, &netpoll_target_type);
-
-	/* Adding, but it is disabled */
-	spin_lock_irqsave(&nts->lock, flags);
-	list_add(&nt->list, &nts->list);
-	spin_unlock_irqrestore(&nts->lock, flags);
-
-	return &nt->item;
-}
-
-static void drop_netpoll_target(struct config_group *group,
-				struct config_item *item)
-{
-	struct netpoll_targets *nts = group_to_targets(group);
-	struct netpoll_target *nt = to_target(item);
-	unsigned long flags;
-
-	spin_lock_irqsave(&nts->lock, flags);
-	list_del(&nt->list);
-	spin_unlock_irqrestore(&nts->lock, flags);
-
-	/*
-	 * The target may have never been disabled, or was disabled due
-	 * to a netdev event, but we haven't had the chance to clean
-	 * things up yet.
-	 *
-	 * We can't wait for the target to be cleaned up by its
-	 * scheduled work however, as that work doesn't pin this module
-	 * in place.
-	 */
-	cancel_work_sync(&nt->cleanup_work);
-	if (nt->np_state == NETPOLL_ENABLED || nt->np_state == NETPOLL_CLEANING)
-		netpoll_cleanup(&nt->np);
-
-	netpoll_target_put(nt);
-}
-
-static struct configfs_group_operations netpoll_subsys_group_ops = {
-	.make_item	= make_netpoll_target,
-	.drop_item	= drop_netpoll_target,
-};
-
-static struct config_item_type netpoll_subsys_type = {
-	.ct_group_ops	= &netpoll_subsys_group_ops,
-	.ct_owner	= THIS_MODULE,
-};
-
-static int __init dynamic_netpoll_targets_init(const char *subsys_name,
-					       struct netpoll_targets *nts)
-{
-	struct configfs_subsystem *subsys = &nts->configfs_subsys;
-
-	config_group_init(&subsys->su_group);
-	mutex_init(&subsys->su_mutex);
-	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, subsys_name,
-		CONFIGFS_ITEM_NAME_LEN);
-	subsys->su_group.cg_item.ci_type = &netpoll_subsys_type;
-	return configfs_register_subsystem(subsys);
-}
-
-static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
-{
-	configfs_unregister_subsystem(&nts->configfs_subsys);
-}
-
-/*
- * Targets that were created by parsing the boot/module option string
- * do not exist in the configfs hierarchy (and have NULL names) and will
- * never go away, so make these a no-op for them.
- */
-static void netpoll_target_get(struct netpoll_target *nt)
-{
-	if (config_item_name(&nt->item))
-		config_item_get(&nt->item);
-}
-
-static void netpoll_target_put(struct netpoll_target *nt)
-{
-	if (config_item_name(&nt->item))
-		config_item_put(&nt->item);
-}
-
-#else	/* !CONFIG_NETPOLL_TARGETS_DYNAMIC */
-
-static int __init dynamic_netpoll_targets_init(const char *subsys_name,
-					       struct netpoll_targets *nts)
-{
-	return 0;
-}
-
-static void __exit dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
-{
-}
-
-/*
- * No danger of targets going away from under us when dynamic
- * reconfigurability is off.
- */
-static void netpoll_target_get(struct netpoll_target *nt)
-{
-}
-
-static void netpoll_target_put(struct netpoll_target *nt)
-{
-}
-
-#endif	/* CONFIG_NETPOLL_TARGETS_DYNAMIC */
-
-/*
- * Call netpoll_cleanup on this target asynchronously.
- * nts->lock is required.
- */
-static void defer_netpoll_cleanup(struct netpoll_target *nt)
-{
-	if (nt->np_state != NETPOLL_ENABLED)
-		return;
-	netpoll_target_get(nt);
-	nt->np_state = NETPOLL_CLEANING;
-	schedule_work(&nt->cleanup_work);
-}
-
-/* Handle network interface device notifications */
-static int netpoll_targets_netdev_event(struct notifier_block *this,
-					unsigned long event,
-					void *ptr)
-{
-	struct netpoll_targets *nts = container_of(this, struct netpoll_targets,
-						   netdev_notifier);
-	unsigned long flags;
-	struct netpoll_target *nt;
-	struct net_device *dev = ptr;
-
-	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
-	      event == NETDEV_BONDING_DESLAVE))
-		goto done;
-
-	spin_lock_irqsave(&nts->lock, flags);
-	list_for_each_entry(nt, &nts->list, list) {
-		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
-			switch (event) {
-			case NETDEV_CHANGENAME:
-				strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
-				break;
-			case NETDEV_BONDING_DESLAVE:
-			case NETDEV_UNREGISTER:
-				/*
-				 * We can't cleanup netpoll in atomic context.
-				 * Kick it off as deferred work.
-				 */
-				defer_netpoll_cleanup(nt);
-			}
-		}
-	}
-	spin_unlock_irqrestore(&nts->lock, flags);
-	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
-		printk(KERN_INFO "%s: network logging stopped, "
-			"interface %s %s\n", nts->subsys_name, dev->name,
-			event == NETDEV_UNREGISTER ? "unregistered" : "released slaves");
-
-done:
-	return NOTIFY_DONE;
-}
-
 static void write_msg(struct console *con, const char *msg, unsigned int len)
 {
 	int frag, left;
@@ -844,86 +104,6 @@ static struct console netconsole = {
 	.write	= write_msg,
 };
 
-static int __init register_netpoll_targets(const char *subsys_name,
-					   struct netpoll_targets *nts,
-					   char *static_targets)
-{
-	int err;
-	struct netpoll_target *nt, *tmp;
-	char *target_config;
-	char *input = static_targets;
-	unsigned long flags;
-
-	if (strnlen(input, MAX_PARAM_LENGTH)) {
-		while ((target_config = strsep(&input, ";"))) {
-			nt = alloc_param_target(nts, target_config);
-			if (IS_ERR(nt)) {
-				err = PTR_ERR(nt);
-				goto fail;
-			}
-
-			spin_lock_irqsave(&nts->lock, flags);
-			list_add(&nt->list, &nts->list);
-			spin_unlock_irqrestore(&nts->lock, flags);
-		}
-	}
-
-	nts->netdev_notifier.notifier_call = netpoll_targets_netdev_event;
-	err = register_netdevice_notifier(&nts->netdev_notifier);
-	if (err)
-		goto fail;
-
-	nts->subsys_name = kstrdup(subsys_name, GFP_KERNEL);
-	err = -ENOMEM;
-	if (!nts->subsys_name)
-		goto undonotifier;
-
-	err = dynamic_netpoll_targets_init(subsys_name, nts);
-	if (err)
-		goto free_subsys_name;
-
-	return 0;
-
-free_subsys_name:
-	kfree(nts->subsys_name);
-undonotifier:
-	unregister_netdevice_notifier(&nts->netdev_notifier);
-fail:
-	/*
-	 * Remove all targets and destroy them (only targets created
-	 * from the boot/module option exist here). Skipping the list
-	 * lock is safe here, and netpoll_cleanup() will sleep.
-	 */
-	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
-		list_del(&nt->list);
-		free_param_target(nt);
-	}
-
-	return err;
-}
-
-static void __exit unregister_netpoll_targets(struct netpoll_targets *nts)
-{
-	struct netpoll_target *nt, *tmp;
-
-	dynamic_netpoll_targets_exit(nts);
-	unregister_netdevice_notifier(&nts->netdev_notifier);
-
-	/*
-	 * Targets created via configfs pin references on our module
-	 * and would first be rmdir(2)'ed from userspace. We reach
-	 * here only when they are already destroyed, and only those
-	 * created from the boot/module option are left, so remove and
-	 * destroy them. Skipping the list lock is safe here, and
-	 * netpoll_cleanup() will sleep.
-	 */
-	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
-		list_del(&nt->list);
-		free_param_target(nt);
-	}
-	kfree(nts->subsys_name);
-}
-
 static int __init init_netconsole(void)
 {
 	int err;
@@ -931,6 +111,7 @@ static int __init init_netconsole(void)
 	targets.default_local_port = 6665;
 	targets.default_remote_port = 6666;
 
+	config[MAX_PARAM_LENGTH - 1] = '\0';
 	err = register_netpoll_targets("netconsole", &targets, config);
 	if (err)
 		return err;
diff --git a/include/linux/netpoll_targets.h b/include/linux/netpoll_targets.h
new file mode 100644
index 0000000..d08dde0
--- /dev/null
+++ b/include/linux/netpoll_targets.h
@@ -0,0 +1,76 @@
+#ifndef _LINUX_NETPOLL_TARGETS_H
+#define _LINUX_NETPOLL_TARGETS_H
+
+#include <linux/netpoll.h>
+
+#include <linux/configfs.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+struct netpoll_targets {
+	struct list_head list;
+	spinlock_t lock;
+	u16 default_local_port, default_remote_port;
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
+	struct configfs_subsystem configfs_subsys;
+#endif
+	struct notifier_block netdev_notifier;
+	char *subsys_name;
+};
+#define DEFINE_NETPOLL_TARGETS(x) struct netpoll_targets x = \
+	{ .list = LIST_HEAD_INIT(x.list), \
+	  .lock = __SPIN_LOCK_UNLOCKED(x.lock) }
+
+#define NETPOLL_DISABLED	0
+#define NETPOLL_SETTINGUP	1
+#define NETPOLL_ENABLED		2
+#define NETPOLL_CLEANING	3
+
+/**
+ * struct netpoll_target - Represents a configured netpoll target.
+ * @list:	Links this target into the netpoll_targets.list.
+ * @item:	Links us into the configfs subsystem hierarchy.
+ * @np_state:	Enabled / Disabled / SettingUp / Cleaning
+ *		Visible from userspace (read-write) as "enabled".
+ *		We maintain a state machine here of the valid states.  Either a
+ *		target is enabled or disabled, but it may also be in a
+ *		transitional state whereby nobody is allowed to act on the
+ *		target other than whoever owns the transition.
+ *
+ *		Also, other parameters of a target may be modified at
+ *		runtime only when it is disabled (np_state == NETPOLL_ENABLED).
+ * @np:		The netpoll structure for this target.
+ *		Contains the other userspace visible parameters:
+ *		dev_name	(read-write)
+ *		local_port	(read-write)
+ *		remote_port	(read-write)
+ *		local_ip	(read-write)
+ *		remote_ip	(read-write)
+ *		local_mac	(read-only)
+ *		remote_mac	(read-write)
+ */
+struct netpoll_target {
+	struct netpoll_targets *nts;
+	struct list_head	list;
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
+	struct config_item	item;
+#endif
+	int			np_state;
+	struct netpoll		np;
+	struct work_struct	cleanup_work;
+};
+
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
+void netpoll_target_get(struct netpoll_target *nt);
+void netpoll_target_put(struct netpoll_target *nt);
+#else
+static void netpoll_target_get(struct netpoll_target *nt) {}
+static void netpoll_target_put(struct netpoll_target *nt) {}
+#endif
+
+int register_netpoll_targets(const char *subsys_name,
+			     struct netpoll_targets *nts,
+			     char *static_targets);
+void unregister_netpoll_targets(struct netpoll_targets *nts);
+
+#endif /* _LINUX_NETPOLL_TARGETS_H */
diff --git a/net/core/Makefile b/net/core/Makefile
index 8a04dd2..262217c 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_XFRM) += flow.o
 obj-y += net-sysfs.o
 obj-$(CONFIG_NET_PKTGEN) += pktgen.o
 obj-$(CONFIG_NETPOLL) += netpoll.o
+obj-$(CONFIG_NETPOLL_TARGETS) += netpoll_targets.o
 obj-$(CONFIG_NET_DMA) += user_dma.o
 obj-$(CONFIG_FIB_RULES) += fib_rules.o
 obj-$(CONFIG_TRACEPOINTS) += net-traces.o
diff --git a/net/core/netpoll_targets.c b/net/core/netpoll_targets.c
new file mode 100644
index 0000000..5fc84f8c
--- /dev/null
+++ b/net/core/netpoll_targets.c
@@ -0,0 +1,749 @@
+#include <linux/netpoll_targets.h>
+
+static void deferred_netpoll_cleanup(struct work_struct *work)
+{
+	struct netpoll_target *nt;
+	struct netpoll_targets *nts;
+	unsigned long flags;
+
+	nt = container_of(work, struct netpoll_target, cleanup_work);
+	nts = nt->nts;
+
+	netpoll_cleanup(&nt->np);
+
+	spin_lock_irqsave(&nts->lock, flags);
+	BUG_ON(nt->np_state != NETPOLL_CLEANING);
+	nt->np_state = NETPOLL_DISABLED;
+	spin_unlock_irqrestore(&nts->lock, flags);
+
+	netpoll_target_put(nt);
+}
+
+/* Allocate new target (from boot/module param) and setup netpoll for it */
+static struct netpoll_target *alloc_param_target(struct netpoll_targets *nts,
+						 char *target_config)
+{
+	int err = -ENOMEM;
+	struct netpoll_target *nt;
+
+	/*
+	 * Allocate and initialize with defaults.
+	 * Note that these targets get their config_item fields zeroed-out.
+	 */
+	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+	if (!nt) {
+		printk(KERN_ERR "%s: failed to allocate memory\n",
+		       nts->subsys_name);
+		goto fail;
+	}
+
+	nt->nts = nts;
+	nt->np.name = nts->subsys_name;
+	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
+	nt->np.local_port = nts->default_local_port;
+	nt->np.remote_port = nts->default_remote_port;
+	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
+
+	/* Parse parameters and setup netpoll */
+	err = netpoll_parse_options(&nt->np, target_config);
+	if (err)
+		goto fail;
+
+	err = netpoll_setup(&nt->np);
+	if (err)
+		goto fail;
+
+	nt->np_state = NETPOLL_ENABLED;
+
+	return nt;
+
+fail:
+	kfree(nt);
+	return ERR_PTR(err);
+}
+
+/* Cleanup netpoll for given target (from boot/module param) and free it */
+static void free_param_target(struct netpoll_target *nt)
+{
+	cancel_work_sync(&nt->cleanup_work);
+	if (nt->np_state == NETPOLL_CLEANING || nt->np_state == NETPOLL_ENABLED)
+		netpoll_cleanup(&nt->np);
+	kfree(nt);
+}
+
+#ifdef	CONFIG_NETPOLL_TARGETS_DYNAMIC
+
+/*
+ * Our subsystem hierarchy is:
+ *
+ * /sys/kernel/config/<subsys_name>/
+ *				|
+ *				<target>/
+ *				|	enabled
+ *				|	dev_name
+ *				|	local_port
+ *				|	remote_port
+ *				|	local_ip
+ *				|	remote_ip
+ *				|	local_mac
+ *				|	remote_mac
+ *				|
+ *				<target>/...
+ */
+
+struct netpoll_target_attr {
+	struct configfs_attribute	attr;
+	ssize_t				(*show)(struct netpoll_target *nt,
+						char *buf);
+	ssize_t				(*store)(struct netpoll_target *nt,
+						 const char *buf,
+						 size_t count);
+};
+
+static struct netpoll_target *to_target(struct config_item *item)
+{
+	return item ?
+		container_of(item, struct netpoll_target, item) :
+		NULL;
+}
+
+/*
+ * Wrapper over simple_strtol (base 10) with sanity and range checking.
+ * We return (signed) long only because we may want to return errors.
+ * Do not use this to convert numbers that are allowed to be negative.
+ */
+static long strtol10_check_range(struct netpoll_targets *nts,
+				 const char *cp, long min, long max)
+{
+	long ret;
+	char *p = (char *) cp;
+
+	WARN_ON(min < 0);
+	WARN_ON(max < min);
+
+	ret = simple_strtol(p, &p, 10);
+
+	if (*p && (*p != '\n')) {
+		printk(KERN_ERR "%s: invalid input\n", nts->subsys_name);
+		return -EINVAL;
+	}
+	if ((ret < min) || (ret > max)) {
+		printk(KERN_ERR "%s: input %ld must be between %ld and %ld\n",
+		       nts->subsys_name, ret, min, max);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/*
+ * Attribute operations for netpoll_target.
+ */
+
+static ssize_t show_enabled(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			nt->np_state == NETPOLL_ENABLED);
+}
+
+static ssize_t show_dev_name(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n", nt->np.dev_name);
+}
+
+static ssize_t show_local_port(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.local_port);
+}
+
+static ssize_t show_remote_port(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", nt->np.remote_port);
+}
+
+static ssize_t show_local_ip(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.local_ip);
+}
+
+static ssize_t show_remote_ip(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.remote_ip);
+}
+
+static ssize_t show_local_mac(struct netpoll_target *nt, char *buf)
+{
+	struct net_device *dev = nt->np.dev;
+	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+
+	return snprintf(buf, PAGE_SIZE, "%pM\n", dev ? dev->dev_addr : bcast);
+}
+
+static ssize_t show_remote_mac(struct netpoll_target *nt, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%pM\n", nt->np.remote_mac);
+}
+
+/*
+ * This one is special -- targets created through the configfs interface
+ * are not enabled (and the corresponding netpoll activated) by default.
+ * The user is expected to set the desired parameters first (which
+ * would enable him to dynamically add new netpoll targets for new
+ * network interfaces as and when they come up).
+ */
+static ssize_t store_enabled(struct netpoll_target *nt,
+			     const char *buf,
+			     size_t count)
+{
+	struct netpoll_targets *nts = nt->nts;
+	unsigned long flags;
+	int err;
+	long enabled;
+
+	enabled = strtol10_check_range(nts, buf, 0, 1);
+	if (enabled < 0)
+		return enabled;
+
+	if (enabled) {	/* 1 */
+		spin_lock_irqsave(&nts->lock, flags);
+		if (nt->np_state != NETPOLL_DISABLED)
+			goto busy;
+		else {
+			nt->np_state = NETPOLL_SETTINGUP;
+			/*
+			 * Nominally, we would grab an extra reference on the
+			 * config_item here for dynamic targets while we let go
+			 * of the lock, but this isn't required in this case
+			 * because there is a reference implicitly held by the
+			 * caller of the store operation.
+			 */
+			spin_unlock_irqrestore(&nts->lock, flags);
+		}
+
+		/*
+		 * Skip netpoll_parse_options() -- all the attributes are
+		 * already configured via configfs. Just print them out.
+		 */
+		netpoll_print_options(&nt->np);
+
+		err = netpoll_setup(&nt->np);
+		spin_lock_irqsave(&nts->lock, flags);
+		if (err)
+			nt->np_state = NETPOLL_DISABLED;
+		else
+			nt->np_state = NETPOLL_ENABLED;
+		spin_unlock_irqrestore(&nts->lock, flags);
+		if (err)
+			return err;
+
+		printk(KERN_INFO "%s: network logging started\n",
+		       nts->subsys_name);
+	} else {	/* 0 */
+		spin_lock_irqsave(&nts->lock, flags);
+		if (nt->np_state == NETPOLL_ENABLED)
+			nt->np_state = NETPOLL_CLEANING;
+		else if (nt->np_state != NETPOLL_DISABLED)
+			goto busy;
+		spin_unlock_irqrestore(&nts->lock, flags);
+
+		netpoll_cleanup(&nt->np);
+
+		spin_lock_irqsave(&nts->lock, flags);
+		nt->np_state = NETPOLL_DISABLED;
+		spin_unlock_irqrestore(&nts->lock, flags);
+	}
+
+	return strnlen(buf, count);
+busy:
+	spin_unlock_irqrestore(&nts->lock, flags);
+	return -EBUSY;
+}
+
+static ssize_t store_dev_name(struct netpoll_target *nt,
+			      const char *buf,
+			      size_t count)
+{
+	size_t len;
+
+	strlcpy(nt->np.dev_name, buf, IFNAMSIZ);
+
+	/* Get rid of possible trailing newline from echo(1) */
+	len = strnlen(nt->np.dev_name, IFNAMSIZ);
+	if (nt->np.dev_name[len - 1] == '\n')
+		nt->np.dev_name[len - 1] = '\0';
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_local_port(struct netpoll_target *nt,
+				const char *buf,
+				size_t count)
+{
+	long local_port;
+#define __U16_MAX	((__u16) ~0U)
+
+	local_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
+	if (local_port < 0)
+		return local_port;
+
+	nt->np.local_port = local_port;
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_remote_port(struct netpoll_target *nt,
+				 const char *buf,
+				 size_t count)
+{
+	long remote_port;
+#define __U16_MAX	((__u16) ~0U)
+
+	remote_port = strtol10_check_range(nt->nts, buf, 0, __U16_MAX);
+	if (remote_port < 0)
+		return remote_port;
+
+	nt->np.remote_port = remote_port;
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_local_ip(struct netpoll_target *nt,
+			      const char *buf,
+			      size_t count)
+{
+	nt->np.local_ip = in_aton(buf);
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_remote_ip(struct netpoll_target *nt,
+			       const char *buf,
+			       size_t count)
+{
+	nt->np.remote_ip = in_aton(buf);
+
+	return strnlen(buf, count);
+}
+
+static ssize_t store_remote_mac(struct netpoll_target *nt,
+				const char *buf,
+				size_t count)
+{
+	u8 remote_mac[ETH_ALEN];
+	char *p = (char *) buf;
+	int i;
+
+	for (i = 0; i < ETH_ALEN - 1; i++) {
+		remote_mac[i] = simple_strtoul(p, &p, 16);
+		if (*p != ':')
+			goto invalid;
+		p++;
+	}
+	remote_mac[ETH_ALEN - 1] = simple_strtoul(p, &p, 16);
+	if (*p && (*p != '\n'))
+		goto invalid;
+
+	memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN);
+
+	return strnlen(buf, count);
+
+invalid:
+	printk(KERN_ERR "%s: invalid input\n", nt->nts->subsys_name);
+	return -EINVAL;
+}
+
+/*
+ * Attribute definitions for netpoll_target.
+ */
+
+#define __NETPOLL_TARGET_ATTR_RO(_name, _prefix_...)			\
+static struct netpoll_target_attr netpoll_target_##_name =	\
+	__CONFIGFS_ATTR(_name, S_IRUGO, show_##_prefix_##_name, NULL)
+
+#define __NETPOLL_TARGET_ATTR_RW(_name, _prefix_...)			\
+static struct netpoll_target_attr netpoll_target_##_name =	\
+	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR,			\
+			show_##_prefix_##_name, store_##_prefix_##_name)
+
+#define NETPOLL_WRAP_ATTR_STORE(_name)					\
+static ssize_t store_locked_##_name(struct netpoll_target *nt,		\
+				    const char *buf,			\
+				    size_t count)			\
+{									\
+	struct netpoll_targets *nts = nt->nts;				\
+	unsigned long flags;						\
+	ssize_t ret;							\
+	spin_lock_irqsave(&nts->lock, flags);				\
+	if (nt->np_state != NETPOLL_DISABLED) {				\
+		printk(KERN_ERR "%s: target (%s) is enabled, "		\
+				"disable to update parameters\n",	\
+				nts->subsys_name,			\
+				config_item_name(&nt->item));		\
+		spin_unlock_irqrestore(&nts->lock, flags);		\
+		return -EBUSY;						\
+	}								\
+	ret = store_##_name(nt, buf, count);				\
+	spin_unlock_irqrestore(&nts->lock, flags);			\
+	return ret;							\
+}
+
+#define NETPOLL_WRAP_ATTR_SHOW(_name)					\
+static ssize_t show_locked_##_name(struct netpoll_target *nt, char *buf) \
+{									\
+	struct netpoll_targets *nts = nt->nts;				\
+	unsigned long flags;						\
+	ssize_t ret;							\
+	spin_lock_irqsave(&nts->lock, flags);				\
+	ret = show_##_name(nt, buf);					\
+	spin_unlock_irqrestore(&nts->lock, flags);			\
+	return ret;							\
+}
+
+#define NETPOLL_TARGET_ATTR_RW(_name)				\
+		NETPOLL_WRAP_ATTR_STORE(_name)			\
+		NETPOLL_WRAP_ATTR_SHOW(_name)			\
+		__NETPOLL_TARGET_ATTR_RW(_name, locked_)
+
+#define NETPOLL_TARGET_ATTR_RO(_name)				\
+		NETPOLL_WRAP_ATTR_SHOW(_name)			\
+		__NETPOLL_TARGET_ATTR_RO(_name, locked_)
+
+__NETPOLL_TARGET_ATTR_RW(enabled);
+NETPOLL_TARGET_ATTR_RW(dev_name);
+NETPOLL_TARGET_ATTR_RW(local_port);
+NETPOLL_TARGET_ATTR_RW(remote_port);
+NETPOLL_TARGET_ATTR_RW(local_ip);
+NETPOLL_TARGET_ATTR_RW(remote_ip);
+NETPOLL_TARGET_ATTR_RO(local_mac);
+NETPOLL_TARGET_ATTR_RW(remote_mac);
+
+static struct configfs_attribute *netpoll_target_attrs[] = {
+	&netpoll_target_enabled.attr,
+	&netpoll_target_dev_name.attr,
+	&netpoll_target_local_port.attr,
+	&netpoll_target_remote_port.attr,
+	&netpoll_target_local_ip.attr,
+	&netpoll_target_remote_ip.attr,
+	&netpoll_target_local_mac.attr,
+	&netpoll_target_remote_mac.attr,
+	NULL,
+};
+
+/*
+ * Item operations and type for netpoll_target.
+ */
+
+static void netpoll_target_release(struct config_item *item)
+{
+	kfree(to_target(item));
+}
+
+static ssize_t netpoll_target_attr_show(struct config_item *item,
+					struct configfs_attribute *attr,
+					char *buf)
+{
+	ssize_t ret = -EINVAL;
+	struct netpoll_target *nt = to_target(item);
+	struct netpoll_target_attr *na =
+		container_of(attr, struct netpoll_target_attr, attr);
+
+	if (na->show)
+		ret = na->show(nt, buf);
+
+	return ret;
+}
+
+static ssize_t netpoll_target_attr_store(struct config_item *item,
+					 struct configfs_attribute *attr,
+					 const char *buf,
+					 size_t count)
+{
+	ssize_t ret = -EINVAL;
+	struct netpoll_target *nt = to_target(item);
+	struct netpoll_target_attr *na =
+		container_of(attr, struct netpoll_target_attr, attr);
+
+	if (na->store)
+		ret = na->store(nt, buf, count);
+
+	return ret;
+}
+
+static struct configfs_item_operations netpoll_target_item_ops = {
+	.release		= netpoll_target_release,
+	.show_attribute		= netpoll_target_attr_show,
+	.store_attribute	= netpoll_target_attr_store,
+};
+
+static struct config_item_type netpoll_target_type = {
+	.ct_attrs		= netpoll_target_attrs,
+	.ct_item_ops		= &netpoll_target_item_ops,
+	.ct_owner		= THIS_MODULE,
+};
+
+static struct netpoll_targets *group_to_targets(struct config_group *group)
+{
+	struct configfs_subsystem *subsys;
+	subsys = container_of(group, struct configfs_subsystem, su_group);
+	return container_of(subsys, struct netpoll_targets, configfs_subsys);
+}
+
+/*
+ * Group operations and type for netpoll_target_subsys.
+ */
+
+static struct config_item *make_netpoll_target(struct config_group *group,
+					       const char *name)
+{
+	struct netpoll_targets *nts = group_to_targets(group);
+	struct netpoll_target *nt;
+	unsigned long flags;
+
+	/*
+	 * Allocate and initialize with defaults.
+	 * Target is disabled at creation (enabled == 0).
+	 */
+	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+	if (!nt) {
+		printk(KERN_ERR "%s: failed to allocate memory\n",
+		       nts->subsys_name);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	nt->nts = nts;
+	nt->np.name = nts->subsys_name;
+	strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
+	nt->np.local_port = nts->default_local_port;
+	nt->np.remote_port = nts->default_remote_port;
+	memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+	INIT_WORK(&nt->cleanup_work, deferred_netpoll_cleanup);
+
+	/* Initialize the config_item member */
+	config_item_init_type_name(&nt->item, name, &netpoll_target_type);
+
+	/* Adding, but it is disabled */
+	spin_lock_irqsave(&nts->lock, flags);
+	list_add(&nt->list, &nts->list);
+	spin_unlock_irqrestore(&nts->lock, flags);
+
+	return &nt->item;
+}
+
+static void drop_netpoll_target(struct config_group *group,
+				struct config_item *item)
+{
+	struct netpoll_targets *nts = group_to_targets(group);
+	struct netpoll_target *nt = to_target(item);
+	unsigned long flags;
+
+	spin_lock_irqsave(&nts->lock, flags);
+	list_del(&nt->list);
+	spin_unlock_irqrestore(&nts->lock, flags);
+
+	/*
+	 * The target may have never been disabled, or was disabled due
+	 * to a netdev event, but we haven't had the chance to clean
+	 * things up yet.
+	 *
+	 * We can't wait for the target to be cleaned up by its
+	 * scheduled work however, as that work doesn't pin this module
+	 * in place.
+	 */
+	cancel_work_sync(&nt->cleanup_work);
+	if (nt->np_state == NETPOLL_ENABLED || nt->np_state == NETPOLL_CLEANING)
+		netpoll_cleanup(&nt->np);
+
+	netpoll_target_put(nt);
+}
+
+static struct configfs_group_operations netpoll_subsys_group_ops = {
+	.make_item	= make_netpoll_target,
+	.drop_item	= drop_netpoll_target,
+};
+
+static struct config_item_type netpoll_subsys_type = {
+	.ct_group_ops	= &netpoll_subsys_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static int dynamic_netpoll_targets_init(const char *subsys_name,
+					struct netpoll_targets *nts)
+{
+	struct configfs_subsystem *subsys = &nts->configfs_subsys;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, subsys_name,
+		CONFIGFS_ITEM_NAME_LEN);
+	subsys->su_group.cg_item.ci_type = &netpoll_subsys_type;
+	return configfs_register_subsystem(subsys);
+}
+
+static void dynamic_netpoll_targets_exit(struct netpoll_targets *nts)
+{
+	configfs_unregister_subsystem(&nts->configfs_subsys);
+}
+
+/*
+ * Targets that were created by parsing the boot/module option string
+ * do not exist in the configfs hierarchy (and have NULL names) and will
+ * never go away, so make these a no-op for them.
+ */
+void netpoll_target_get(struct netpoll_target *nt)
+{
+	if (config_item_name(&nt->item))
+		config_item_get(&nt->item);
+}
+EXPORT_SYMBOL_GPL(netpoll_target_get);
+
+void netpoll_target_put(struct netpoll_target *nt)
+{
+	if (config_item_name(&nt->item))
+		config_item_put(&nt->item);
+}
+EXPORT_SYMBOL_GPL(netpoll_target_put);
+
+#else	/* CONFIG_NETPOLL_TARGETS_DYNAMIC */
+static int dynamic_netpoll_targets_init(const char *subsys_name,
+					struct netpoll_targets *nts) {}
+static int dynamic_netpoll_targets_exit(struct netpoll_targets *nts) {}
+#endif	/* CONFIG_NETPOLL_TARGETS_DYNAMIC */
+
+/*
+ * Call netpoll_cleanup on this target asynchronously.
+ * nts->lock is required.
+ */
+static void defer_netpoll_cleanup(struct netpoll_target *nt)
+{
+	if (nt->np_state != NETPOLL_ENABLED)
+		return;
+	netpoll_target_get(nt);
+	nt->np_state = NETPOLL_CLEANING;
+	schedule_work(&nt->cleanup_work);
+}
+
+/* Handle network interface device notifications */
+static int netpoll_targets_netdev_event(struct notifier_block *this,
+					unsigned long event,
+					void *ptr)
+{
+	struct netpoll_targets *nts = container_of(this, struct netpoll_targets,
+						   netdev_notifier);
+	unsigned long flags;
+	struct netpoll_target *nt;
+	struct net_device *dev = ptr;
+
+	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
+	      event == NETDEV_BONDING_DESLAVE))
+		goto done;
+
+	spin_lock_irqsave(&nts->lock, flags);
+	list_for_each_entry(nt, &nts->list, list) {
+		if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
+			switch (event) {
+			case NETDEV_CHANGENAME:
+				strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
+				break;
+			case NETDEV_BONDING_DESLAVE:
+			case NETDEV_UNREGISTER:
+				/*
+				 * We can't cleanup netpoll in atomic context.
+				 * Kick it off as deferred work.
+				 */
+				defer_netpoll_cleanup(nt);
+			}
+		}
+	}
+	spin_unlock_irqrestore(&nts->lock, flags);
+	if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
+		printk(KERN_INFO "%s: network logging stopped, "
+			"interface %s %s\n", nts->subsys_name, dev->name,
+			event == NETDEV_UNREGISTER ? "unregistered" : "released slaves");
+
+done:
+	return NOTIFY_DONE;
+}
+
+int register_netpoll_targets(const char *subsys_name,
+			     struct netpoll_targets *nts,
+			     char *static_targets)
+{
+	int err;
+	struct netpoll_target *nt, *tmp;
+	char *target_config;
+	char *input = static_targets;
+	unsigned long flags;
+
+	if (strlen(input)) {
+		while ((target_config = strsep(&input, ";"))) {
+			nt = alloc_param_target(nts, target_config);
+			if (IS_ERR(nt)) {
+				err = PTR_ERR(nt);
+				goto fail;
+			}
+
+			spin_lock_irqsave(&nts->lock, flags);
+			list_add(&nt->list, &nts->list);
+			spin_unlock_irqrestore(&nts->lock, flags);
+		}
+	}
+
+	nts->netdev_notifier.notifier_call = netpoll_targets_netdev_event;
+	err = register_netdevice_notifier(&nts->netdev_notifier);
+	if (err)
+		goto fail;
+
+	nts->subsys_name = kstrdup(subsys_name, GFP_KERNEL);
+	err = -ENOMEM;
+	if (!nts->subsys_name)
+		goto undonotifier;
+
+	err = dynamic_netpoll_targets_init(subsys_name, nts);
+	if (err)
+		goto free_subsys_name;
+
+	return 0;
+
+free_subsys_name:
+	kfree(nts->subsys_name);
+undonotifier:
+	unregister_netdevice_notifier(&nts->netdev_notifier);
+fail:
+	/*
+	 * Remove all targets and destroy them (only targets created
+	 * from the boot/module option exist here). Skipping the list
+	 * lock is safe here, and netpoll_cleanup() will sleep.
+	 */
+	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
+		list_del(&nt->list);
+		free_param_target(nt);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(register_netpoll_targets);
+
+void unregister_netpoll_targets(struct netpoll_targets *nts)
+{
+	struct netpoll_target *nt, *tmp;
+
+	dynamic_netpoll_targets_exit(nts);
+	unregister_netdevice_notifier(&nts->netdev_notifier);
+
+	/*
+	 * Targets created via configfs pin references on our module
+	 * and would first be rmdir(2)'ed from userspace. We reach
+	 * here only when they are already destroyed, and only those
+	 * created from the boot/module option are left, so remove and
+	 * destroy them. Skipping the list lock is safe here, and
+	 * netpoll_cleanup() will sleep.
+	 */
+	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
+		list_del(&nt->list);
+		free_param_target(nt);
+	}
+	kfree(nts->subsys_name);
+}
+EXPORT_SYMBOL_GPL(unregister_netpoll_targets);
+

Re: [PATCH v3 14/22] netpoll: Move target code into netpoll_targets.c

[Mike Waychison]

Ugh.  I just realized that I was missing a MODULE_LICENSE("GPL"); for
this new module, otherwise module load fails to find the link for
cancel_work_sync.

---
netpoll: Add missing module license for netpoll_targets

Add a missing module license declaration to netpoll_targets that allows
it to use exported symbols (cancel_work_sync).

Signed-off-by: Mike Waychison <mikew@google.com>

diff --git a/net/core/netpoll_targets.c b/net/core/netpoll_targets.c
index 5fc84f8c..0887268 100644
--- a/net/core/netpoll_targets.c
+++ b/net/core/netpoll_targets.c
@@ -747,3 +747,4 @@ void unregister_netpoll_targets(struct netpoll_targets *nts)
 }
 EXPORT_SYMBOL_GPL(unregister_netpoll_targets);

+MODULE_LICENSE("GPL");

[PATCH v3 16/22] kmsg_dumper: Pass pt_regs along to dumpers.

[Mike Waychison]

A subsequent commit adding netoops functionality would like access to the
pt_regs to have them recorded in the dump.  Pass the register values along if
available.

Signed-off-by: Mike Waychison <mikew@google.com>
---
 drivers/char/ramoops.c    |    4 +++-
 drivers/mtd/mtdoops.c     |    4 +++-
 include/linux/kmsg_dump.h |    8 ++++++--
 kernel/kexec.c            |    5 +++--
 kernel/panic.c            |    4 ++--
 kernel/printk.c           |    4 ++--
 6 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
index 73dcb0e..9d79492 100644
--- a/drivers/char/ramoops.c
+++ b/drivers/char/ramoops.c
@@ -58,7 +58,9 @@ static struct ramoops_context {
 } oops_cxt;
 
 static void ramoops_do_dump(struct kmsg_dumper *dumper,
-		enum kmsg_dump_reason reason, const char *s1, unsigned long l1,
+		enum kmsg_dump_reason reason,
+		struct pt_regs *pt_regs,
+		const char *s1, unsigned long l1,
 		const char *s2, unsigned long l2)
 {
 	struct ramoops_context *cxt = container_of(dumper,
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 1ee72f3..cf07cf5 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -298,7 +298,9 @@ static void find_next_position(struct mtdoops_context *cxt)
 }
 
 static void mtdoops_do_dump(struct kmsg_dumper *dumper,
-		enum kmsg_dump_reason reason, const char *s1, unsigned long l1,
+		enum kmsg_dump_reason reason,
+		struct pt_regs *pt_regs,
+		const char *s1, unsigned long l1,
 		const char *s2, unsigned long l2)
 {
 	struct mtdoops_context *cxt = container_of(dumper,
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 24b4414..a229acc 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -14,6 +14,8 @@
 
 #include <linux/list.h>
 
+struct pt_regs;
+
 enum kmsg_dump_reason {
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_PANIC,
@@ -30,6 +32,7 @@ enum kmsg_dump_reason {
  */
 struct kmsg_dumper {
 	void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
+			struct pt_regs *pt_regs,
 			const char *s1, unsigned long l1,
 			const char *s2, unsigned long l2);
 	struct list_head list;
@@ -37,13 +40,14 @@ struct kmsg_dumper {
 };
 
 #ifdef CONFIG_PRINTK
-void kmsg_dump(enum kmsg_dump_reason reason);
+void kmsg_dump(enum kmsg_dump_reason reason, struct pt_regs *pt_regs);
 
 int kmsg_dump_register(struct kmsg_dumper *dumper);
 
 int kmsg_dump_unregister(struct kmsg_dumper *dumper);
 #else
-static inline void kmsg_dump(enum kmsg_dump_reason reason)
+static inline void kmsg_dump(enum kmsg_dump_reason reason,
+			     struct pt_regs *pt_regs)
 {
 }
 
diff --git a/kernel/kexec.c b/kernel/kexec.c
index b55045b..840af0c 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1078,10 +1078,11 @@ void crash_kexec(struct pt_regs *regs)
 		if (kexec_crash_image) {
 			struct pt_regs fixed_regs;
 
-			kmsg_dump(KMSG_DUMP_KEXEC);
-
 			crash_setup_regs(&fixed_regs, regs);
 			crash_save_vmcoreinfo();
+
+			kmsg_dump(KMSG_DUMP_KEXEC, &fixed_regs);
+
 			machine_crash_shutdown(&fixed_regs);
 			machine_kexec(kexec_crash_image);
 		}
diff --git a/kernel/panic.c b/kernel/panic.c
index c3f39cd..b3a4440 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -87,7 +87,7 @@ NORET_TYPE void panic(const char * fmt, ...)
 	 */
 	crash_kexec(NULL);
 
-	kmsg_dump(KMSG_DUMP_PANIC);
+	kmsg_dump(KMSG_DUMP_PANIC, NULL);
 
 	/*
 	 * Note smp_send_stop is the usual smp shutdown function, which
@@ -353,7 +353,7 @@ void oops_exit(struct pt_regs *regs)
 {
 	do_oops_enter_exit();
 	print_oops_end_marker();
-	kmsg_dump(KMSG_DUMP_OOPS);
+	kmsg_dump(KMSG_DUMP_OOPS, regs);
 }
 
 #ifdef WANT_WARN_ON_SLOWPATH
diff --git a/kernel/printk.c b/kernel/printk.c
index 9a2264f..6d4d8c5 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1552,7 +1552,7 @@ static const char *kmsg_to_str(enum kmsg_dump_reason reason)
  * Iterate through each of the dump devices and call the oops/panic
  * callbacks with the log buffer.
  */
-void kmsg_dump(enum kmsg_dump_reason reason)
+void kmsg_dump(enum kmsg_dump_reason reason, struct pt_regs *pt_regs)
 {
 	unsigned long end;
 	unsigned chars;
@@ -1589,7 +1589,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
 		return;
 	}
 	list_for_each_entry(dumper, &dump_list, list)
-		dumper->dump(dumper, reason, s1, l1, s2, l2);
+		dumper->dump(dumper, reason, pt_regs, s1, l1, s2, l2);
 	spin_unlock_irqrestore(&dump_list_lock, flags);
 }
 #endif

[PATCH v3 17/22] kmsg_dumper: Introduce a new 'SOFT' dump reason

[Mike Waychison]

It is a useful to be able to exercise kmsg_dumper implementations without
requiring a kernel oops or panic.  This commit adds a new reason called
KMSG_DUMP_SOFT, which signifies that the system isn't really going down.

This logic is used in a later commit that introduces the netoops driver.

Signed-off-by: Mike Waychison <mikew@google.com>
---
Notes:

It is also possible that we not introduce KMSG_DUMP_SOFT, and simply overload
the existing KMSG_DUMP_OOPS reason, but I figured that this would be cleaner.
---
 include/linux/kmsg_dump.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index a229acc..0abc2d7 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -20,6 +20,7 @@ enum kmsg_dump_reason {
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_PANIC,
 	KMSG_DUMP_KEXEC,
+	KMSG_DUMP_SOFT,
 };
 
 /**

[PATCH v3 18/22] sys-rq: Add option to soft dump

[Mike Waychison]

It is very useful to provide some means to force the kernel logs to make it out
via the kmsg_oops implementations on the console.  Add a new option 'Y' to
sysrq to allow dumping of logs to kmsg_dumper drivers.

Signed-off-by: Mike Waychison <mikew@google.com>
---
Changelog:
- v3
  - Added a note to Documentation/sysrq.txt about the command.

TODO: Figure out a better letter?  Can we reuse 'v'?
---
 Documentation/sysrq.txt |    4 ++++
 drivers/tty/sysrq.c     |   14 +++++++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/Documentation/sysrq.txt b/Documentation/sysrq.txt
index 312e375..3c7ee87 100644
--- a/Documentation/sysrq.txt
+++ b/Documentation/sysrq.txt
@@ -118,6 +118,10 @@ On all -  write a character to /proc/sysrq-trigger.  e.g.:
 'x'	- Used by xmon interface on ppc/powerpc platforms.
 
 'y'	- Show global CPU Registers [SPARC-64 specific]
+'y'	- Trigger a 'soft' kmsg dump. Any kmsg_dump clients (mtdoops,
+          netoops, ramoops) will initiate a dump to their respective
+          backend.
+
 
 'z'	- Dump the ftrace buffer
 
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c556ed9..f79f34a 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -41,6 +41,7 @@
 #include <linux/oom.h>
 #include <linux/slab.h>
 #include <linux/input.h>
+#include <linux/kmsg_dump.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq_regs.h>
@@ -395,6 +396,17 @@ static struct sysrq_key_op sysrq_unrt_op = {
 	.enable_mask	= SYSRQ_ENABLE_RTNICE,
 };
 
+static void sysrq_handle_softdump(int key)
+{
+	kmsg_dump(KMSG_DUMP_SOFT, NULL);
+}
+static struct sysrq_key_op sysrq_softdump_op = {
+	.handler	= sysrq_handle_softdump,
+	.help_msg	= "soft-dump(Y)",
+	.action_msg	= "Trigger a soft dump",
+	.enable_mask	= SYSRQ_ENABLE_DUMP,
+};
+
 /* Key Operations table and lock */
 static DEFINE_SPINLOCK(sysrq_key_table_lock);
 
@@ -451,7 +463,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	/* x: May be registered on ppc/powerpc for xmon */
 	NULL,				/* x */
 	/* y: May be registered on sparc64 for global register dump */
-	NULL,				/* y */
+	&sysrq_softdump_op,		/* y */
 	&sysrq_ftrace_dump_op,		/* z */
 };
 

[PATCH v3 08/22] netconsole: Split out netpoll_targets init/exit

[Mike Waychison]

As part of the factoring to move target handling out of netconsole,
abstract the construction and destruction of struct netpoll_targets
into their own functions.

Signed-off-by: Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Acked-by: Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>
---
 drivers/net/netconsole.c |   69 ++++++++++++++++++++++++++++------------------
 1 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index fed427d..57451a7 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -663,13 +663,14 @@ static struct config_item_type netconsole_subsys_type = {
 	.ct_owner	= THIS_MODULE,
 };
 
-static int __init dynamic_netpoll_targets_init(struct netpoll_targets *nts)
+static int __init dynamic_netpoll_targets_init(const char *subsys_name,
+					       struct netpoll_targets *nts)
 {
 	struct configfs_subsystem *subsys = &nts->configfs_subsys;
 
 	config_group_init(&subsys->su_group);
 	mutex_init(&subsys->su_mutex);
-	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, "netconsole",
+	strncpy((char *)&subsys->su_group.cg_item.ci_namebuf, subsys_name,
 		CONFIGFS_ITEM_NAME_LEN);
 	subsys->su_group.cg_item.ci_type = &netconsole_subsys_type;
 	return configfs_register_subsystem(subsys);
@@ -817,13 +818,15 @@ static struct console netconsole = {
 	.write	= write_msg,
 };
 
-static int __init init_netconsole(void)
+static int __init register_netpoll_targets(const char *subsys_name,
+					   struct netpoll_targets *nts,
+					   char *static_targets)
 {
 	int err;
 	struct netconsole_target *nt, *tmp;
-	unsigned long flags;
 	char *target_config;
-	char *input = config;
+	char *input = static_targets;
+	unsigned long flags;
 
 	if (strnlen(input, MAX_PARAM_LENGTH)) {
 		while ((target_config = strsep(&input, ";"))) {
@@ -832,41 +835,33 @@ static int __init init_netconsole(void)
 				err = PTR_ERR(nt);
 				goto fail;
 			}
-			/* Dump existing printks when we register */
-			netconsole.flags |= CON_PRINTBUFFER;
 
-			spin_lock_irqsave(&targets.lock, flags);
-			list_add(&nt->list, &targets.list);
-			spin_unlock_irqrestore(&targets.lock, flags);
+			spin_lock_irqsave(&nts->lock, flags);
+			list_add(&nt->list, &nts->list);
+			spin_unlock_irqrestore(&nts->lock, flags);
 		}
 	}
 
-	targets.netdev_notifier.notifier_call = netconsole_netdev_event;
-	err = register_netdevice_notifier(&targets.netdev_notifier);
+	nts->netdev_notifier.notifier_call = netconsole_netdev_event;
+	err = register_netdevice_notifier(&nts->netdev_notifier);
 	if (err)
 		goto fail;
 
-	err = dynamic_netpoll_targets_init(&targets);
+	err = dynamic_netpoll_targets_init(subsys_name, nts);
 	if (err)
 		goto undonotifier;
 
-	register_console(&netconsole);
-	printk(KERN_INFO "netconsole: network logging started\n");
-
-	return err;
+	return 0;
 
 undonotifier:
-	unregister_netdevice_notifier(&targets.netdev_notifier);
-
+	unregister_netdevice_notifier(&nts->netdev_notifier);
 fail:
-	printk(KERN_ERR "netconsole: cleaning up\n");
-
 	/*
 	 * Remove all targets and destroy them (only targets created
 	 * from the boot/module option exist here). Skipping the list
 	 * lock is safe here, and netpoll_cleanup() will sleep.
 	 */
-	list_for_each_entry_safe(nt, tmp, &targets.list, list) {
+	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
 		list_del(&nt->list);
 		free_param_target(nt);
 	}
@@ -874,13 +869,12 @@ fail:
 	return err;
 }
 
-static void __exit cleanup_netconsole(void)
+static void __exit unregister_netpoll_targets(struct netpoll_targets *nts)
 {
 	struct netconsole_target *nt, *tmp;
 
-	unregister_console(&netconsole);
-	dynamic_netpoll_targets_exit(&targets);
-	unregister_netdevice_notifier(&targets.netdev_notifier);
+	dynamic_netpoll_targets_exit(nts);
+	unregister_netdevice_notifier(&nts->netdev_notifier);
 
 	/*
 	 * Targets created via configfs pin references on our module
@@ -890,11 +884,32 @@ static void __exit cleanup_netconsole(void)
 	 * destroy them. Skipping the list lock is safe here, and
 	 * netpoll_cleanup() will sleep.
 	 */
-	list_for_each_entry_safe(nt, tmp, &targets.list, list) {
+	list_for_each_entry_safe(nt, tmp, &nts->list, list) {
 		list_del(&nt->list);
 		free_param_target(nt);
 	}
 }
 
+static int __init init_netconsole(void)
+{
+	int err;
+	err = register_netpoll_targets("netconsole", &targets, config);
+	if (err)
+		return err;
+	/* Dump existing printks if we registered any targets */
+	if (!list_empty(&targets.list))
+		netconsole.flags |= CON_PRINTBUFFER;
+	register_console(&netconsole);
+	printk(KERN_INFO "netconsole: network logging started\n");
+
+	return 0;
+}
+
+static void __exit cleanup_netconsole(void)
+{
+	unregister_console(&netconsole);
+	unregister_netpoll_targets(&targets);
+}
+
 module_init(init_netconsole);
 module_exit(cleanup_netconsole);

[PATCH v3 15/22] Oops: Pass regs to oops_exit()

[Mike Waychison]

Later commits in this series will want to see pt_regs if available when a
machine oopses.  Pass regs if available on to oops_exit().

Signed-off-by: Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/kernel/traps.c     |    2 +-
 arch/parisc/kernel/traps.c  |    2 +-
 arch/powerpc/kernel/traps.c |    2 +-
 arch/s390/kernel/traps.c    |    2 +-
 arch/sh/kernel/traps_32.c   |    2 +-
 arch/x86/kernel/dumpstack.c |    2 +-
 include/linux/kernel.h      |    2 +-
 kernel/panic.c              |    2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 446aee9..0de185c 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -277,7 +277,7 @@ void die(const char *str, struct pt_regs *regs, int err)
 	bust_spinlocks(0);
 	add_taint(TAINT_DIE);
 	spin_unlock_irq(&die_lock);
-	oops_exit();
+	oops_exit(regs);
 
 	if (in_interrupt())
 		panic("Fatal exception in interrupt");
diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c
index 8b58bf0..4aa5514 100644
--- a/arch/parisc/kernel/traps.c
+++ b/arch/parisc/kernel/traps.c
@@ -294,7 +294,7 @@ void die_if_kernel(char *str, struct pt_regs *regs, long err)
 		panic("Fatal exception");
 	}
 
-	oops_exit();
+	oops_exit(regs);
 	do_exit(SIGSEGV);
 }
 
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 1b2cdc8..76b950e 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -170,7 +170,7 @@ int die(const char *str, struct pt_regs *regs, long err)
 	if (panic_on_oops)
 		panic("Fatal exception");
 
-	oops_exit();
+	oops_exit(regs);
 	do_exit(err);
 
 	return 0;
diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c
index 7064082..73f3f2f 100644
--- a/arch/s390/kernel/traps.c
+++ b/arch/s390/kernel/traps.c
@@ -305,7 +305,7 @@ void die(const char * str, struct pt_regs * regs, long err)
 		panic("Fatal exception in interrupt");
 	if (panic_on_oops)
 		panic("Fatal exception: panic_on_oops");
-	oops_exit();
+	oops_exit(regs);
 	do_exit(SIGSEGV);
 }
 
diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c
index 3484c2f..425bd8f 100644
--- a/arch/sh/kernel/traps_32.c
+++ b/arch/sh/kernel/traps_32.c
@@ -103,7 +103,7 @@ void die(const char * str, struct pt_regs * regs, long err)
 	bust_spinlocks(0);
 	add_taint(TAINT_DIE);
 	spin_unlock_irq(&die_lock);
-	oops_exit();
+	oops_exit(regs);
 
 	if (kexec_should_crash(current))
 		crash_kexec(regs);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 6e8752c..89c720c 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -254,7 +254,7 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)
 		/* Nest count reaches zero, release the lock. */
 		arch_spin_unlock(&die_lock);
 	raw_local_irq_restore(flags);
-	oops_exit();
+	oops_exit(regs);
 
 	if (!signr)
 		return;
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index b6de9a6..c59c44e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -165,7 +165,7 @@ extern long (*panic_blink)(int state);
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2))) __cold;
 extern void oops_enter(void);
-extern void oops_exit(void);
+extern void oops_exit(struct pt_regs *);
 void print_oops_end_marker(void);
 extern int oops_may_print(void);
 NORET_TYPE void do_exit(long error_code)
diff --git a/kernel/panic.c b/kernel/panic.c
index 4c13b1a8..c3f39cd 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -349,7 +349,7 @@ void print_oops_end_marker(void)
  * Called when the architecture exits its oops handler, after printing
  * everything.
  */
-void oops_exit(void)
+void oops_exit(struct pt_regs *regs)
 {
 	do_oops_enter_exit();
 	print_oops_end_marker();

[PATCH v3 19/22] netoops: add core functionality

[Mike Waychison]

The kernel network dumper provides information about a crashed machine
on the network.

On a crash, the kernel spits out the contents of the kernel message buffer
along with a few other useful tidbits of information via netpoll UDP.
Each packet is sent a total of three times to deal with packet loss on the
connection. Furthermore a small amount critical data is present in every
packet, so even if only a single packet gets through, we still witness the
crash.  In the same vein, we send packet in reverse order to handle cases where
the kernel fatally crashes before transmission can be completed because often
the most interesting bits of a crash can be found in the tail of the log.

Configuration of the netoops device currently uses the same mechanism as
netpoll, that is, it uses a directory in configfs called "netpoll" that
users can create new targets.  It also supports targets as a module
parameter and as a kernel command line when built in.

Signed-off-by: Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
Changelog:
- v3
   - Bumped version to 3 to signify that this is different than the
     versions already used in-house.
   - Reserved fields for packet header have been removed in this version
     of the patch.  They were useful when I was trying to make a packet
     header that was compatible with our in-house protocol, but are no
     longer neccesary now that I am restructuring the packet header
     format to work better for other architectures.
   - Packet now has a section called 'arch_data' for per-arch specific
     bits to be handled.  The offset is also explicitly declared in the
     packet header.
   - Fields in the packet are explicitly little-endian.
- v2
   - Now uses netpoll_targets abstracted from netconsole.
   - As a side effect, we now don't have to hardcode port numbers; they
     are user overridable.
   - Compiles as a module in this patch.
---
 drivers/net/Kconfig   |   10 ++
 drivers/net/Makefile  |    1 
 drivers/net/netoops.c |  244 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/netoops.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 2ae9818..0e9fae3 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3394,6 +3394,16 @@ config NETCONSOLE_DYNAMIC
 	  at runtime through a userspace interface exported using configfs.
 	  See <file:Documentation/networking/netconsole.txt> for details.
 
+config NETOOPS
+	tristate "Network oops support"
+	select NETPOLL_TARGETS
+	select NETPOLL_TARGETS_DYNAMIC
+	help
+	  This option enables the ability to have the kernel logs emitted on
+	  the network when a machine Oopses or Panics.  Configuration of this
+	  option is done at runtime by configuring a destination IP address.
+	  If unsure, say N.
+
 config NETPOLL
 	def_bool false
 
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index b90738d..58d7181 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -288,6 +288,7 @@ obj-$(CONFIG_ETRAX_ETHERNET) += cris/
 obj-$(CONFIG_ENP2611_MSF_NET) += ixp2000/
 
 obj-$(CONFIG_NETCONSOLE) += netconsole.o
+obj-$(CONFIG_NETOOPS) += netoops.o
 
 obj-$(CONFIG_FS_ENET) += fs_enet/
 
diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
new file mode 100644
index 0000000..1940ea5
--- /dev/null
+++ b/drivers/net/netoops.c
@@ -0,0 +1,244 @@
+/*
+ * drivers/net/netoops.c
+ * Copyright (C) 2004 and beyond Google Inc.
+ *
+ * Original Author Ross Biro
+ * Revisions Rebecca Schultz
+ * Cleaned up by Mike Waychison <mikew-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
+ *
+ * This is very simple code to use the polling
+ * mode of the network drivers to send the
+ * contents of the printk buffer via udp w/o
+ * checksum to a unicast address.
+ */
+
+#include <linux/in.h>
+#include <linux/notifier.h>
+#include <linux/kernel.h>
+#include <linux/netpoll.h>
+#include <linux/nmi.h>
+#include <linux/utsname.h>
+#include <linux/watchdog.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/kmsg_dump.h>
+#include <linux/netpoll_targets.h>
+
+#define NETOOPS_TYPE_PRINTK_BUFFER 1
+#define NETOOPS_TYPE_PRINTK_BUFFER_SOFT 3
+#define NETOOPS_VERSION 0x0003
+#define NETOOPS_PORT 2004
+#define NETOOPS_RETRANSMIT_COUNT 3
+
+static DEFINE_NETPOLL_TARGETS(targets);
+
+#define MAX_PARAM_LENGTH	256
+static char __initdata config[MAX_PARAM_LENGTH];
+module_param_string(netoops, config, MAX_PARAM_LENGTH, 0);
+MODULE_PARM_DESC(netoops, " netoops=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr]");
+
+#ifndef	MODULE
+static int __init option_setup(char *opt)
+{
+	strlcpy(config, opt, MAX_PARAM_LENGTH);
+	return 1;
+}
+__setup("netoops=", option_setup);
+#endif	/* MODULE */
+
+/*
+ * Architecture specific support.
+ */
+#if defined(__i386__) || defined(__x86_64__)
+#define NETOOPS_ARCH 2
+struct netoops_arch_data {
+
+} __attribute__((packed));
+#else
+#error "unsupported architecture"
+#endif
+
+/*
+ * Architecture independent support.
+ */
+#define NETOOPS_DATA_BYTES 1024
+struct netoops_msg {
+	struct {
+		__le16 version;    /* MUST be @ offset 0 */
+		__le16 dump_id;
+		/* Offset into packet before data[] starts.  */
+		__le16 data_offset;
+		__le16 arch;
+		/* Offset into packet before struct arch_data starts. */
+		__le16 arch_offset;
+		__le16 type;
+		__le32 packet_count;
+		__le32 packet_no;
+		/*
+		 * NOTE: fixed length strings for a packet.  NULL
+		 * termination not required.
+		 */
+		char kernel_version[64];
+	} __attribute__ ((packed)) header;
+	struct netoops_arch_data arch_data;
+	char data[NETOOPS_DATA_BYTES];
+} __attribute__ ((packed));
+
+static struct netoops_msg msg;
+
+static void setup_packet_header(int packet_count, int soft_dump)
+{
+	typeof(msg.header) *h = &msg.header;
+
+	h->version = cpu_to_le16(NETOOPS_VERSION);
+	h->data_offset = cpu_to_le16(offsetof(struct netoops_msg, data));
+	h->arch = cpu_to_le16(NETOOPS_ARCH);
+	h->arch_offset = cpu_to_le16(offsetof(struct netoops_msg, arch_data));
+	h->dump_id = cpu_to_le16((jiffies/HZ) & 0xffff);
+	h->type = cpu_to_le16(soft_dump ? NETOOPS_TYPE_PRINTK_BUFFER_SOFT :
+					  NETOOPS_TYPE_PRINTK_BUFFER);
+	h->packet_count = cpu_to_le32(packet_count);
+	strncpy(h->kernel_version, utsname()->release,
+		min(sizeof(msg.header.kernel_version),
+		    sizeof(utsname()->release)));
+}
+
+static int packet_count_from_length(unsigned long l)
+{
+	return (l + NETOOPS_DATA_BYTES - 1) / NETOOPS_DATA_BYTES;
+}
+
+/* Send the packet to all targets */
+static void netoops_send_packet(int packet_nr)
+{
+	struct netpoll_target *nt;
+
+	msg.header.packet_no = cpu_to_le32(packet_nr);
+
+	list_for_each_entry(nt, &targets.list, list) {
+		if (nt->np_state == NETPOLL_ENABLED
+		    && netif_running(nt->np.dev)) {
+			netpoll_send_udp(&nt->np, (char *)&msg, sizeof(msg));
+		}
+	}
+
+}
+
+/*
+ * Send the passed in segment of kmsg via netpoll.  Packets are sent in reverse
+ * order, with the tail packet (the first one transmitted) zero-padded.
+ */
+static void netoops_send_segment(int packet_offset,
+				 const char *s, unsigned long l)
+{
+	int packet_count = packet_count_from_length(l);
+	size_t data_length;
+	int i;
+
+	for (i = packet_count - 1; i >= 0; i--) {
+		/* Usually messages completely fill the data field */
+		data_length = NETOOPS_DATA_BYTES;
+		if (i == packet_count - 1) {
+			/* Except the tail packet, which is zero-padded */
+			data_length = l % NETOOPS_DATA_BYTES;
+			memset(msg.data + data_length, 0,
+			       NETOOPS_DATA_BYTES - data_length);
+		}
+		BUG_ON(data_length > NETOOPS_DATA_BYTES);
+
+		/* Copy the payload into the packet and send */
+		memcpy(msg.data, s + (i * NETOOPS_DATA_BYTES), data_length);
+		netoops_send_packet((packet_count - i - 1) + packet_offset);
+
+		touch_nmi_watchdog();
+	}
+}
+
+/*
+ * Callback used by the kmsg_dumper.
+ *
+ * Called with interrupts disabled locally.
+ */
+static void netoops(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
+		    struct pt_regs *regs,
+		    const char *s1, unsigned long l1,
+		    const char *s2, unsigned long l2) {
+	unsigned long flags;
+	int packet_count_1, packet_count_2;
+	int soft_dump = 0;
+	int i;
+
+	/* Only handle fatal problems */
+	if (reason != KMSG_DUMP_OOPS
+	 && reason != KMSG_DUMP_PANIC
+	 && reason != KMSG_DUMP_SOFT)
+		return;
+
+	if (reason == KMSG_DUMP_SOFT)
+		soft_dump = 1;
+
+	spin_lock_irqsave(&targets.lock, flags);
+
+	/* compute total length of the message we are going to send */
+	packet_count_1 = packet_count_from_length(l1);
+	packet_count_2 = packet_count_from_length(l2);
+
+	/* setup the non varying parts of the message */
+	memset(&msg, 0, sizeof(msg));
+	setup_packet_header(packet_count_1 + packet_count_2, soft_dump);
+
+	/* Transmission loop */
+	for (i = 0; i < NETOOPS_RETRANSMIT_COUNT; i++) {
+		/* Send the full packets from the second segment */
+		netoops_send_segment(0, s2, l2);
+		netoops_send_segment(packet_count_2, s1, l1);
+	}
+
+	spin_unlock_irqrestore(&targets.lock, flags);
+}
+
+static struct kmsg_dumper netoops_dumper = {
+	.dump = netoops,
+};
+
+static int __init netoops_init(void)
+{
+	int retval = -EINVAL;
+
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.version) != 0);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.dump_id) != 2);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.data_offset) != 4);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.arch) != 6);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.arch_offset) != 8);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.type) != 10);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.packet_count) != 12);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.packet_no) != 16);
+
+	targets.default_local_port = NETOOPS_PORT;
+	targets.default_remote_port = NETOOPS_PORT;
+
+	config[MAX_PARAM_LENGTH - 1] = '\0';
+	retval = register_netpoll_targets("netoops", &targets, config);
+	if (retval)
+		goto out;
+
+	retval = kmsg_dump_register(&netoops_dumper);
+	if (retval)
+		goto out_targets;
+
+	return 0;
+out_targets:
+	unregister_netpoll_targets(&targets);
+out:
+	return retval;
+}
+
+static void __exit netoops_exit(void)
+{
+	kmsg_dump_unregister(&netoops_dumper);
+	unregister_netpoll_targets(&targets);
+}
+
+module_init(netoops_init);
+module_exit(netoops_exit);
+MODULE_LICENSE("GPL");

[PATCH v3 20/22] netoops: Add x86 specific bits to packet headers

[Mike Waychison]

We need to be able to gather information about the CPUs that caused the
crash.  Rather than hope that the data is available in the dmesg
recieved, and is still parsable, send a binary representation in each
packet.  This allows us to quickly identify crashes even in cases where
we crash and burn as part of the netoops process (or if we timeout
asynchronously on a hardware watchdog).

This commit only handles x86, but it should be easy to add any
per-architecture specific information going forward in a compatible way.

Signed-off-by: Mike Waychison <mikew@google.com>
---
Changelog:
- v3
  - Added x86 vendor to the packet header.
  - Split out the arch specific data into its own data structure.  Only
    x86 is available at the moment.  This data will want to transition
    into per-arch code at some point.
---
 drivers/net/netoops.c |   26 +++++++++++++++++++++++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index 1940ea5..f7665ad 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -52,8 +52,25 @@ __setup("netoops=", option_setup);
 #if defined(__i386__) || defined(__x86_64__)
 #define NETOOPS_ARCH 2
 struct netoops_arch_data {
-
+	u8 x86_vendor;
+	u8 x86_family;
+	u8 x86_model;
+	u8 x86_stepping;
+	/* NOTE: regs is 60 or 168 bytes */
+	struct pt_regs regs;
 } __attribute__((packed));
+
+static void setup_packet_arch_data(struct netoops_arch_data *arch_data,
+				   struct pt_regs *regs) {
+	arch_data->x86_vendor   = current_cpu_data.x86_vendor;
+	arch_data->x86_family   = current_cpu_data.x86;
+	arch_data->x86_model    = current_cpu_data.x86_model;
+	arch_data->x86_stepping = current_cpu_data.x86_mask;
+	if (regs != NULL)
+		memcpy(&arch_data->regs, regs, sizeof(arch_data->regs));
+	else
+		memset(&arch_data->regs, 0, sizeof(arch_data->regs));
+}
 #else
 #error "unsupported architecture"
 #endif
@@ -86,7 +103,8 @@ struct netoops_msg {
 
 static struct netoops_msg msg;
 
-static void setup_packet_header(int packet_count, int soft_dump)
+static void setup_packet_header(int packet_count, struct pt_regs *regs,
+				int soft_dump)
 {
 	typeof(msg.header) *h = &msg.header;
 
@@ -101,6 +119,8 @@ static void setup_packet_header(int packet_count, int soft_dump)
 	strncpy(h->kernel_version, utsname()->release,
 		min(sizeof(msg.header.kernel_version),
 		    sizeof(utsname()->release)));
+
+	setup_packet_arch_data(&msg.arch_data, regs);
 }
 
 static int packet_count_from_length(unsigned long l)
@@ -185,7 +205,7 @@ static void netoops(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
 
 	/* setup the non varying parts of the message */
 	memset(&msg, 0, sizeof(msg));
-	setup_packet_header(packet_count_1 + packet_count_2, soft_dump);
+	setup_packet_header(packet_count_1 + packet_count_2, regs, soft_dump);
 
 	/* Transmission loop */
 	for (i = 0; i < NETOOPS_RETRANSMIT_COUNT; i++) {

[PATCH v3 21/22] netoops: Add user-programmable boot_id

[Mike Waychison]

Add support for letting userland define a 32bit boot id.  This is useful
for users to be able to correlate netoops reports to specific boot
instances offline.

Signed-off-by: Mike Waychison <mikew@google.com>
---
Changelog:
- v3
  - boot_id is always little-endian on the network.
  - This patch adds a dependency on sysfs.  Declare the dependency in
    the Kconfig.
---
 drivers/net/Kconfig   |    1 +
 drivers/net/netoops.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0e9fae3..0098124 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3396,6 +3396,7 @@ config NETCONSOLE_DYNAMIC
 
 config NETOOPS
 	tristate "Network oops support"
+	depends on SYSFS
 	select NETPOLL_TARGETS
 	select NETPOLL_TARGETS_DYNAMIC
 	help
diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index f7665ad..6c4c0f2 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -91,6 +91,7 @@ struct netoops_msg {
 		__le16 type;
 		__le32 packet_count;
 		__le32 packet_no;
+		__le32 boot_id;
 		/*
 		 * NOTE: fixed length strings for a packet.  NULL
 		 * termination not required.
@@ -103,6 +104,8 @@ struct netoops_msg {
 
 static struct netoops_msg msg;
 
+static u32 netoops_boot_id;
+
 static void setup_packet_header(int packet_count, struct pt_regs *regs,
 				int soft_dump)
 {
@@ -116,6 +119,7 @@ static void setup_packet_header(int packet_count, struct pt_regs *regs,
 	h->type = cpu_to_le16(soft_dump ? NETOOPS_TYPE_PRINTK_BUFFER_SOFT :
 					  NETOOPS_TYPE_PRINTK_BUFFER);
 	h->packet_count = cpu_to_le32(packet_count);
+	h->boot_id = cpu_to_le32(netoops_boot_id);
 	strncpy(h->kernel_version, utsname()->release,
 		min(sizeof(msg.header.kernel_version),
 		    sizeof(utsname()->release)));
@@ -217,6 +221,49 @@ static void netoops(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
 	spin_unlock_irqrestore(&targets.lock, flags);
 }
 
+static ssize_t netoops_show(struct kobject *kobj,
+			    struct kobj_attribute *attr,
+			    char *buf) {
+	if (!strcmp(attr->attr.name, "netoops_boot_id"))
+		snprintf(buf, PAGE_SIZE, "%d\n", netoops_boot_id);
+	buf[PAGE_SIZE - 1] = '\0';
+	return strnlen(buf, PAGE_SIZE);
+}
+
+static ssize_t netoops_store(struct kobject *kobj,
+			     struct kobj_attribute *attr,
+			     const char *buf,
+			     size_t count) {
+	if (!count)
+		return count;
+
+	if (!strcmp(attr->attr.name, "netoops_boot_id")) {
+		unsigned long tmp;
+		if (strict_strtoul(buf, 0, &tmp))
+			return -EINVAL;
+		if (tmp > UINT_MAX)
+			printk("Warning: truncating boot_id to 32bits.");
+		netoops_boot_id = tmp;
+	} else
+		return -EINVAL;
+
+	return count;
+}
+
+static struct kobj_attribute netoops_boot_number_attribute =
+	__ATTR(netoops_boot_id, 0666, netoops_show, netoops_store);
+
+static struct attribute *attrs[] = {
+	&netoops_boot_number_attribute.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = attrs,
+};
+
+static struct kobject *netoops_kobj;
+
 static struct kmsg_dumper netoops_dumper = {
 	.dump = netoops,
 };
@@ -233,6 +280,7 @@ static int __init netoops_init(void)
 	BUILD_BUG_ON(offsetof(struct netoops_msg, header.type) != 10);
 	BUILD_BUG_ON(offsetof(struct netoops_msg, header.packet_count) != 12);
 	BUILD_BUG_ON(offsetof(struct netoops_msg, header.packet_no) != 16);
+	BUILD_BUG_ON(offsetof(struct netoops_msg, header.boot_id) != 20);
 
 	targets.default_local_port = NETOOPS_PORT;
 	targets.default_remote_port = NETOOPS_PORT;
@@ -242,11 +290,23 @@ static int __init netoops_init(void)
 	if (retval)
 		goto out;
 
+	netoops_kobj = kobject_create_and_add("netoops", kernel_kobj);
+	if (!netoops_kobj)
+		goto out_targets;
+
+	retval = sysfs_create_group(netoops_kobj, &attr_group);
+	if (retval)
+		goto out_kobj;
+
 	retval = kmsg_dump_register(&netoops_dumper);
 	if (retval)
-		goto out_targets;
+		goto out_sysfs_group;
 
 	return 0;
+out_sysfs_group:
+	sysfs_remove_group(netoops_kobj, &attr_group);
+out_kobj:
+	kobject_put(netoops_kobj);
 out_targets:
 	unregister_netpoll_targets(&targets);
 out:
@@ -256,6 +316,8 @@ out:
 static void __exit netoops_exit(void)
 {
 	kmsg_dump_unregister(&netoops_dumper);
+	sysfs_remove_group(netoops_kobj, &attr_group);
+	kobject_put(netoops_kobj);
 	unregister_netpoll_targets(&targets);
 }
 

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Matt Mackall]

On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> Add support for letting userland define a 32bit boot id.  This is useful
> for users to be able to correlate netoops reports to specific boot
> instances offline.

This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
that's used by kerneloops.org.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Mike Waychison]

On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
>> Add support for letting userland define a 32bit boot id.  This is useful
>> for users to be able to correlate netoops reports to specific boot
>> instances offline.
>
> This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> that's used by kerneloops.org.

Could be.  I'm looking at it now... There is no documentation for this
boot_id field?

Reusing this guy would work, except that it doesn't appear to allow
arbitrary values to be set.  We need to inject our boot sequence
number (which is figured out in userland) in the packet somehow as we
need to correlate it to our other monitoring systems.

I agree that having the uuid included in the netoops messages would be
very useful.

netoops is probably the wrong place to have a system-wide boot
sequence ID set in retrospect.  For our purposes, I can have it
encoded into the 'user_blob' portion of the packet if folks don't
think the kernel should have any notion of boot sequence number
programmable by userland.

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Matt Mackall]

On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm@selenic.com> wrote:
> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> >> Add support for letting userland define a 32bit boot id.  This is useful
> >> for users to be able to correlate netoops reports to specific boot
> >> instances offline.
> >
> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> > that's used by kerneloops.org.
> 
> Could be.  I'm looking at it now... There is no documentation for this
> boot_id field?

Probably not. It's just a random number generated at boot.

> Reusing this guy would work, except that it doesn't appear to allow
> arbitrary values to be set.  We need to inject our boot sequence
> number (which is figured out in userland) in the packet somehow as we
> need to correlate it to our other monitoring systems.

What happens if you oops before userspace is available?

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Mike Waychison]

On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
>> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm@selenic.com> wrote:
>> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
>> >> Add support for letting userland define a 32bit boot id.  This is useful
>> >> for users to be able to correlate netoops reports to specific boot
>> >> instances offline.
>> >
>> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
>> > that's used by kerneloops.org.
>>
>> Could be.  I'm looking at it now... There is no documentation for this
>> boot_id field?
>
> Probably not. It's just a random number generated at boot.
>
>> Reusing this guy would work, except that it doesn't appear to allow
>> arbitrary values to be set.  We need to inject our boot sequence
>> number (which is figured out in userland) in the packet somehow as we
>> need to correlate it to our other monitoring systems.
>
> What happens if you oops before userspace is available?
>

Either one of two general cases:
  - The crash is a one-off and the machine comes back.  The boot
number sequence will see a hole in it, which is a clue that something
bad happened.
  - The machine is in a crash loop.  This has the same failure mode
for us as if the machine never made it onto the network due to
whatever reason: bad cables, bad firmware, bad ram, ...

In both cases, we can detect that something is wrong and handle it.
Note that our firmware is responsible for incrementing the boot
sequence at bootup, which is why the above works.   In general though,
our machines do make it up to userland -- staying alive once booted is
the hard part ;)

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Matt Mackall]

On Tue, 2010-12-14 at 14:33 -0800, Mike Waychison wrote:
> On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> > On Tue, 2010-12-14 at 13:59 -0800, Mike Waychison wrote:
> >> On Tue, Dec 14, 2010 at 1:42 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> >> > On Tue, 2010-12-14 at 13:30 -0800, Mike Waychison wrote:
> >> >> Add support for letting userland define a 32bit boot id.  This is useful
> >> >> for users to be able to correlate netoops reports to specific boot
> >> >> instances offline.
> >> >
> >> > This sounds a lot like the pre-existing /proc/sys/kernel/random/boot_id
> >> > that's used by kerneloops.org.
> >>
> >> Could be.  I'm looking at it now... There is no documentation for this
> >> boot_id field?
> >
> > Probably not. It's just a random number generated at boot.
> >
> >> Reusing this guy would work, except that it doesn't appear to allow
> >> arbitrary values to be set.  We need to inject our boot sequence
> >> number (which is figured out in userland) in the packet somehow as we
> >> need to correlate it to our other monitoring systems.
> >
> > What happens if you oops before userspace is available?
> >
> 
> Either one of two general cases:
>   - The crash is a one-off and the machine comes back.  The boot
> number sequence will see a hole in it, which is a clue that something
> bad happened.
>   - The machine is in a crash loop.  This has the same failure mode
> for us as if the machine never made it onto the network due to
> whatever reason: bad cables, bad firmware, bad ram, ...
> 
> In both cases, we can detect that something is wrong and handle it.
> Note that our firmware is responsible for incrementing the boot
> sequence at bootup, which is why the above works.   In general though,
> our machines do make it up to userland -- staying alive once booted is
> the hard part ;)

Interesting. Is this Google-specific firmware magic? I'd probably accept
a hook in random.c to fold a number into the UUID, which would unify
things. 

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Mike Waychison]

On Tue, Dec 14, 2010 at 2:47 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2010-12-14 at 14:33 -0800, Mike Waychison wrote:
>> On Tue, Dec 14, 2010 at 2:06 PM, Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org> wrote:
>> > What happens if you oops before userspace is available?
>> >
>>
>> Either one of two general cases:
>>   - The crash is a one-off and the machine comes back.  The boot
>> number sequence will see a hole in it, which is a clue that something
>> bad happened.
>>   - The machine is in a crash loop.  This has the same failure mode
>> for us as if the machine never made it onto the network due to
>> whatever reason: bad cables, bad firmware, bad ram, ...
>>
>> In both cases, we can detect that something is wrong and handle it.
>> Note that our firmware is responsible for incrementing the boot
>> sequence at bootup, which is why the above works.   In general though,
>> our machines do make it up to userland -- staying alive once booted is
>> the hard part ;)
>
> Interesting. Is this Google-specific firmware magic?

Ya, this is a Google-ism.  I'd be surprised if there weren't other
platforms that had the same thing though (though I don't know of
anything standard on x86).

> I'd probably accept
> a hook in random.c to fold a number into the UUID, which would unify
> things.

I'm not sure there is a _good_ way to support this, is there?  I just
read through RFC 4122 and UUIDs seem to be pretty well structured;
it's probably not such a great idea to allow folks to override
portions of it.  Like I mentioned in my last email though, I'm okay
pushing this boot sequence ID down into the user blob which acts like
a place for "vendor extensions" if there isn't a good place for it in
the kernel.

Re: [PATCH v3 21/22] netoops: Add user-programmable boot_id

[Ted Ts'o]

On Tue, Dec 14, 2010 at 03:19:50PM -0800, Mike Waychison wrote:
> 
> I'm not sure there is a _good_ way to support this, is there?  I just
> read through RFC 4122 and UUIDs seem to be pretty well structured;
> it's probably not such a great idea to allow folks to override
> portions of it.  Like I mentioned in my last email though, I'm okay
> pushing this boot sequence ID down into the user blob which acts like
> a place for "vendor extensions" if there isn't a good place for it in
> the kernel.

If you really wanted to do it I could imagine using a time-based UUID
(see RFC 4122, section 4.2), where the boot sequence number could be
mapped into the 14-bit clock sequence, and using the time read from
the hardware counter and the ethernet MAC address from... umm the
"first" ethernet port (depending on how you define that) to form a
UUID.  I'm not convinced it's really worth it, though, unless someone
really wants a good per-boot session UUID generated some other way
than a random number generator that may not be all that adequately
seed at the time when you first sample the boot session UUID.

I suspect pushing the boot sequence ID into its own special field with
the semantics that you want is probably the better way to go.

     	     	    	      	  	- Ted

[PATCH v3 22/22] netoops: Add a user programmable blob to the netoops packet.

[Mike Waychison]

In our environment, it is important for us to capture motherboard name
and firmware version for consideration when analyzing netoops dumps.

We would like to collect this information in userland at system startup
(in platform specific ways) and plug this information into the netoops
driver via /sys/kernel/netoops/netoops_user_blob.

Files introduced:
  /sys/kernel/netoops/netoops_user_blob

Signed-off-by: Mike Waychison <mikew@google.com>
---
Changelog:
- v3
  - Rewritten to support an opaque user blob, rather than fw_version and
    board_name specifically.
---
 drivers/net/netoops.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index 6c4c0f2..036e4d8 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -29,6 +29,7 @@
 #define NETOOPS_VERSION 0x0003
 #define NETOOPS_PORT 2004
 #define NETOOPS_RETRANSMIT_COUNT 3
+#define NETOOPS_BLOB_BYTES (size_t)128
 
 static DEFINE_NETPOLL_TARGETS(targets);
 
@@ -97,6 +98,11 @@ struct netoops_msg {
 		 * termination not required.
 		 */
 		char kernel_version[64];
+		/*
+		 * Data that comes from userland.  Can be anything, but
+		 * is currently capped at NETOOPS_BLOB_BYTES.
+		 */
+		char user_blob[NETOOPS_BLOB_BYTES];
 	} __attribute__ ((packed)) header;
 	struct netoops_arch_data arch_data;
 	char data[NETOOPS_DATA_BYTES];
@@ -104,6 +110,8 @@ struct netoops_msg {
 
 static struct netoops_msg msg;
 
+static size_t netoops_user_blob_length;
+static char netoops_user_blob[NETOOPS_BLOB_BYTES];
 static u32 netoops_boot_id;
 
 static void setup_packet_header(int packet_count, struct pt_regs *regs,
@@ -120,6 +128,7 @@ static void setup_packet_header(int packet_count, struct pt_regs *regs,
 					  NETOOPS_TYPE_PRINTK_BUFFER);
 	h->packet_count = cpu_to_le32(packet_count);
 	h->boot_id = cpu_to_le32(netoops_boot_id);
+	memcpy(h->user_blob, netoops_user_blob, netoops_user_blob_length);
 	strncpy(h->kernel_version, utsname()->release,
 		min(sizeof(msg.header.kernel_version),
 		    sizeof(utsname()->release)));
@@ -224,10 +233,15 @@ static void netoops(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
 static ssize_t netoops_show(struct kobject *kobj,
 			    struct kobj_attribute *attr,
 			    char *buf) {
-	if (!strcmp(attr->attr.name, "netoops_boot_id"))
+	if (!strcmp(attr->attr.name, "netoops_user_blob")) {
+		memcpy(buf, netoops_user_blob, netoops_user_blob_length);
+		return netoops_user_blob_length;
+	}
+	if (!strcmp(attr->attr.name, "netoops_boot_id")) {
 		snprintf(buf, PAGE_SIZE, "%d\n", netoops_boot_id);
-	buf[PAGE_SIZE - 1] = '\0';
-	return strnlen(buf, PAGE_SIZE);
+		return strnlen(buf, PAGE_SIZE);
+	}
+	return -EINVAL;
 }
 
 static ssize_t netoops_store(struct kobject *kobj,
@@ -237,7 +251,10 @@ static ssize_t netoops_store(struct kobject *kobj,
 	if (!count)
 		return count;
 
-	if (!strcmp(attr->attr.name, "netoops_boot_id")) {
+	if (!strcmp(attr->attr.name, "netoops_user_blob")) {
+		count = min(count, NETOOPS_BLOB_BYTES);
+		memcpy(netoops_user_blob, buf, count);
+	} else if (!strcmp(attr->attr.name, "netoops_boot_id")) {
 		unsigned long tmp;
 		if (strict_strtoul(buf, 0, &tmp))
 			return -EINVAL;
@@ -250,10 +267,13 @@ static ssize_t netoops_store(struct kobject *kobj,
 	return count;
 }
 
+static struct kobj_attribute netoops_user_blob_attribute =
+	__ATTR(netoops_user_blob, 0644, netoops_show, netoops_store);
 static struct kobj_attribute netoops_boot_number_attribute =
 	__ATTR(netoops_boot_id, 0666, netoops_show, netoops_store);
 
 static struct attribute *attrs[] = {
+	&netoops_user_blob_attribute.attr,
 	&netoops_boot_number_attribute.attr,
 	NULL,
 };