lkml.org 
[lkml]   [2015]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 11/16] netconsole: consolidate enable/disable and create/destroy paths
Date
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 <tj@kernel.org>
Cc: David Miller <davem@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


\
 
 \ /
  Last update: 2015-04-17 01:21    [W:0.222 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site