netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: akpm@linux-foundation.org, davem@davemloft.net
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 08/16] netconsole: punt disabling to workqueue from netdevice_notifier
Date: Thu, 16 Apr 2015 19:03:45 -0400	[thread overview]
Message-ID: <1429225433-11946-9-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1429225433-11946-1-git-send-email-tj@kernel.org>

The netdevice_notifier callback, netconsole_netdev_event(), needs to
perform netpoll_cleanup() for the affected targets; however, the
notifier is called with rtnl_lock held which the netpoll_cleanup()
path also grabs.  To avoid deadlock, the path uses __netpoll_cleanup()
instead and making the code path different from others.

The planned reliable netconsole support will add more logic to the
cleanup path making the slightly different paths painful.  Let's punt
netconsole_target disabling to workqueue so that it can later share
the same cleanup path.  This would also allow ditching
target_list_lock and depending on console_lock for synchronization.

Note that this introduces a narrow race window where the asynchronous
disabling may race against disabling from configfs ending up executing
netpoll_cleanup() more than once on the same instance.  The follow up
patches will fix it by introducing a mutex to protect overall enable /
disable operations; unfortunately, the locking update couldn't be
ordered before this change due to the locking order with rtnl_lock.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: David Miller <davem@davemloft.net>
---
 drivers/net/netconsole.c | 58 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 17692b8..d355776 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -105,6 +105,7 @@ struct netconsole_target {
 	struct config_item	item;
 #endif
 	bool			enabled;
+	bool			disable_scheduled;
 	struct mutex		mutex;
 	struct netpoll		np;
 };
@@ -660,6 +661,39 @@ static struct configfs_subsystem netconsole_subsys = {
 
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
 
+static void netconsole_deferred_disable_work_fn(struct work_struct *work)
+{
+	struct netconsole_target *nt, *to_disable;
+	unsigned long flags;
+
+repeat:
+	to_disable = NULL;
+	spin_lock_irqsave(&target_list_lock, flags);
+	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;
+	}
+	spin_unlock_irqrestore(&target_list_lock, flags);
+
+	if (to_disable) {
+		netpoll_cleanup(&to_disable->np);
+		netconsole_target_put(to_disable);
+		goto repeat;
+	}
+}
+
+static DECLARE_WORK(netconsole_deferred_disable_work,
+		    netconsole_deferred_disable_work_fn);
+
 /* Handle network interface device notifications */
 static int netconsole_netdev_event(struct notifier_block *this,
 				   unsigned long event, void *ptr)
@@ -674,9 +708,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
 		goto done;
 
 	spin_lock_irqsave(&target_list_lock, flags);
-restart:
 	list_for_each_entry(nt, &target_list, list) {
-		netconsole_target_get(nt);
 		if (nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
@@ -685,23 +717,14 @@ restart:
 			case NETDEV_RELEASE:
 			case NETDEV_JOIN:
 			case NETDEV_UNREGISTER:
-				/* rtnl_lock already held
-				 * we might sleep in __netpoll_cleanup()
-				 */
-				spin_unlock_irqrestore(&target_list_lock, flags);
-
-				__netpoll_cleanup(&nt->np);
-
-				spin_lock_irqsave(&target_list_lock, flags);
-				dev_put(nt->np.dev);
-				nt->np.dev = NULL;
-				nt->enabled = false;
+				/* rtnl_lock already held, punt to workqueue */
+				if (nt->enabled && !nt->disable_scheduled) {
+					nt->disable_scheduled = true;
+					schedule_work(&netconsole_deferred_disable_work);
+				}
 				stopped = true;
-				netconsole_target_put(nt);
-				goto restart;
 			}
 		}
