From: Mike Waychison <mikew@google.com>
To: simon.kagstrom@netinsight.net, davem@davemloft.net,
nhorman@tuxdriver.com, Matt Mackall <mpm@selenic.com>
Cc: adurbin@google.com, linux-kernel@vger.kernel.org,
chavey@google.com, "Greg KH" <greg@kroah.com>,
netdev@vger.kernel.org, "Américo Wang" <xiyou.wangcong@gmail.com>,
akpm@linux-foundation.org, linux-api@vger.kernel.org
Subject: [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields
Date: Tue, 14 Dec 2010 13:29:04 -0800 [thread overview]
Message-ID: <20101214212904.17022.16604.stgit@mike.mtv.corp.google.com> (raw)
In-Reply-To: <20101214212846.17022.64836.stgit@mike.mtv.corp.google.com>
The netconsole driver currently doesn't do any locking over its
configuration fields. This can cause problems if we were to ever have
concurrent writing to fields while somebody is enabling the service.
For simplicity, this patch extends targets_list_lock to cover all
configuration fields within the targets. Macros are also added here to
wrap accessors so that we check whether the target has been enabled with
locking handled.
Signed-off-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Mackall <mpm@selenic.com>
---
drivers/net/netconsole.c | 114 ++++++++++++++++++++++++++--------------------
1 files changed, 64 insertions(+), 50 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index c87a49e..6e16888 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -327,6 +327,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
const char *buf,
size_t count)
{
+ unsigned long flags;
int err;
long enabled;
@@ -335,6 +336,10 @@ static ssize_t store_enabled(struct netconsole_target *nt,
return enabled;
if (enabled) { /* 1 */
+ spin_lock_irqsave(&target_list_lock, flags);
+ if (nt->enabled)
+ goto busy;
+ spin_unlock_irqrestore(&target_list_lock, flags);
/*
* 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);
next prev parent reply other threads:[~2010-12-14 21:29 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-14 21:28 [PATCH v3 00/22] netoops support Mike Waychison
2010-12-14 21:28 ` [PATCH v3 01/22] netconsole: Remove unneeded reference counting Mike Waychison
2010-12-14 21:29 ` Mike Waychison [this message]
2010-12-14 23:15 ` [PATCH v3 02/22] netconsole: Introduce locking over the netpoll fields Michał Mirosław
2010-12-14 23:30 ` Mike Waychison
2010-12-14 21:29 ` [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine Mike Waychison
2010-12-14 23:21 ` Michał Mirosław
2010-12-14 23:33 ` Mike Waychison
2010-12-17 20:52 ` Neil Horman
2010-12-17 21:20 ` Mike Waychison
2010-12-14 21:29 ` [PATCH v3 04/22] netconsole: Call netpoll_cleanup() in process context Mike Waychison
2010-12-14 21:29 ` [PATCH v3 05/22] netconsole: Wrap the list and locking in a structure Mike Waychison
2010-12-14 21:29 ` [PATCH v3 06/22] netconsole: Push configfs_subsystem into netpoll_targets Mike Waychison
2010-12-14 21:29 ` [PATCH v3 07/22] netconsole: Move netdev_notifier " Mike Waychison
2010-12-14 21:29 ` [PATCH v3 08/22] netconsole: Split out netpoll_targets init/exit Mike Waychison
2010-12-14 21:29 ` [PATCH v3 09/22] netconsole: Add pointer to netpoll_targets Mike Waychison
2010-12-14 21:29 ` [PATCH v3 10/22] netconsole: Rename netconsole_target -> netpoll_target Mike Waychison
2010-12-14 21:29 ` [PATCH v3 11/22] netconsole: Abstract away the subsystem name Mike Waychison
2010-12-14 21:29 ` [PATCH v3 12/22] netconsole: Move setting of default ports Mike Waychison
2010-12-14 21:30 ` [PATCH v3 13/22] netpoll: Introduce netpoll_target configs Mike Waychison
2010-12-14 21:30 ` [PATCH v3 14/22] netpoll: Move target code into netpoll_targets.c Mike Waychison
2010-12-15 0:07 ` Mike Waychison
2010-12-14 21:30 ` [PATCH v3 15/22] Oops: Pass regs to oops_exit() Mike Waychison
2010-12-14 21:30 ` [PATCH v3 16/22] kmsg_dumper: Pass pt_regs along to dumpers Mike Waychison
2010-12-14 21:30 ` [PATCH v3 17/22] kmsg_dumper: Introduce a new 'SOFT' dump reason Mike Waychison
2010-12-14 21:30 ` [PATCH v3 18/22] sys-rq: Add option to soft dump Mike Waychison
2010-12-14 21:30 ` [PATCH v3 19/22] netoops: add core functionality Mike Waychison
2010-12-14 21:30 ` [PATCH v3 20/22] netoops: Add x86 specific bits to packet headers Mike Waychison
2010-12-14 21:30 ` [PATCH v3 21/22] netoops: Add user-programmable boot_id Mike Waychison
2010-12-14 21:42 ` Matt Mackall
2010-12-14 21:59 ` Mike Waychison
2010-12-14 22:06 ` Matt Mackall
2010-12-14 22:33 ` Mike Waychison
2010-12-14 22:47 ` Matt Mackall
2010-12-14 23:19 ` Mike Waychison
2010-12-15 2:16 ` Ted Ts'o
2010-12-14 21:30 ` [PATCH v3 22/22] netoops: Add a user programmable blob to the netoops packet Mike Waychison
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101214212904.17022.16604.stgit@mike.mtv.corp.google.com \
--to=mikew@google.com \
--cc=adurbin@google.com \
--cc=akpm@linux-foundation.org \
--cc=chavey@google.com \
--cc=davem@davemloft.net \
--cc=greg@kroah.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpm@selenic.com \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=simon.kagstrom@netinsight.net \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome