2010-11-08 20:32:13

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 00/23] netoops support

This patchset applies to v2.6.37-rc1.

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 that decides 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. I have tried to clean this code up internally, but
there are still several unresolved issues that would need to be worked
out as of this version. In particular:

* I am _NOT_ happy with the remaining userland ABIs presented in this
patchset. Specifically the files "net_dump_now",
"net_dump_one_shot", "netdump_fw_version", "netdump_board_name" and
"netdump_boot_id" should be considered. These files have been
cobbled together by a variety of engineers over the years, and they
aren't very pretty. I present them none-the-less to express the
scope of the functionality that we would like to maintain.

* I am _NOT_ happy with the data format of the transmitted packets. It is
very specific to our server environment and currently:

* is hard-coded to support both userland provided information (that may
not be applicable to others) and

* only supports i386 and x86_64.

I'd like to resolve each of the above issues in subsequent versions of this
patchset. I need help in identifying what the ABI should look like in
particular.

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

Patches 1 through 4 inclusive are fixes to the existing netconsole code,
adding locking consistency, fixing races and deadlocks.

Patches 5 through 14 inclusive splits the target configuration portion
of netconsole out into a new component in net/core/netpoll_targets.c.

Patches 15 through 18 inclusive are core changes to support
functionality in the netoops driver.

Patches 19 through 23 is 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 - netpoll: Introduce netpoll_target configs
13 - netconsole: Move setting of default ports.
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 fields to the netoops packet.
22 - netoops: Add one-shot mode
23 - netoops: Add an interface to trigger various types of crashes.


Diffstat
========
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/char/sysrq.c | 14
drivers/mtd/mtdoops.c | 4
drivers/net/Kconfig | 26 +
drivers/net/Makefile | 1
drivers/net/netconsole.c | 735 +--------------------------------------
drivers/net/netoops.c | 401 +++++++++++++++++++++
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 | 5
net/core/Makefile | 1
net/core/netpoll_targets.c | 746 ++++++++++++++++++++++++++++++++++++++++
21 files changed, 1309 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.

* Dealing with excessive logging:
This patchset introduces a 'one-shot' mode, which has saved our bacon
several times in the past. It's not totally uncommon for the kernel's
crash path to be buggy, in turn causing the kernel to emit Oopses until
the cows come home (or rather, until the hardware watchdogs trip).
One-shot keeps us from emitting too much garbage on the network when this
happens.

As well, while console filtering of printk()ed messages is common
practice, we would like to see *all* kernel messages, including KERN_DEBUG
messages when investigating a kernel crash. Using kmsg_dumper to get at
the full ring buffer provides access to this sort of data, whereas
netconsole would be subject to system-wide filtering policies (which also
affect the serial console).

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


2010-11-08 20:32:20

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 02/23] netconsole: Introduce locking over the netpoll fields

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 <[email protected]>
---
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);

2010-11-08 20:32:23

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 03/23] netconsole: Introduce 'enabled' state-machine

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 <[email protected]>
---
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

2010-11-08 20:32:33

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 04/23] netconsole: Call netpoll_cleanup() in process context

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 <[email protected]>
---
drivers/net/netconsole.c | 55 ++++++++++++++++++++++++++++++++++++++++------
1 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 288a025..02ba5c4 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);
@@ -660,6 +688,9 @@ 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 it queued for cleanup already, that is fine, as that path holds a
+ * reference to the config_item.
*/
if (nt->np_state == NETPOLL_ENABLED)
netpoll_cleanup(&nt->np);
@@ -689,6 +720,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 +756,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);
}
}
}

2010-11-08 20:32:41

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 07/23] netconsole: Move netdev_notifier into netpoll_targets

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 <[email protected]>
---
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 55f72ba..8ea4c5c 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), \
@@ -737,6 +738,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;
@@ -745,8 +748,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:
@@ -762,7 +765,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,
@@ -772,10 +775,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;
@@ -839,7 +838,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;

@@ -853,7 +853,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");
@@ -877,7 +877,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

2010-11-08 20:32:42

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 05/23] netconsole: Wrap the list and locking in a structure

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 <[email protected]>
---
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 02ba5c4..f28681b 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 enabled, or was manually disabled
@@ -695,7 +698,7 @@ static void drop_netconsole_target(struct config_group *group,
if (nt->np_state == NETPOLL_ENABLED)
netpoll_cleanup(&nt->np);

- config_item_put(&nt->item);
+ netconsole_target_put(nt);
}

static struct configfs_group_operations netconsole_subsys_group_ops = {
@@ -722,7 +725,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)
{
@@ -746,8 +749,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:
@@ -763,7 +766,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,
@@ -785,11 +788,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)) {
/*
@@ -807,7 +810,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 = {
@@ -834,9 +837,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);
}
}

@@ -864,7 +867,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);
}
@@ -888,7 +891,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);
}

2010-11-08 20:32:54

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 09/23] netconsole: Add pointer to netpoll_targets

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 <[email protected]>
---
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 54d124c..ec8bda2 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 enabled, or was manually disabled
@@ -723,7 +742,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)
{
@@ -827,7 +846,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;

2010-11-08 20:33:12

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 11/23] netconsole: Abstract away the subsystem name

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 <[email protected]>
---
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 068df64..4348d23 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;
@@ -787,8 +793,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:
@@ -863,12 +869,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:
@@ -904,6 +917,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)

2010-11-08 20:33:25

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 13/23] netconsole: Move setting of default ports.

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 <[email protected]>
---
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 2fe571d..0d77fc8 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_NETPOLL_TARGETS_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);

@@ -923,6 +924,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;

2010-11-08 20:33:34

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 14/23] netpoll: Move target code into netpoll_targets.c

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 <[email protected]>
---
drivers/net/netconsole.c | 820 ---------------------------------------
include/linux/netpoll_targets.h | 76 ++++
net/core/Makefile | 1
net/core/netpoll_targets.c | 746 +++++++++++++++++++++++++++++++++++
4 files changed, 825 insertions(+), 818 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 0d77fc8..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 <[email protected]>");
MODULE_DESCRIPTION("Console driver for network interfaces");
@@ -65,743 +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 enabled, or was manually disabled
- * before being removed so netpoll may have already been cleaned up.
- *
- * If it queued for cleanup already, that is fine, as that path holds a
- * reference to the config_item.
- */
- if (nt->np_state == NETPOLL_ENABLED)
- 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;
@@ -841,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;
@@ -928,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..6817389
--- /dev/null
+++ b/net/core/netpoll_targets.c
@@ -0,0 +1,746 @@
+#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 enabled, or was manually disabled
+ * before being removed so netpoll may have already been cleaned up.
+ *
+ * If it queued for cleanup already, that is fine, as that path holds a
+ * reference to the config_item.
+ */
+ if (nt->np_state == NETPOLL_ENABLED)
+ 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);
+

2010-11-08 20:33:43

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 16/23] kmsg_dumper: Pass pt_regs along to dumpers.

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 <[email protected]>
---
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 b2ebaee..159b4d6 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1533,7 +1533,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;
@@ -1570,7 +1570,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

2010-11-08 20:33:46

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 17/23] kmsg_dumper: Introduce a new 'SOFT' dump reason

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 <[email protected]>
---

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.

TODO: Make sure mtdoops and ramoops do something useful with this flag?
---
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,
};

/**

2010-11-08 20:32:51

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 08/23] netconsole: Split out netpoll_targets init/exit

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 <[email protected]>
---
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 8ea4c5c..54d124c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -660,13 +660,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);
@@ -814,13 +815,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, ";"))) {
@@ -829,41 +832,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);
}
@@ -871,13 +866,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
@@ -887,11 +881,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);

2010-11-08 20:33:53

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 18/23] sys-rq: Add option to soft dump

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 <[email protected]>
---
drivers/char/sysrq.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index eaa5d3e..058d3c8 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/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 */
};

2010-11-08 20:33:57

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 20/23] netoops: Add x86 specific bits to packet headers

We need to be able to gather information about the CPUs that caused the crash.

This commit only handles x86, but it is desirable to come up with some new
packet format that can accommodate any architecture.

Signed-off-by: Mike Waychison <[email protected]>
---
TODO: This should be made more general to other architectures. As is, we are
probably okay exporting some value for the 'arch' field. Different
architectures though will likely want to gather different data.
---
drivers/net/netoops.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index dc1ee97..f63e12a 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -69,16 +69,24 @@ struct netoops_msg {
u32 packet_count;
u32 packet_no;
u32 __reserved1;
- u8 __reserved2;
- u8 __reserved3;
- u8 __reserved4;
+ u8 x86_family;
+ u8 x86_model;
+ u8 x86_stepping;
/*
* NOTE: fixed length strings for a packet. NULL
* termination not required.
*/
char kernel_version[64];
- char __reserved5[64];
- char __reserved6[64];
+ char __reserved2[64];
+ char __reserved3[64];
+ /* NOTE: regs is 60 or 168 bytes */
+ struct pt_regs regs; /* arch specific. */
+ /*
+ * NOTE: The header is potentially ~385 bytes
+ * already. That doesn't leave much room for
+ * expansion unless we reduce the data size
+ * or truncate above fields.
+ */
} __attribute__ ((packed)) header;
char data[NETOOPS_DATA_BYTES];
} __attribute__ ((packed));
@@ -94,10 +102,17 @@ static void setup_packet_header(int packet_count, int soft_dump)
msg.header.dump_id = (jiffies/HZ) & 0xffff;
msg.header.packet_count = packet_count;
msg.header.header_size = sizeof(msg.header);
+#ifndef CONFIG_UML
+ msg.header.x86_family = current_cpu_data.x86;
+ msg.header.x86_model = current_cpu_data.x86_model;
+ msg.header.x86_stepping = current_cpu_data.x86_mask;
+#endif
strncpy(msg.header.kernel_version,
utsname()->release,
min(sizeof(msg.header.kernel_version),
sizeof(utsname()->release)));
+ if (regs != NULL)
+ memcpy(&msg.header.regs, regs, sizeof(msg.header.regs));
}

static int packet_count_from_length(unsigned long l)
@@ -182,7 +197,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++) {

2010-11-08 20:34:04

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 19/23] netoops: add core functionality

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 <[email protected]>
---
Changelog:
- 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.


Notes:

The packet format described in this and subsequent patches currently represent
the packet format used by Google. It is _not_ generally applicable though, as
it does contain several fields that are x86 specific. I've included them here
nevertheless to foster discussion as to how best to abstract this sort of
information away.

In this commit, there are several fields that are marked "__reserved*" in the
packet header. These are replaced with actual definitions in later commits.
---
drivers/net/Kconfig | 10 ++
drivers/net/Makefile | 1
drivers/net/netoops.c | 235 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 246 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/netoops.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index b014cd6..0aa8dfc 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3382,6 +3382,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"
+ depends on PROC_FS
+ 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 652fc6b..5285e18 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..dc1ee97
--- /dev/null
+++ b/drivers/net/netoops.c
@@ -0,0 +1,235 @@
+/*
+ * drivers/net/netoops.c
+ * Copyright (C) 2004 and beyond Google Inc.
+ *
+ * Original Author Ross Biro
+ * Revisions Rebecca Schultz
+ * Cleaned up by Mike Waychison <[email protected]>
+ *
+ * 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 0x0002
+#define NETOOPS_PORT 2004
+#define NETOOPS_RETRANSMIT_COUNT 3
+
+#if defined(__i386__) || defined(__x86_64__)
+#define NETOOPS_ARCH 2
+#else
+#error "unsupported architecture"
+#endif
+
+#define MAX_PARAM_LENGTH 256
+
+static DEFINE_NETPOLL_TARGETS(targets);
+
+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 */
+
+
+#define NETOOPS_DATA_BYTES 1024
+
+struct netoops_msg {
+ struct {
+ u16 version; /* MUST be @ offset 0 */
+ /*
+ * Size of this header before data[] starts.
+ */
+ u16 header_size;
+ u16 arch;
+ u16 dump_id; /* MUST be @ offset 6 */
+ u16 type;
+ u32 packet_count;
+ u32 packet_no;
+ u32 __reserved1;
+ u8 __reserved2;
+ u8 __reserved3;
+ u8 __reserved4;
+ /*
+ * NOTE: fixed length strings for a packet. NULL
+ * termination not required.
+ */
+ char kernel_version[64];
+ char __reserved5[64];
+ char __reserved6[64];
+ } __attribute__ ((packed)) header;
+ char data[NETOOPS_DATA_BYTES];
+} __attribute__ ((packed));
+
+static struct netoops_msg msg;
+
+static void setup_packet_header(int packet_count, int soft_dump)
+{
+ msg.header.version = NETOOPS_VERSION;
+ msg.header.arch = NETOOPS_ARCH;
+ msg.header.type = soft_dump ? NETOOPS_TYPE_PRINTK_BUFFER_SOFT :
+ NETOOPS_TYPE_PRINTK_BUFFER;
+ msg.header.dump_id = (jiffies/HZ) & 0xffff;
+ msg.header.packet_count = packet_count;
+ msg.header.header_size = sizeof(msg.header);
+ strncpy(msg.header.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 = 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) != 6);
+
+ 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");

2010-11-08 20:34:15

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 23/23] netoops: Add an interface to trigger various types of crashes.

It is very useful to be able to test the crash path, but in order to do so, we
need to expose various ways to crash the kernel in a deterministic fashion.

This commit adds a file, /proc/sys/kernel/net_dump_now that takes various
tokens that will crash the kernel in various ways.

This requires kmsg_dump() be accessible to modules, so export it.

Signed-off-by: Mike Waychison <[email protected]>
---
Changelog:
- v2
- Folded EXPORT_SYMBOL_GPL(kmsg_dump) into this patch

TODO: User visible ABI needs a better interface
---
drivers/net/netoops.c | 31 +++++++++++++++++++++++++++++++
kernel/printk.c | 1 +
2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index 1ffd0f0..60683c5 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -288,6 +288,32 @@ static struct kmsg_dumper netoops_dumper = {
.dump = netoops,
};