-		netconsole_target_put(nt);
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
 	if (stopped) {
@@ -810,7 +833,7 @@ static int __init init_netconsole(void)
 
 undonotifier:
 	unregister_netdevice_notifier(&netconsole_netdev_notifier);
-
+	cancel_work_sync(&netconsole_deferred_disable_work);
 fail:
 	pr_err("cleaning up\n");
 
@@ -834,6 +857,7 @@ static void __exit cleanup_netconsole(void)
 	unregister_console(&netconsole);
 	dynamic_netconsole_exit();
 	unregister_netdevice_notifier(&netconsole_netdev_notifier);
+	cancel_work_sync(&netconsole_deferred_disable_work);
 
 	/*
 	 * Targets created via configfs pin references on our module
-- 
2.1.0

  parent reply	other threads:[~2015-04-16 23:04 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-16 23:03 [PATCHSET] printk, netconsole: implement reliable netconsole Tejun Heo
2015-04-16 23:03 ` [PATCH 01/16] printk: guard the amount written per line by devkmsg_read() Tejun Heo
2015-04-20 12:11   ` Petr Mladek
2015-04-20 12:33     ` Petr Mladek
2015-04-16 23:03 ` [PATCH 02/16] printk: factor out message formatting from devkmsg_read() Tejun Heo
2015-04-20 12:30   ` Petr Mladek
2015-04-16 23:03 ` [PATCH 03/16] printk: move LOG_NOCONS skipping into call_console_drivers() Tejun Heo
2015-04-20 12:50   ` Petr Mladek
2015-04-16 23:03 ` [PATCH 04/16] printk: implement support for extended console drivers Tejun Heo
2015-04-20 15:43   ` Petr Mladek
2015-04-21 10:03     ` Petr Mladek
2015-04-27 21:09     ` Tejun Heo
2015-04-28  9:42       ` Petr Mladek
2015-04-28 14:10         ` Tejun Heo
2015-04-28 14:24           ` Petr Mladek
2015-04-16 23:03 ` [PATCH 05/16] printk: implement log_seq_range() and ext_log_from_seq() Tejun Heo
2015-04-16 23:03 ` [PATCH 06/16] netconsole: make netconsole_target->enabled a bool Tejun Heo
2015-04-16 23:03 ` [PATCH 07/16] netconsole: factor out alloc_netconsole_target() Tejun Heo
2015-04-16 23:03 ` Tejun Heo [this message]
2015-04-16 23:03 ` [PATCH 09/16] netconsole: replace target_list_lock with console_lock Tejun Heo
2015-04-16 23:03 ` [PATCH 10/16] netconsole: introduce netconsole_mutex Tejun Heo
2015-04-16 23:03 ` [PATCH 11/16] netconsole: consolidate enable/disable and create/destroy paths Tejun Heo
2015-04-16 23:03 ` [PATCH 12/16] netconsole: implement extended console support Tejun Heo
2015-04-16 23:03 ` [PATCH 13/16] netconsole: implement retransmission support for extended consoles Tejun Heo
2015-04-16 23:03 ` [PATCH 14/16] netconsole: implement ack handling and emergency transmission Tejun Heo
2015-04-16 23:03 ` [PATCH 15/16] netconsole: implement netconsole receiver library Tejun Heo
2015-04-16 23:03 ` [PATCH 16/16] netconsole: update documentation for extended netconsole Tejun Heo
2015-04-17 15:35 ` [PATCHSET] printk, netconsole: implement reliable netconsole Tetsuo Handa
2015-04-17 16:28   ` Tejun Heo
2015-04-17 17:17     ` David Miller
2015-04-17 17:37       ` Tejun Heo
2015-04-17 17:43         ` Tetsuo Handa
2015-04-17 17:45           ` Tejun Heo
2015-04-17 18:03             ` Tetsuo Handa
2015-04-17 18:07               ` Tejun Heo
2015-04-17 18:20                 ` Tetsuo Handa
2015-04-17 18:26                   ` Tejun Heo
2015-04-18 13:09                     ` Tetsuo Handa
2015-04-17 18:04         ` Tejun Heo
2015-04-17 18:55         ` David Miller
2015-04-17 19:52           ` Tejun Heo
2015-04-17 20:06             ` David Miller
2015-04-21 21:51       ` Stephen Hemminger
2015-04-19  7:25 ` Rob Landley
2015-04-20 12:00   ` David Laight
2015-04-20 14:33   ` Tejun Heo

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=1429225433-11946-9-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).