netconsole management paths are scattered around in different places.
This patch reorganizes them so that
* All enable logic is in netconsole_enable() and disable in
netconsole_disable(). Both should be called with netconsole_mutex
held and netconsole_disable() may be invoked without intervening
enable.
* alloc_param_target() now also handles linking the new target to
target_list. It's renamed to create_param_target() and now returns
errno.
* store_enabled() now uses netconsole_enable/disable() instead of
open-coding the logic. This also fixes the missing synchronization
against write path when manipulating ->enabled flag.
* drop_netconsole_target() and netconsole_deferred_disable_work_fn()
updated to use netconsole_disable().
* init/cleanup_netconsole()'s destruction paths are consolidated into
netconsole_destroy_all() which uses netconsole_disable().
free_param_target() is no longer used and removed.
While the conversions aren't one-to-one equivalent, this patch
shouldn't cause any visible behavior differences and makes extension
of the enable/disable logic a lot easier.
Signed-off-by: Tejun Heo <t...@kernel.org>
Cc: David Miller <da...@davemloft.net>
---
drivers/net/netconsole.c | 147 +++++++++++++++++++++++++----------------------
1 file changed, 79 insertions(+), 68 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index f0ac9f6..d72d902 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -83,6 +83,8 @@ static LIST_HEAD(target_list);
/* protects target creation/destruction and enable/disable */
static DEFINE_MUTEX(netconsole_mutex);
+static struct console netconsole;
+
/**
* struct netconsole_target - Represents a configured netconsole target.
* @list: Links this target into the target_list.
@@ -192,8 +194,43 @@ static struct netconsole_target
*alloc_netconsole_target(void)
return nt;
}
+static int netconsole_enable(struct netconsole_target *nt)
+{
+ int err;
+
+ lockdep_assert_held(&netconsole_mutex);
+ WARN_ON_ONCE(nt->enabled);
+
+ err = netpoll_setup(&nt->np);
+ if (err)
+ return err;
+
+ console_lock();
+ nt->enabled = true;
+ console_unlock();
+ return 0;
+}
+
+static void netconsole_disable(struct netconsole_target *nt)
+{
+ lockdep_assert_held(&netconsole_mutex);
+
+ /*
+ * We need to disable the netconsole before cleaning it up
+ * otherwise we might end up in write_msg() with !nt->np.dev &&
+ * nt->enabled.
+ */
+ if (nt->enabled) {
+ console_lock();
+ nt->enabled = false;
+ console_unlock();
+
+ netpoll_cleanup(&nt->np);
+ }
+}
+
/* Allocate new target (from boot/module param) and setup netpoll for it */
-static struct netconsole_target *alloc_param_target(char *target_config)
+static int create_param_target(char *target_config)
{
int err = -ENOMEM;
struct netconsole_target *nt;
@@ -209,24 +246,26 @@ static struct netconsole_target *alloc_param_target(char
*target_config)
if (err)
goto fail;
- err = netpoll_setup(&nt->np);
+ console_lock();
+ list_add(&nt->list, &target_list);
+ console_unlock();
+
+ err = netconsole_enable(nt);
if (err)
- goto fail;
+ goto fail_del;
- nt->enabled = true;
+ /* Dump existing printks when we register */
+ netconsole.flags |= CON_PRINTBUFFER;
- return nt;
+ return 0;
+fail_del:
+ console_lock();
+ list_del(&nt->list);
+ console_unlock();
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 netconsole_target *nt)
-{
- netpoll_cleanup(&nt->np);
- kfree(nt);
+ return err;
}
#ifdef CONFIG_NETCONSOLE_DYNAMIC
@@ -350,24 +389,15 @@ static ssize_t store_enabled(struct netconsole_target *nt,
*/
netpoll_print_options(&nt->np);
- err = netpoll_setup(&nt->np);
+ err = netconsole_enable(nt);
if (err)
return err;
pr_info("netconsole: network logging started\n");
} else { /* false */
- /* We need to disable the netconsole before cleaning it up
- * otherwise we might end up in write_msg() with
- * nt->np.dev == NULL and nt->enabled == true
- */
- console_lock();
- nt->enabled = false;
- console_unlock();
- netpoll_cleanup(&nt->np);
+ netconsole_disable(nt);
}
- nt->enabled = enabled;
-
return strnlen(buf, count);
}
@@ -633,13 +663,7 @@ static void drop_netconsole_target(struct config_group
*group,
list_del(&nt->list);
console_unlock();
- /*
- * 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)
- netpoll_cleanup(&nt->np);
-
+ netconsole_disable(nt);
config_item_put(&nt->item);
mutex_unlock(&netconsole_mutex);
}
@@ -675,22 +699,16 @@ repeat:
to_disable = NULL;
console_lock();
list_for_each_entry(nt, &target_list, list) {
- if (!nt->disable_scheduled)
- continue;
- nt->disable_scheduled = false;
-
- if (!nt->enabled)
- continue;
-
- netconsole_target_get(nt);
- nt->enabled = false;
- to_disable = nt;
- break;
+ if (nt->disable_scheduled) {
+ nt->disable_scheduled = false;
+ netconsole_target_get(nt);
+ to_disable = nt;
+ }
}
console_unlock();
if (to_disable) {
- netpoll_cleanup(&to_disable->np);
+ netconsole_disable(to_disable);
netconsole_target_put(to_disable);
goto repeat;
}
@@ -795,10 +813,23 @@ static struct console netconsole = {
.write = write_msg,
};
+static void __init_or_module netconsole_destroy_all(void)
+{
+ struct netconsole_target *nt, *tmp;
+
+ lockdep_assert_held(&netconsole_mutex);
+
+ /* targets are already inactive, skipping the console lock is safe */
+ list_for_each_entry_safe(nt, tmp, &target_list, list) {
+ list_del(&nt->list);
+ netconsole_disable(nt);
+ kfree(nt);
+ }
+}
+
static int __init init_netconsole(void)
{
int err;
- struct netconsole_target *nt, *tmp;
char *target_config;
char *input = config;
@@ -806,17 +837,9 @@ static int __init init_netconsole(void)
if (strnlen(input, MAX_PARAM_LENGTH)) {
while ((target_config = strsep(&input, ";"))) {
- nt = alloc_param_target(target_config);
- if (IS_ERR(nt)) {
- err = PTR_ERR(nt);
+ err = create_param_target(target_config);
+ if (err)
goto fail;
- }
- /* Dump existing printks when we register */
- netconsole.flags |= CON_PRINTBUFFER;
-
- console_lock();
- list_add(&nt->list, &target_list);
- console_unlock();
}
}
@@ -838,12 +861,7 @@ undonotifier:
unregister_netdevice_notifier(&netconsole_netdev_notifier);
fail:
pr_err("cleaning up\n");
-
- /* targets are already inactive, skipping the console lock is safe */
- list_for_each_entry_safe(nt, tmp, &target_list, list) {
- list_del(&nt->list);
- free_param_target(nt);
- }
+ netconsole_destroy_all();
mutex_unlock(&netconsole_mutex);
cancel_work_sync(&netconsole_deferred_disable_work);
return err;
@@ -851,19 +869,12 @@ fail:
static void __exit cleanup_netconsole(void)
{
- struct netconsole_target *nt, *tmp;
-
mutex_lock(&netconsole_mutex);
unregister_console(&netconsole);
dynamic_netconsole_exit();
unregister_netdevice_notifier(&netconsole_netdev_notifier);
-
- /* targets are already inactive, skipping the console lock is safe */
- list_for_each_entry_safe(nt, tmp, &target_list, list) {
- list_del(&nt->list);
- free_param_target(nt);
- }
+ netconsole_destroy_all();
mutex_unlock(&netconsole_mutex);
cancel_work_sync(&netconsole_deferred_disable_work);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