+static int proc_netoops_now(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+ if (write) {
+ char magic[20];
+ /* just crash in kernel mode. */
+ if (copy_from_user(magic, buffer, min(*lenp, sizeof(magic))))
+ return -EFAULT;
+ magic[min(*lenp, sizeof(magic))-1] = 0;
+ if (!strcmp(magic, "elgooG")) {
+ /* Test a simple crash */
+ *(unsigned long *)0 = 0;
+ } else if (!strcmp(magic, "guB")) {
+ /* Test the BUG() handler */
+ BUG();
+ } else if (!strcmp(magic, "cinaP")) {
+ panic("Testing panic");
+ } else if (!strcmp(magic, "pmuD")) {
+ kmsg_dump(KMSG_DUMP_SOFT, NULL);
+ }
+ return 0;
+ }
+ *lenp = 0;
+ return 0;
+}
+
static struct ctl_table kern_table[] = {
{
.procname = "net_dump_one_shot",
@@ -296,6 +322,11 @@ static struct ctl_table kern_table[] = {
.mode = 0644,
.proc_handler = &proc_dointvec,
},
+ {
+ .procname = "net_dump_now",
+ .mode = 0600,
+ .proc_handler = &proc_netoops_now,
+ },
{},
};

diff --git a/kernel/printk.c b/kernel/printk.c
index 159b4d6..9323339 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1573,4 +1573,5 @@ void kmsg_dump(enum kmsg_dump_reason reason, struct pt_regs *pt_regs)
dumper->dump(dumper, reason, pt_regs, s1, l1, s2, l2);
spin_unlock_irqrestore(&dump_list_lock, flags);
}
+EXPORT_SYMBOL_GPL(kmsg_dump);
#endif

2010-11-08 20:34:17

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 22/23] netoops: Add one-shot mode

Sometimes it is possible to have a kernel crashing that continuously
Oopses. In this case, we do not want the network dumper get called over
and over again, as we may only be interested in the first Oops that a
kernel emits (especially in cases where we panic_on_oops).

In order to support this dumping policy, this commit introduces a file
call /proc/sys/kernel/net_dump_one_shot that contains a boolean as to
whether or not the netoops driver will operate in one-shot mode.

Note that 'soft' dumps do not disable the netoops driver when one-shot
mode is enabled.

Signed-off-by: Mike Waychison <[email protected]>
---
TODO: ABI needs a better home.
---
drivers/net/netoops.c | 37 +++++++++++++++++++++++++++++++++++++
1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index 5cbb732..1ffd0f0 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -97,6 +97,8 @@ struct netoops_msg {
} __attribute__ ((packed));

static struct netoops_msg msg;
+static int network_dumper_one_shot = 1;
+static int network_dumper_disabled;

static char netoops_fw_version[80];
static char netoops_board_name[80];
@@ -221,6 +223,10 @@ static void netoops(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
netoops_send_segment(packet_count_2, s1, l1);
}

+ /* Should we disable ourselves now? */
+ if (!soft_dump && network_dumper_one_shot)
+ network_dumper_disabled = 1;
+
spin_unlock_irqrestore(&targets.lock, flags);
}

@@ -282,6 +288,28 @@ static struct kmsg_dumper netoops_dumper = {
.dump = netoops,
};

+static struct ctl_table kern_table[] = {
+ {
+ .procname = "net_dump_one_shot",
+ .data = &network_dumper_one_shot,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec,
+ },
+ {},
+};
+
+static struct ctl_table root_table[] = {
+ {
+ .procname = "kernel",
+ .mode = 0555,
+ .child = kern_table,
+ },
+ {}
+};
+
+static struct ctl_table_header *proc_table_header;
+
static int __init netoops_init(void)
{
int retval = -EINVAL;
@@ -309,7 +337,15 @@ static int __init netoops_init(void)
if (retval)
goto out_sysfs_group;

+ proc_table_header = register_sysctl_table(root_table);
+ if (!proc_table_header) {
+ retval = -EINVAL;
+ goto out_kmsg_dump;
+ }
+
return 0;
+out_kmsg_dump:
+ kmsg_dump_unregister(&netoops_dumper);
out_sysfs_group:
sysfs_remove_group(netoops_kobj, &attr_group);
out_kobj:
@@ -322,6 +358,7 @@ out:

static void __exit netoops_exit(void)
{
+ unregister_sysctl_table(proc_table_header);
kmsg_dump_unregister(&netoops_dumper);
sysfs_remove_group(netoops_kobj, &attr_group);
kobject_put(netoops_kobj);

2010-11-08 20:34:41

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 21/23] netoops: Add user programmable fields to the netoops packet.

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

We collect this information in userland at system startup (in platform
specific ways) and plug this information into the netoops driver via
files available in sysfs.

Files introduced:
/sys/kernel/netdump/netdump_board_name
/sys/kernel/netdump/netdump_fw_version
/sys/kernel/netdump/netdump_boot_id

If this were to be rewritten, we would probably like to have a more
general way for userland to add to netdump packets.

Signed-off-by: Mike Waychison <[email protected]>
---
TODO: This would probably be better handled by having the kernel accept
some blob of data. It doesn't need to know what the data itself is, as
long as it is delivered in each packet.
---
drivers/net/netoops.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
index f63e12a..5cbb732 100644
--- a/drivers/net/netoops.c
+++ b/drivers/net/netoops.c
@@ -68,7 +68,7 @@ struct netoops_msg {
u16 type;
u32 packet_count;
u32 packet_no;
- u32 __reserved1;
+ u32 blog_boot_id;
u8 x86_family;
u8 x86_model;
u8 x86_stepping;
@@ -77,8 +77,13 @@ struct netoops_msg {
* termination not required.
*/
char kernel_version[64];
- char __reserved2[64];
- char __reserved3[64];
+ /*
+ * bios and board info come from smbios which
+ * has a maximum string length of 64 according
+ * to page 26 of the smbios 2.3 spec.
+ */
+ char bios_version[64];
+ char motherboard_type[64];
/* NOTE: regs is 60 or 168 bytes */
struct pt_regs regs; /* arch specific. */
/*
@@ -93,7 +98,12 @@ struct netoops_msg {

static struct netoops_msg msg;

-static void setup_packet_header(int packet_count, int soft_dump)
+static char netoops_fw_version[80];
+static char netoops_board_name[80];
+static int netoops_boot_number;
+
+static void setup_packet_header(int packet_count, struct pt_regs *regs,
+ int soft_dump)
{
msg.header.version = NETOOPS_VERSION;
msg.header.arch = NETOOPS_ARCH;
@@ -102,11 +112,16 @@ static void setup_packet_header(int packet_count, int soft_dump)
msg.header.dump_id = (jiffies/HZ) & 0xffff;
msg.header.packet_count = packet_count;
msg.header.header_size = sizeof(msg.header);
+ msg.header.blog_boot_id = (u32)netoops_boot_number;
#ifndef CONFIG_UML
msg.header.x86_family = current_cpu_data.x86;
msg.header.x86_model = current_cpu_data.x86_model;
msg.header.x86_stepping = current_cpu_data.x86_mask;
#endif
+ strncpy(msg.header.bios_version, netoops_fw_version,
+ sizeof(msg.header.bios_version));
+ strncpy(msg.header.motherboard_type, netoops_board_name,
+ sizeof(msg.header.motherboard_type));
strncpy(msg.header.kernel_version,
utsname()->release,
min(sizeof(msg.header.kernel_version),
@@ -209,6 +224,60 @@ 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, "netdump_fw_version"))
+ strncpy(buf, netoops_fw_version, PAGE_SIZE);
+ else if (!strcmp(attr->attr.name, "netdump_board_name"))
+ strncpy(buf, netoops_board_name, PAGE_SIZE);
+ else if (!strcmp(attr->attr.name, "netdump_boot_number"))
+ snprintf(buf, PAGE_SIZE, "%d\n", netoops_boot_number);
+ 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, "netdump_fw_version")) {
+ strncpy(netoops_fw_version, buf, sizeof(netoops_fw_version));
+ netoops_fw_version[sizeof(netoops_fw_version) - 1] = '\0';
+ } else if (!strcmp(attr->attr.name, "netdump_board_name")) {
+ strncpy(netoops_board_name, buf, sizeof(netoops_board_name));
+ netoops_board_name[sizeof(netoops_board_name) - 1] = '\0';
+ } else if (!strcmp(attr->attr.name, "netdump_boot_number")) {
+ ((char *)buf)[count - 1] = '\0';
+ sscanf(buf, "%du", &netoops_boot_number);
+ }
+ return count;
+}
+
+static struct kobj_attribute netoops_fw_version_attribute =
+ __ATTR(netoops_fw_version, 0666, netoops_show, netoops_store);
+static struct kobj_attribute netoops_board_name_attribute =
+ __ATTR(netoops_board_name, 0666, netoops_show, netoops_store);
+static struct kobj_attribute netoops_boot_number_attribute =
+ __ATTR(netoops_boot_number, 0666, netoops_show, netoops_store);
+
+static struct attribute *attrs[] = {
+ &netoops_fw_version_attribute.attr,
+ &netoops_board_name_attribute.attr,
+ &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,
};
@@ -228,11 +297,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:
@@ -242,6 +323,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);
}

2010-11-08 20:33:08

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 10/23] netconsole: Rename netconsole_target -> netpoll_target

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 <[email protected]>
---
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 ec8bda2..068df64 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);
@@ -666,16 +666,16 @@ static void drop_netconsole_target(struct config_group *group,
if (nt->np_state == NETPOLL_ENABLED)
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,
};

@@ -688,7 +688,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);
}

@@ -702,13 +702,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);
@@ -730,11 +730,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)
{
}

@@ -744,24 +744,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 ||
@@ -799,7 +799,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 */
@@ -839,7 +839,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;
@@ -858,7 +858,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;
@@ -887,7 +887,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);

2010-11-08 20:33:33

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 15/23] Oops: Pass regs to oops_exit()

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 <[email protected]>
---
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 cda78d5..45cc7c0 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -280,7 +280,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 450092c..442b4a5 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -197,7 +197,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 4c13b1a..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();

2010-11-08 20:35:50

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

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.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/net/Kconfig | 16 +++++++++++++---
drivers/net/netconsole.c | 10 +++++-----
2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index f6668cd..b014cd6 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3367,14 +3367,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)
@@ -3382,7 +3383,16 @@ config NETCONSOLE_DYNAMIC
See <file:Documentation/networking/netconsole.txt> for details.

