From patchwork Fri May 9 17:47:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 347496 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 54575140083 for ; Sat, 10 May 2014 03:47:44 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756149AbaEIRrk (ORCPT ); Fri, 9 May 2014 13:47:40 -0400 Received: from mail-pa0-f44.google.com ([209.85.220.44]:43761 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754465AbaEIRrj (ORCPT ); Fri, 9 May 2014 13:47:39 -0400 Received: by mail-pa0-f44.google.com with SMTP id ld10so4631003pab.17 for ; Fri, 09 May 2014 10:47:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=rB1fPmHD5uoHd7ygb8BFuh9teJr/xqkUKVSikOvdmvI=; b=nUhCyShF1FPmLZKqVtp38wQCDouLlBS37McF+uFrXgtva+sRNZf7JU1ZzrLubZkFeA x2vryCZP+5TTFzOwcyAQ0AJB8jBXVYBdWwK2i1/garzHNaHG97eiM3E9pnabZSY1IPHK Qh/2kceiKVXZtmUm67LgFAXWq8cgYM2PtsndPPhRn6C88OIBk2Qw6dAVHSynyBL+L50p XksDko3vC8MHD8uo5GJv7j9PaMFcAVC5IeQjRUpqIey98yWVDtb0scNXVl008EOQsdMC yJSrSsapJ8tOiwAIFiH0yFUaKtxbRPJwXVxo+82bRXosKrnzvYSyEYggikafRIybJZcv i3PQ== X-Received: by 10.66.254.166 with SMTP id aj6mr22994741pad.11.1399657658576; Fri, 09 May 2014 10:47:38 -0700 (PDT) Received: from localhost.net ([8.25.197.25]) by mx.google.com with ESMTPSA id ja8sm8703487pbd.3.2014.05.09.10.47.37 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 May 2014 10:47:37 -0700 (PDT) From: Cong Wang To: netdev@vger.kernel.org Cc: "David S. Miller" , Cong Wang , "Eric W. Biederman" , Cong Wang Subject: [Patch net] rtnetlink: call rtnl_lock_unregistering() in rtnl_link_unregister() Date: Fri, 9 May 2014 10:47:33 -0700 Message-Id: <1399657653-4909-1-git-send-email-xiyou.wangcong@gmail.com> X-Mailer: git-send-email 1.8.3.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Cong Wang commit 50624c934db18ab90 (net: Delay default_device_exit_batch until no devices are unregistering) introduced rtnl_lock_unregistering() for default_device_exit_batch(). Same race could happen we when rmmod a driver which calls rtnl_link_unregister() as we call dev->destructor without rtnl lock. I know making rtnl_lock_unregistering() a macro is ugly, but I don't find any less ugly way to fix it unless we duplicate the code. For long term, I think we should clean up the mess of netdev_run_todo() and net namespce exit code. Cc: Eric W. Biederman Cc: David S. Miller Signed-off-by: Cong Wang Signed-off-by: Cong Wang --- include/linux/rtnetlink.h | 31 +++++++++++++++++++++++++++++++ net/core/dev.c | 32 ++------------------------------ net/core/rtnetlink.c | 2 +- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h index 8e3e66a..e70218d 100644 --- a/include/linux/rtnetlink.h +++ b/include/linux/rtnetlink.h @@ -4,6 +4,7 @@ #include #include +#include #include extern int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo); @@ -22,6 +23,36 @@ extern void rtnl_lock(void); extern void rtnl_unlock(void); extern int rtnl_trylock(void); extern int rtnl_is_locked(void); + +extern wait_queue_head_t netdev_unregistering_wq; +/* Return with the rtnl_lock held when there are no network + * devices unregistering in any network namespace in net_list. + */ +#define rtnl_lock_unregistering(net_list, member) \ +do { \ + struct net *net; \ + bool unregistering; \ + DEFINE_WAIT(wait); \ + \ + for (;;) { \ + prepare_to_wait(&netdev_unregistering_wq, &wait, \ + TASK_UNINTERRUPTIBLE); \ + unregistering = false; \ + rtnl_lock(); \ + list_for_each_entry(net, net_list, member) { \ + if (net->dev_unreg_count > 0) { \ + unregistering = true; \ + break; \ + } \ + } \ + if (!unregistering) \ + break; \ + __rtnl_unlock(); \ + schedule(); \ + } \ + finish_wait(&netdev_unregistering_wq, &wait); \ +} while (0) + #ifdef CONFIG_PROVE_LOCKING extern int lockdep_rtnl_is_held(void); #else diff --git a/net/core/dev.c b/net/core/dev.c index c619b86..25f7ed2 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5541,7 +5541,7 @@ static int dev_new_index(struct net *net) /* Delayed registration/unregisteration */ static LIST_HEAD(net_todo_list); -static DECLARE_WAIT_QUEUE_HEAD(netdev_unregistering_wq); +DECLARE_WAIT_QUEUE_HEAD(netdev_unregistering_wq); static void net_set_todo(struct net_device *dev) { @@ -6926,34 +6926,6 @@ static void __net_exit default_device_exit(struct net *net) rtnl_unlock(); } -static void __net_exit rtnl_lock_unregistering(struct list_head *net_list) -{ - /* Return with the rtnl_lock held when there are no network - * devices unregistering in any network namespace in net_list. - */ - struct net *net; - bool unregistering; - DEFINE_WAIT(wait); - - for (;;) { - prepare_to_wait(&netdev_unregistering_wq, &wait, - TASK_UNINTERRUPTIBLE); - unregistering = false; - rtnl_lock(); - list_for_each_entry(net, net_list, exit_list) { - if (net->dev_unreg_count > 0) { - unregistering = true; - break; - } - } - if (!unregistering) - break; - __rtnl_unlock(); - schedule(); - } - finish_wait(&netdev_unregistering_wq, &wait); -} - static void __net_exit default_device_exit_batch(struct list_head *net_list) { /* At exit all network devices most be removed from a network @@ -6976,7 +6948,7 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list) * will run in the rtnl_unlock() at the end of * default_device_exit_batch. */ - rtnl_lock_unregistering(net_list); + rtnl_lock_unregistering(net_list, exit_list); list_for_each_entry(net, net_list, exit_list) { for_each_netdev_reverse(net, dev) { if (dev->rtnl_link_ops) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 9837beb..fd33a83 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -359,7 +359,7 @@ EXPORT_SYMBOL_GPL(__rtnl_link_unregister); */ void rtnl_link_unregister(struct rtnl_link_ops *ops) { - rtnl_lock(); + rtnl_lock_unregistering(&net_namespace_list, list); __rtnl_link_unregister(ops); rtnl_unlock(); }