config NETPOLL
- def_bool NETCONSOLE
+ def_bool false
+
+config NETPOLL_TARGETS
+ bool
+ select NETPOLL
+
+config NETPOLL_TARGETS_DYNAMIC
+ bool
+ select CONFIGFS_FS
+ select NETPOLL_TARGETS

config NETPOLL_TRAP
bool "Netpoll traffic trapping"
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4348d23..2fe571d 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -68,7 +68,7 @@ __setup("netconsole=", option_setup);
struct netpoll_targets {
struct list_head list;
spinlock_t lock;
-#ifdef CONFIG_NETCONSOLE_DYNAMIC
+#ifdef CONFIG_NETPOLL_TARGETS_DYNAMIC
struct configfs_subsystem configfs_subsys;
#endif
struct notifier_block netdev_notifier;
@@ -110,7 +110,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;
@@ -193,7 +193,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:
@@ -720,7 +720,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)
@@ -744,7 +744,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.

2010-11-08 20:32:11

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 01/23] netconsole: Remove unneeded reference counting

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 <[email protected]>
---
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);
}

2010-11-08 20:36:22

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 06/23] netconsole: Push configfs_subsystem into netpoll_targets

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 <[email protected]>
---
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 f28681b..55f72ba 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)
{
@@ -711,15 +659,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 */

@@ -847,7 +843,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;

@@ -880,7 +876,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);

/*

2010-11-08 20:55:33

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH v2 00/23] netoops support

On Mon, 2010-11-08 at 12:31 -0800, Mike Waychison wrote:
> This patchset applies to v2.6.37-rc1.
>
> 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 that decides 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. I have tried to clean this code up internally, but
> there are still several unresolved issues that would need to be worked
> out as of this version. In particular:
>
> * I am _NOT_ happy with the remaining userland ABIs presented in this
> patchset. Specifically the files "net_dump_now",
> "net_dump_one_shot", "netdump_fw_version", "netdump_board_name" and
> "netdump_boot_id" should be considered. These files have been
> cobbled together by a variety of engineers over the years, and they
> aren't very pretty. I present them none-the-less to express the
> scope of the functionality that we would like to maintain.
>
> * I am _NOT_ happy with the data format of the transmitted packets. It is
> very specific to our server environment and currently:
>
> * is hard-coded to support both userland provided information (that may
> not be applicable to others) and
>
> * only supports i386 and x86_64.
>
> I'd like to resolve each of the above issues in subsequent versions of this
> patchset. I need help in identifying what the ABI should look like in
> particular.
>
> Patchset summary
> ================
>
> Patches 1 through 4 inclusive are fixes to the existing netconsole code,
> adding locking consistency, fixing races and deadlocks.
>
> Patches 5 through 14 inclusive splits the target configuration portion
> of netconsole out into a new component in net/core/netpoll_targets.c.
>
> Patches 15 through 18 inclusive are core changes to support
> functionality in the netoops driver.
>
> Patches 19 through 23 is 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 - netpoll: Introduce netpoll_target configs
> 13 - netconsole: Move setting of default ports.
> 14 - netpoll: Move target code into netpoll_targets.c

This much of your set looks very nice to me, but I'd like to get some
more eyeballs on the first four. Dave?

Acked-by: Matt Mackall <[email protected]>

--
Mathematics is the supreme nostalgia of our time.

2010-11-08 21:11:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2 18/23] sys-rq: Add option to soft dump

On Mon, 08 Nov 2010 12:33:23 -0800 Mike Waychison wrote:

> 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 <[email protected]>

Hi,

Please add 'y' to Documentation/sysrq.txt .

> ---
> drivers/char/sysrq.c | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> index eaa5d3e..058d3c8 100644
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/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 */
> };


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2010-11-08 21:20:29

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 00/23] netoops support

From: Matt Mackall <[email protected]>
Date: Mon, 08 Nov 2010 14:55:27 -0600

> This much of your set looks very nice to me, but I'd like to get some
> more eyeballs on the first four. Dave?

If it's not sent to netdev, I'm less likely to see it :-)

2010-11-08 21:43:41

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 00/23] netoops support

On Mon, Nov 8, 2010 at 1:20 PM, David Miller <[email protected]> wrote:
> From: Matt Mackall <[email protected]>
> Date: Mon, 08 Nov 2010 14:55:27 -0600
>
>> This much of your set looks very nice to me, but I'd like to get some
>> more eyeballs on the first four. Dave?
>
> If it's not sent to netdev, I'm less likely to see it :-)
>

My bad :( I'll send it to netdev on the next version (unless told otherwise).

2010-11-08 22:28:24

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 18/23] sys-rq: Add option to soft dump

On Mon, Nov 8, 2010 at 1:09 PM, Randy Dunlap <[email protected]> wrote:
> On Mon, 08 Nov 2010 12:33:23 -0800 Mike Waychison wrote:
>
>> 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 <[email protected]>
>
> Hi,
>
> Please add 'y' to Documentation/sysrq.txt .

Ok.

Looking at the docs though, sparc64 has 'y' mapped to "Show global CPU
Registers". The only slot available is 'a' if the list is up to date.

An alternative may be to overload 'v' "Forcefully restores framebuffer
console" and "Causes ETM buffer dump [ARM-specific]" as these actions
seem to be about making crash data visible.

>
>> ---
>> ?drivers/char/sysrq.c | ? 14 +++++++++++++-
>> ?1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
>> index eaa5d3e..058d3c8 100644
>> --- a/drivers/char/sysrq.c
>> +++ b/drivers/char/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 */
>> ?};
>
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

2010-11-08 22:33:41

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2 18/23] sys-rq: Add option to soft dump

On Mon, 8 Nov 2010 14:27:59 -0800 Mike Waychison wrote:

> On Mon, Nov 8, 2010 at 1:09 PM, Randy Dunlap <[email protected]> wrote:
> > On Mon, 08 Nov 2010 12:33:23 -0800 Mike Waychison wrote:
> >
> >> 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 <[email protected]>
> >
> > Hi,
> >
> > Please add 'y' to Documentation/sysrq.txt .
>
> Ok.
>
> Looking at the docs though, sparc64 has 'y' mapped to "Show global CPU
> Registers". The only slot available is 'a' if the list is up to date.

'a' has a big fat warning not to use it.
'x' is powerpc-specific, just as 'y' is sparc64 specific.

DaveM, any comments/suggestions on these?

Looks like one day soon we'll have to make a way to allow more sysrq keys...


> An alternative may be to overload 'v' "Forcefully restores framebuffer
> console" and "Causes ETM buffer dump [ARM-specific]" as these actions
> seem to be about making crash data visible.
>
> >
> >> ---
> >> ?drivers/char/sysrq.c | ? 14 +++++++++++++-
> >> ?1 files changed, 13 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> >> index eaa5d3e..058d3c8 100644
> >> --- a/drivers/char/sysrq.c
> >> +++ b/drivers/char/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 */
> >> ?};
> >
> >
> > ---
> > ~Randy
> > *** Remember to use Documentation/SubmitChecklist when testing your code ***
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2010-11-09 01:28:33

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 00/23] netoops support

Mike Waychison <[email protected]> writes:

I'm still not fully convinced this needs to be a fully separate
mechanism from netconsole. It seems a lot of work to implement
basically log level.

Maybe netconsole could be patched to support only dumping KERN_EMERG?

But I guess the structured logging has some advantages, although
there have been certainly oops parsers (e.g. kerneloops.org) without it.

> * I am _NOT_ happy with the remaining userland ABIs presented in this
> patchset. Specifically the files "net_dump_now",
> "net_dump_one_shot", "netdump_fw_version", "netdump_board_name"
> and

fw_version and board_name is known by the kernel anyways through the DMI
interface, assuming the BIOS supplies that. We also dump it already
on a standard oops. So why not just use that directly?

As a general user ABI perhaps simply an unstructured string is better
that is included in the packet.

-Andi

--
[email protected] -- Speaking for myself only.

2010-11-09 03:31:38

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Mon, Nov 08, 2010 at 12:32:47PM -0800, Mike Waychison wrote:
>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.
>
>Signed-off-by: Mike Waychison <[email protected]>
>---
> drivers/net/Kconfig | 16 +++++++++++++---
> drivers/net/netconsole.c | 10 +++++-----
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>index f6668cd..b014cd6 100644
>--- a/drivers/net/Kconfig
>+++ b/drivers/net/Kconfig
>@@ -3367,14 +3367,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)
>@@ -3382,7 +3383,16 @@ config NETCONSOLE_DYNAMIC
> See <file:Documentation/networking/netconsole.txt> for details.
>
> config NETPOLL
>- def_bool NETCONSOLE
>+ def_bool false
>+
>+config NETPOLL_TARGETS
>+ bool
>+ select NETPOLL
>+
>+config NETPOLL_TARGETS_DYNAMIC
>+ bool
>+ select CONFIGFS_FS
>+ select NETPOLL_TARGETS
>

Hey, have you tried CONFIG_CONFIGFS_FS=m? :)

Actually, I tried almost the same thing, but finally failed due to
CONFIG_CONFIGFS_FS=m. NETPOLL can't be a module, and configfs can be,
thus you can't use the API provided by configfs.

So, either we need to de-modulize configfs or replace configfs API
with sysfs API. Personally, I prefer the former one, I don't think
configfs should be a module as long as it can provide API's
for other subsystems, like debugfs.

Thanks.

2010-11-09 04:21:01

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 00/23] netoops support

On Tue, Nov 09, 2010 at 02:28:26AM +0100, Andi Kleen wrote:
>Mike Waychison <[email protected]> writes:
>
>I'm still not fully convinced this needs to be a fully separate
>mechanism from netconsole. It seems a lot of work to implement
>basically log level.
>
>Maybe netconsole could be patched to support only dumping KERN_EMERG?
>

I tend to agree, IMHO, we can introduce another parameter like "oops_only"
to netconsole.


>But I guess the structured logging has some advantages, although
>there have been certainly oops parsers (e.g. kerneloops.org) without it.
>

+1

2010-11-09 04:22:56

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Tue, Nov 09, 2010 at 11:30:24AM +0800, Américo Wang wrote:
>
....
>
>So, either we need to de-modulize configfs or replace configfs API
>with sysfs API. Personally, I prefer the former one, I don't think
>configfs should be a module as long as it can provide API's
>for other subsystems, like debugfs.
>

To clarify, I meant "as long as the API it provides can be used by
other core subsystems".

2010-11-09 05:49:19

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v2 17/23] kmsg_dumper: Introduce a new 'SOFT' dump reason

Hi

> 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 <[email protected]>
> ---
>
> 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.
>
> TODO: Make sure mtdoops and ramoops do something useful with this flag?

Yes. If userland explicitly want to log, we have no reason to refuse it. :)
But, I don't think KMSG_DUMP_SOFT is good name because _SOFT don't explain
anything.



> ---
> 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,
> };
>
> /**
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


2010-11-09 05:54:59

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v2 17/23] kmsg_dumper: Introduce a new 'SOFT' dump reason

> Hi
>
> > 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 <[email protected]>
> > ---
> >
> > 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.
> >
> > TODO: Make sure mtdoops and ramoops do something useful with this flag?
>
> Yes. If userland explicitly want to log, we have no reason to refuse it. :)

I meant I think your change has no problem.




> But, I don't think KMSG_DUMP_SOFT is good name because _SOFT don't explain
> anything.


2010-11-09 08:34:52

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Mon, Nov 8, 2010 at 8:27 PM, Am?rico Wang <[email protected]> wrote:
> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Am?rico Wang wrote:
>>
> ....
>>
>>So, either we need to de-modulize configfs or replace configfs API
>>with sysfs API. Personally, I prefer the former one, I don't think
>>configfs should be a module as long as it can provide API's
>>for other subsystems, like debugfs.
>>
>
> To clarify, I meant "as long as the API it provides can be used by
> other core subsystems".
>

Ya, I see the problem with it being a tristate.

Why not just make netconsole support being compiled in force configfs
to be compiled in? Or does that just set bad precedent?

2010-11-09 09:02:10

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Tue, Nov 09, 2010 at 12:34:24AM -0800, Mike Waychison wrote:
>On Mon, Nov 8, 2010 at 8:27 PM, Américo Wang <[email protected]> wrote:
>> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Américo Wang wrote:
>>>
>> ....
>>>
>>>So, either we need to de-modulize configfs or replace configfs API
>>>with sysfs API. Personally, I prefer the former one, I don't think
>>>configfs should be a module as long as it can provide API's
>>>for other subsystems, like debugfs.
>>>
>>
>> To clarify, I meant "as long as the API it provides can be used by
>> other core subsystems".
>>
>
>Ya, I see the problem with it being a tristate.
>
>Why not just make netconsole support being compiled in force configfs
>to be compiled in? Or does that just set bad precedent?

That is what netconsole does now, and this is fine, since netconsole is
a module too, however, after you move that code into netpoll, then netpoll
will have a dependence on it, we will have problems.

I think we can let NETPOLL_TARGET depend on CONFIGFS_FS=y, but I still
see no reason why CONFIGFS_FS should be a module.

2010-11-09 09:34:15

by Cong Wang

[permalink] [raw]
Subject: [RFC PATCH] configfs: make it not be a module any more

On Tue, Nov 09, 2010 at 05:06:45PM +0800, Américo Wang wrote:
>On Tue, Nov 09, 2010 at 12:34:24AM -0800, Mike Waychison wrote:
>>On Mon, Nov 8, 2010 at 8:27 PM, Américo Wang <[email protected]> wrote:
>>> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Américo Wang wrote:
>>>>
>>> ....
>>>>
>>>>So, either we need to de-modulize configfs or replace configfs API
>>>>with sysfs API. Personally, I prefer the former one, I don't think
>>>>configfs should be a module as long as it can provide API's
>>>>for other subsystems, like debugfs.
>>>>
>>>
>>> To clarify, I meant "as long as the API it provides can be used by
>>> other core subsystems".
>>>
>>
>>Ya, I see the problem with it being a tristate.
>>
>>Why not just make netconsole support being compiled in force configfs
>>to be compiled in? Or does that just set bad precedent?
>
>That is what netconsole does now, and this is fine, since netconsole is
>a module too, however, after you move that code into netpoll, then netpoll
>will have a dependence on it, we will have problems.
>
>I think we can let NETPOLL_TARGET depend on CONFIGFS_FS=y, but I still
>see no reason why CONFIGFS_FS should be a module.
>

Okay, here we go.

------------------>

Netpoll will use configfs API's to provide generic
netpoll target support. These API's provided by
configfs could also be used by other core subsystems.
So, make configfs not a module any more, like debugfs.

Signed-off-by: WANG Cong <[email protected]>

---
diff --git a/fs/configfs/Kconfig b/fs/configfs/Kconfig
index 13587cc..4bdfe1e 100644
--- a/fs/configfs/Kconfig
+++ b/fs/configfs/Kconfig
@@ -1,5 +1,5 @@
config CONFIGFS_FS
- tristate "Userspace-driven configuration filesystem"
+ bool "Userspace-driven configuration filesystem"
depends on SYSFS
help
configfs is a ram-based filesystem that provides the converse
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index da6061a..714a192 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -66,7 +66,6 @@ extern int configfs_is_root(struct config_item *item);
extern struct inode * configfs_new_inode(mode_t mode, struct configfs_dirent *);
extern int configfs_create(struct dentry *, int mode, int (*init)(struct inode *));
extern int configfs_inode_init(void);
-extern void configfs_inode_exit(void);

extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
extern int configfs_make_dirent(struct configfs_dirent *,
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index 253476d..3508bc1 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -295,7 +295,3 @@ int __init configfs_inode_init(void)
return bdi_init(&configfs_backing_dev_info);
}

-void __exit configfs_inode_exit(void)
-{
- bdi_destroy(&configfs_backing_dev_info);
-}
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index 7d3607f..1cf57b3 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -111,7 +111,6 @@ static struct dentry *configfs_do_mount(struct file_system_type *fs_type,
}

static struct file_system_type configfs_fs_type = {
- .owner = THIS_MODULE,
.name = "configfs",
.mount = configfs_do_mount,
.kill_sb = kill_litter_super,
@@ -167,20 +166,5 @@ static int __init configfs_init(void)
out:
return err;
}
+core_initcall(configfs_init);

-static void __exit configfs_exit(void)
-{
- unregister_filesystem(&configfs_fs_type);
- kobject_put(config_kobj);
- kmem_cache_destroy(configfs_dir_cachep);
- configfs_dir_cachep = NULL;
- configfs_inode_exit();
-}
-
-MODULE_AUTHOR("Oracle");
-MODULE_LICENSE("GPL");
-MODULE_VERSION("0.0.2");
-MODULE_DESCRIPTION("Simple RAM filesystem for user driven kernel subsystem configuration.");
-
-module_init(configfs_init);
-module_exit(configfs_exit);

2010-11-09 12:10:13

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH v2 04/23] netconsole: Call netpoll_cleanup() in process context

On Mon, Nov 08, 2010 at 12:32:00PM -0800, Mike Waychison wrote:
> 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 <[email protected]>
> ---
> drivers/net/netconsole.c | 55 ++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 288a025..02ba5c4 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);
> +}
> +
Where is the synchronization on the new work queue when the module is getting
removed? The target get/put code does nothing to the module refcount, and
cleanup_netconsole just deletes targets, it doesn't block or fail on netconsole
refcounts, so you could run this work after the module has been removed and oops
the system.

Neil

2010-11-09 14:20:50

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Tue, Nov 09, 2010 at 05:06:45PM +0800, Am??rico Wang wrote:
> On Tue, Nov 09, 2010 at 12:34:24AM -0800, Mike Waychison wrote:
> >On Mon, Nov 8, 2010 at 8:27 PM, Am??rico Wang <[email protected]> wrote:
> >> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Am??rico Wang wrote:
> >>>
> >> ....
> >>>
> >>>So, either we need to de-modulize configfs or replace configfs API
> >>>with sysfs API. Personally, I prefer the former one, I don't think
> >>>configfs should be a module as long as it can provide API's
> >>>for other subsystems, like debugfs.
> >>>
> >>
> >> To clarify, I meant "as long as the API it provides can be used by
> >> other core subsystems".
> >>
> >
> >Ya, I see the problem with it being a tristate.
> >
> >Why not just make netconsole support being compiled in force configfs
> >to be compiled in? Or does that just set bad precedent?
>
> That is what netconsole does now, and this is fine, since netconsole is
> a module too, however, after you move that code into netpoll, then netpoll
> will have a dependence on it, we will have problems.
>
> I think we can let NETPOLL_TARGET depend on CONFIGFS_FS=y, but I still
> see no reason why CONFIGFS_FS should be a module.

No, just have the NETPOLL_TARGET set CONFIG_FS to y, don't force the
code to always be that way if the user isn't going to want that option.

So as it originally was should have been fine.

thanks,

greg k-h

2010-11-09 14:24:27

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH v2 20/23] netoops: Add x86 specific bits to packet headers

On Mon, Nov 08, 2010 at 12:33:35PM -0800, Mike Waychison wrote:
> We need to be able to gather information about the CPUs that caused the crash.
>
> This commit only handles x86, but it is desirable to come up with some new
> packet format that can accommodate any architecture.
>
> Signed-off-by: Mike Waychison <[email protected]>
> ---
> TODO: This should be made more general to other architectures. As is, we are
> probably okay exporting some value for the 'arch' field. Different
> architectures though will likely want to gather different data.
> ---
> drivers/net/netoops.c | 27 +++++++++++++++++++++------
> 1 files changed, 21 insertions(+), 6 deletions(-)
>
Not sure I see the value in encapsulating arch specific data in a netoops
message. Ostensibly this information can be inferred at the time of the crash
by the name/ip of the system crashing (one presumes that the sysadmin knows what
systems are what arch, or can look it up easily).

If thats not the case, why not just dump out the contents of /proc/cpuinfo in
ascii form, so that no arch specific data is needed?

Regards
Neil

> diff --git a/drivers/net/netoops.c b/drivers/net/netoops.c
> index dc1ee97..f63e12a 100644
> --- a/drivers/net/netoops.c
> +++ b/drivers/net/netoops.c
> @@ -69,16 +69,24 @@ struct netoops_msg {
> u32 packet_count;
> u32 packet_no;
> u32 __reserved1;
> - u8 __reserved2;
> - u8 __reserved3;
> - u8 __reserved4;
> + u8 x86_family;
> + u8 x86_model;
> + u8 x86_stepping;
> /*
> * NOTE: fixed length strings for a packet. NULL
> * termination not required.
> */
> char kernel_version[64];
> - char __reserved5[64];
> - char __reserved6[64];
> + char __reserved2[64];
> + char __reserved3[64];
> + /* NOTE: regs is 60 or 168 bytes */
> + struct pt_regs regs; /* arch specific. */
> + /*
> + * NOTE: The header is potentially ~385 bytes
> + * already. That doesn't leave much room for
> + * expansion unless we reduce the data size
> + * or truncate above fields.
> + */
> } __attribute__ ((packed)) header;
> char data[NETOOPS_DATA_BYTES];
> } __attribute__ ((packed));
> @@ -94,10 +102,17 @@ static void setup_packet_header(int packet_count, int soft_dump)
> msg.header.dump_id = (jiffies/HZ) & 0xffff;
> msg.header.packet_count = packet_count;
> msg.header.header_size = sizeof(msg.header);
> +#ifndef CONFIG_UML
> + msg.header.x86_family = current_cpu_data.x86;
> + msg.header.x86_model = current_cpu_data.x86_model;
> + msg.header.x86_stepping = current_cpu_data.x86_mask;
> +#endif
> strncpy(msg.header.kernel_version,
> utsname()->release,
> min(sizeof(msg.header.kernel_version),
> sizeof(utsname()->release)));
> + if (regs != NULL)
> + memcpy(&msg.header.regs, regs, sizeof(msg.header.regs));
> }
>
> static int packet_count_from_length(unsigned long l)
> @@ -182,7 +197,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++) {
>
>

2010-11-09 17:19:22

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 04/23] netconsole: Call netpoll_cleanup() in process context

On Tue, Nov 9, 2010 at 4:07 AM, Neil Horman <[email protected]> wrote:
> On Mon, Nov 08, 2010 at 12:32:00PM -0800, Mike Waychison wrote:
>> +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);
>> +}
>> +
> Where is the synchronization on the new work queue when the module is getting
> removed? The target get/put code does nothing to the module refcount, and
> cleanup_netconsole just deletes targets, it doesn't block or fail on netconsole
> refcounts, so you could run this work after the module has been removed and oops
> the system.
>

The synchronization here is actually handled in free_param_target()
with the call to cancel_work_sync(). After that call is made in the
exit path, we know a task cannot be rescheduled for cleanup, so we can
clean things up directly.

The comment/name of free_param_target should probably change. I used
to have this cancel_work_sync() call elsewhere, and folded both the
dynamic and param-based target cleanup into one as they ended up being
the same.

Removal of the item from configfs however isn't handled.

How about something like (no idea if this will get whitespace damage):


diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 02ba5c4..ddd5e4f 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -686,13 +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.
*
- * If it queued for cleanup already, that is fine, as that path holds a
- * reference to the config_item.
+ * 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);



I can fold this diff snippet into this patch for the next round if you
agree it plugs the remaining hole.

2010-11-09 17:24:59

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Tue, Nov 9, 2010 at 6:20 AM, Greg KH <[email protected]> wrote:
> On Tue, Nov 09, 2010 at 05:06:45PM +0800, Am??rico Wang wrote:
>> On Tue, Nov 09, 2010 at 12:34:24AM -0800, Mike Waychison wrote:
>> >On Mon, Nov 8, 2010 at 8:27 PM, Am??rico Wang <[email protected]> wrote:
>> >> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Am??rico Wang wrote:
>> >>>
>> >> ....
>> >>>
>> >>>So, either we need to de-modulize configfs or replace configfs API
>> >>>with sysfs API. Personally, I prefer the former one, I don't think
>> >>>configfs should be a module as long as it can provide API's
>> >>>for other subsystems, like debugfs.
>> >>>
>> >>
>> >> To clarify, I meant "as long as the API it provides can be used by
>> >> other core subsystems".
>> >>
>> >
>> >Ya, I see the problem with it being a tristate.
>> >
>> >Why not just make netconsole support being compiled in force configfs
>> >to be compiled in? ?Or does that just set bad precedent?
>>
>> That is what netconsole does now, and this is fine, since netconsole is
>> a module too, however, after you move that code into netpoll, then netpoll
>> will have a dependence on it, we will have problems.
>>
>> I think we can let NETPOLL_TARGET depend on CONFIGFS_FS=y, but I still
>> see no reason why CONFIGFS_FS should be a module.
>
> No, just have the NETPOLL_TARGET set CONFIG_FS to y, don't force the
> code to always be that way if the user isn't going to want that option.

Can you clarify what you mean by "that way" here?

Presumably, only NETPOLL_TARGET_DYNAMIC needs CONFIGFS_FS=y ?

Maybe none of this matters: NETPOLL_TARGET itself *can* be a module,
as it doesn't do anything more than stack a target management layer on
top of the existing netpoll code. Nothing in this patchset actually
changes anything within netpoll itself.

Mike Waychison

2010-11-09 17:34:15

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 12/23] netpoll: Introduce netpoll_target configs

On Tue, Nov 09, 2010 at 09:24:33AM -0800, Mike Waychison wrote:
> On Tue, Nov 9, 2010 at 6:20 AM, Greg KH <[email protected]> wrote:
> > On Tue, Nov 09, 2010 at 05:06:45PM +0800, Am??rico Wang wrote:
> >> On Tue, Nov 09, 2010 at 12:34:24AM -0800, Mike Waychison wrote:
> >> >On Mon, Nov 8, 2010 at 8:27 PM, Am??rico Wang <[email protected]> wrote:
> >> >> On Tue, Nov 09, 2010 at 11:30:24AM +0800, Am??rico Wang wrote:
> >> >>>
> >> >> ....
> >> >>>
> >> >>>So, either we need to de-modulize configfs or replace configfs API
> >> >>>with sysfs API. Personally, I prefer the former one, I don't think
> >> >>>configfs should be a module as long as it can provide API's
> >> >>>for other subsystems, like debugfs.
> >> >>>
> >> >>
> >> >> To clarify, I meant "as long as the API it provides can be used by
> >> >> other core subsystems".
> >> >>
> >> >
> >> >Ya, I see the problem with it being a tristate.
> >> >
> >> >Why not just make netconsole support being compiled in force configfs
> >> >to be compiled in? ?Or does that just set bad precedent?
> >>
> >> That is what netconsole does now, and this is fine, since netconsole is
> >> a module too, however, after you move that code into netpoll, then netpoll
> >> will have a dependence on it, we will have problems.
> >>
> >> I think we can let NETPOLL_TARGET depend on CONFIGFS_FS=y, but I still
> >> see no reason why CONFIGFS_FS should be a module.
> >
> > No, just have the NETPOLL_TARGET set CONFIG_FS to y, don't force the
> > code to always be that way if the user isn't going to want that option.
>
> Can you clarify what you mean by "that way" here?

I think I can't remember, as this is a bit complex.

I'm just objecting to the changing of configfs to only be able to be
built into the kernel. If this patch set requires configfs, great, then
set the value to "Y", but don't change configfs like the proposed patch
later in this email thread did.

thanks,

greg k-h

2010-11-09 17:56:29

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 20/23] netoops: Add x86 specific bits to packet headers

On Tue, Nov 9, 2010 at 6:22 AM, Neil Horman <[email protected]> wrote:
> On Mon, Nov 08, 2010 at 12:33:35PM -0800, Mike Waychison wrote:
>> We need to be able to gather information about the CPUs that caused the crash.
>>
>> This commit only handles x86, but it is desirable to come up with some new
>> packet format that can accommodate any architecture.
>>
>> Signed-off-by: Mike Waychison <[email protected]>
>> ---
>> TODO: This should be made more general to other architectures. ?As is, we are
>> probably okay exporting some value for the 'arch' field. ?Different
>> architectures though will likely want to gather different data.
>> ---
>> ?drivers/net/netoops.c | ? 27 +++++++++++++++++++++------
>> ?1 files changed, 21 insertions(+), 6 deletions(-)
>>
> Not sure I see the value in encapsulating arch specific data in a netoops
> message. ?Ostensibly this information can be inferred at the time of the crash
> by the name/ip of the system crashing (one presumes that the sysadmin knows what
> systems are what arch, or can look it up easily).

This actually becomes harder than it appears at first. The
distributed nature of our systems means that we cannot ever rely on a
central data source that describes the machines we have without having
to worry about network partitions and service downtimes. The
alternative is to post-process crashes, looking up machine information
in various data sources and hoping that the results are consistent.
This becomes yet another job in the cluster, which seems a little
silly when we could just have the machine self describe itself at the
time of the crash.

>
> If thats not the case, why not just dump out the contents of /proc/cpuinfo in
> ascii form, so that no arch specific data is needed?

As a segment of the dump? I'm okay with doing this, as long it never
makes it's way into log_buf. log_buf is a real pain to parse given
the lack of transactions and the fact that many other cores may be
scribbling all over it.

A couple years ago, we speced out a different wire protocol for these
packets, version 3 (yes, this has already had a version bump).
Anyhow, we came up with a design that used (key,length)->value fields.
Keys were designed to be 16bit wide integers and clients could
easily ignore fields that it doesn't understand. We never implemented
this, but it'd be great if folks bought into it. It'd allow us to
ship things like file contents side by side with other structured
fields like pt_regs snapshots, the log_buf and a user defined buffer.

How do folks feel about something like that?

2010-11-09 19:35:48

by Neil Horman

[permalink] [raw]
Subject: Re: [PATCH v2 04/23] netconsole: Call netpoll_cleanup() in process context

On Tue, Nov 09, 2010 at 09:18:57AM -0800, Mike Waychison wrote:
> On Tue, Nov 9, 2010 at 4:07 AM, Neil Horman <[email protected]> wrote:
> > On Mon, Nov 08, 2010 at 12:32:00PM -0800, Mike Waychison wrote:
> >> +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);
> >> +}
> >> +
> > Where is the synchronization on the new work queue when the module is getting
> > removed? The target get/put code does nothing to the module refcount, and
> > cleanup_netconsole just deletes targets, it doesn't block or fail on netconsole
> > refcounts, so you could run this work after the module has been removed and oops
> > the system.
> >
>
> The synchronization here is actually handled in free_param_target()
> with the call to cancel_work_sync(). After that call is made in the
> exit path, we know a task cannot be rescheduled for cleanup, so we can
> clean things up directly.
>
> The comment/name of free_param_target should probably change. I used
> to have this cancel_work_sync() call elsewhere, and folded both the
> dynamic and param-based target cleanup into one as they ended up being
> the same.
>
> Removal of the item from configfs however isn't handled.
>
> How about something like (no idea if this will get whitespace damage):
>
That would seem clearer to me yes. Thanks!
Neil

>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 02ba5c4..ddd5e4f 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -686,13 +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.
> *
> - * If it queued for cleanup already, that is fine, as that path holds a
> - * reference to the config_item.
> + * 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);
>
>
>
> I can fold this diff snippet into this patch for the next round if you
> agree it plugs the remaining hole.
>