On 30.03.2018 12:23, Kirill Tkhai wrote: > This reverts: > > 152f253152cc net: Remove rtnl_lock() in nf_ct_iterate_destroy() > ec9c780925c5 ovs: Remove rtnl_lock() from ovs_exit_net() > 350311aab4c0 security: Remove rtnl_lock() in selinux_xfrm_notify_policyload() > 10256debb918 net: Don't take rtnl_lock() in wireless_nlevent_flush() > f0b07bb151b0 net: Introduce net_rwsem to protect net_namespace_list > > There are missed, that down_read() can't be taken recursive. > This is because of rw_semaphore design, which prevents it > to be occupied by only readers forever. > > So, we can't take it in register_netdevice_notifier(), as it's also > taken in wext_netdev_notifier_call()->wireless_nlevent_flush(). > > The solution is to protect net_namespace_list modifications > in register_netdevice_notifier() via pernet_ops_rwsem, as it's > made in: > > https://patchwork.ozlabs.org/project/netdev/list/?series=36495 > > Then, we won't have to take net_rwsem in call_netevent_notifiers(). > > But since the patchset is not in kernel, let's just revert net_rwsem > for now, and I'll resubmit it later (after the above patchset). > > Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>
Or, if https://patchwork.ozlabs.org/project/netdev/list/?series=36495 is ok enough to go in kernel in a little while, I'll make another fix (removing down_read(&net_rwsem) from {,un}register_netdevice_notifiers()). Please, let me know if some other actions are required. Thanks, Kirill > --- > drivers/infiniband/core/roce_gid_mgmt.c | 2 -- > include/linux/rtnetlink.h | 1 - > include/net/net_namespace.h | 1 - > net/core/dev.c | 5 ----- > net/core/fib_notifier.c | 2 -- > net/core/net_namespace.c | 18 +++++------------- > net/core/rtnetlink.c | 5 ----- > net/netfilter/nf_conntrack_core.c | 4 ++-- > net/openvswitch/datapath.c | 4 ++-- > net/wireless/wext-core.c | 6 ++++-- > security/selinux/include/xfrm.h | 4 ++-- > 11 files changed, 15 insertions(+), 37 deletions(-) > > diff --git a/drivers/infiniband/core/roce_gid_mgmt.c > b/drivers/infiniband/core/roce_gid_mgmt.c > index cc2966380c0c..5a52ec77940a 100644 > --- a/drivers/infiniband/core/roce_gid_mgmt.c > +++ b/drivers/infiniband/core/roce_gid_mgmt.c > @@ -403,12 +403,10 @@ static void enum_all_gids_of_dev_cb(struct ib_device > *ib_dev, > * our feet > */ > rtnl_lock(); > - down_read(&net_rwsem); > for_each_net(net) > for_each_netdev(net, ndev) > if (is_eth_port_of_netdev(ib_dev, port, rdma_ndev, > ndev)) > add_netdev_ips(ib_dev, port, rdma_ndev, ndev); > - up_read(&net_rwsem); > rtnl_unlock(); > } > > diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h > index 5225832bd6ff..c7d1e4689325 100644 > --- a/include/linux/rtnetlink.h > +++ b/include/linux/rtnetlink.h > @@ -37,7 +37,6 @@ extern int rtnl_lock_killable(void); > > extern wait_queue_head_t netdev_unregistering_wq; > extern struct rw_semaphore pernet_ops_rwsem; > -extern struct rw_semaphore net_rwsem; > > #ifdef CONFIG_PROVE_LOCKING > extern bool lockdep_rtnl_is_held(void); > diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h > index 47e35cce3b64..1ab4f920f109 100644 > --- a/include/net/net_namespace.h > +++ b/include/net/net_namespace.h > @@ -291,7 +291,6 @@ static inline struct net *read_pnet(const possible_net_t > *pnet) > #endif > } > > -/* Protected by net_rwsem */ > #define for_each_net(VAR) \ > list_for_each_entry(VAR, &net_namespace_list, list) > > diff --git a/net/core/dev.c b/net/core/dev.c > index eca5458b2753..e13807b5c84d 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1629,7 +1629,6 @@ int register_netdevice_notifier(struct notifier_block > *nb) > goto unlock; > if (dev_boot_phase) > goto unlock; > - down_read(&net_rwsem); > for_each_net(net) { > for_each_netdev(net, dev) { > err = call_netdevice_notifier(nb, NETDEV_REGISTER, dev); > @@ -1643,7 +1642,6 @@ int register_netdevice_notifier(struct notifier_block > *nb) > call_netdevice_notifier(nb, NETDEV_UP, dev); > } > } > - up_read(&net_rwsem); > > unlock: > rtnl_unlock(); > @@ -1666,7 +1664,6 @@ int register_netdevice_notifier(struct notifier_block > *nb) > } > > outroll: > - up_read(&net_rwsem); > raw_notifier_chain_unregister(&netdev_chain, nb); > goto unlock; > } > @@ -1697,7 +1694,6 @@ int unregister_netdevice_notifier(struct notifier_block > *nb) > if (err) > goto unlock; > > - down_read(&net_rwsem); > for_each_net(net) { > for_each_netdev(net, dev) { > if (dev->flags & IFF_UP) { > @@ -1708,7 +1704,6 @@ int unregister_netdevice_notifier(struct notifier_block > *nb) > call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); > } > } > - up_read(&net_rwsem); > unlock: > rtnl_unlock(); > return err; > diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c > index 13a40b831d6d..b793b523aba3 100644 > --- a/net/core/fib_notifier.c > +++ b/net/core/fib_notifier.c > @@ -39,7 +39,6 @@ static unsigned int fib_seq_sum(void) > struct net *net; > > rtnl_lock(); > - down_read(&net_rwsem); > for_each_net(net) { > rcu_read_lock(); > list_for_each_entry_rcu(ops, &net->fib_notifier_ops, list) { > @@ -50,7 +49,6 @@ static unsigned int fib_seq_sum(void) > } > rcu_read_unlock(); > } > - up_read(&net_rwsem); > rtnl_unlock(); > > return fib_seq; > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c > index 7fdf321d4997..b5796d17a302 100644 > --- a/net/core/net_namespace.c > +++ b/net/core/net_namespace.c > @@ -33,10 +33,6 @@ static struct list_head *first_device = &pernet_list; > LIST_HEAD(net_namespace_list); > EXPORT_SYMBOL_GPL(net_namespace_list); > > -/* Protects net_namespace_list. Nests iside rtnl_lock() */ > -DECLARE_RWSEM(net_rwsem); > -EXPORT_SYMBOL_GPL(net_rwsem); > - > struct net init_net = { > .count = REFCOUNT_INIT(1), > .dev_base_head = LIST_HEAD_INIT(init_net.dev_base_head), > @@ -313,9 +309,9 @@ static __net_init int setup_net(struct net *net, struct > user_namespace *user_ns) > if (error < 0) > goto out_undo; > } > - down_write(&net_rwsem); > + rtnl_lock(); > list_add_tail_rcu(&net->list, &net_namespace_list); > - up_write(&net_rwsem); > + rtnl_unlock(); > out: > return error; > > @@ -454,7 +450,7 @@ static void unhash_nsid(struct net *net, struct net *last) > * and this work is the only process, that may delete > * a net from net_namespace_list. So, when the below > * is executing, the list may only grow. Thus, we do not > - * use for_each_net_rcu() or net_rwsem. > + * use for_each_net_rcu() or rtnl_lock(). > */ > for_each_net(tmp) { > int id; > @@ -489,7 +485,7 @@ static void cleanup_net(struct work_struct *work) > down_read(&pernet_ops_rwsem); > > /* Don't let anyone else find us. */ > - down_write(&net_rwsem); > + rtnl_lock(); > llist_for_each_entry(net, net_kill_list, cleanup_list) > list_del_rcu(&net->list); > /* Cache last net. After we unlock rtnl, no one new net > @@ -503,7 +499,7 @@ static void cleanup_net(struct work_struct *work) > * useless anyway, as netns_ids are destroyed there. > */ > last = list_last_entry(&net_namespace_list, struct net, list); > - up_write(&net_rwsem); > + rtnl_unlock(); > > llist_for_each_entry(net, net_kill_list, cleanup_list) { > unhash_nsid(net, last); > @@ -904,9 +900,6 @@ static int __register_pernet_operations(struct list_head > *list, > > list_add_tail(&ops->list, list); > if (ops->init || (ops->id && ops->size)) { > - /* We held write locked pernet_ops_rwsem, and parallel > - * setup_net() and cleanup_net() are not possible. > - */ > for_each_net(net) { > error = ops_init(ops, net); > if (error) > @@ -930,7 +923,6 @@ static void __unregister_pernet_operations(struct > pernet_operations *ops) > LIST_HEAD(net_exit_list); > > list_del(&ops->list); > - /* See comment in __register_pernet_operations() */ > for_each_net(net) > list_add_tail(&net->exit_list, &net_exit_list); > ops_exit_list(ops, &net_exit_list); > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index e86b28482ca7..2d3949789cef 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -418,11 +418,9 @@ void __rtnl_link_unregister(struct rtnl_link_ops *ops) > { > struct net *net; > > - down_read(&net_rwsem); > for_each_net(net) { > __rtnl_kill_links(net, ops); > } > - up_read(&net_rwsem); > list_del(&ops->list); > } > EXPORT_SYMBOL_GPL(__rtnl_link_unregister); > @@ -440,9 +438,6 @@ static void rtnl_lock_unregistering_all(void) > for (;;) { > unregistering = false; > rtnl_lock(); > - /* We held write locked pernet_ops_rwsem, and parallel > - * setup_net() and cleanup_net() are not possible. > - */ > for_each_net(net) { > if (net->dev_unreg_count > 0) { > unregistering = true; > diff --git a/net/netfilter/nf_conntrack_core.c > b/net/netfilter/nf_conntrack_core.c > index 41ff04ee2554..705198de671d 100644 > --- a/net/netfilter/nf_conntrack_core.c > +++ b/net/netfilter/nf_conntrack_core.c > @@ -1763,14 +1763,14 @@ nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, > void *data), void *data) > { > struct net *net; > > - down_read(&net_rwsem); > + rtnl_lock(); > for_each_net(net) { > if (atomic_read(&net->ct.count) == 0) > continue; > __nf_ct_unconfirmed_destroy(net); > nf_queue_nf_hook_drop(net); > } > - up_read(&net_rwsem); > + rtnl_unlock(); > > /* Need to wait for netns cleanup worker to finish, if its > * running -- it might have deleted a net namespace from > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c > index 015e24e08909..ef38e5aecd28 100644 > --- a/net/openvswitch/datapath.c > +++ b/net/openvswitch/datapath.c > @@ -2363,10 +2363,10 @@ static void __net_exit ovs_exit_net(struct net *dnet) > list_for_each_entry_safe(dp, dp_next, &ovs_net->dps, list_node) > __dp_destroy(dp); > > - down_read(&net_rwsem); > + rtnl_lock(); > for_each_net(net) > list_vports_from_net(net, dnet, &head); > - up_read(&net_rwsem); > + rtnl_unlock(); > > /* Detach all vports from given namespace. */ > list_for_each_entry_safe(vport, vport_next, &head, detach_list) { > diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c > index 5e677dac2a0c..9efbfc753347 100644 > --- a/net/wireless/wext-core.c > +++ b/net/wireless/wext-core.c > @@ -347,13 +347,13 @@ void wireless_nlevent_flush(void) > struct sk_buff *skb; > struct net *net; > > - down_read(&net_rwsem); > + ASSERT_RTNL(); > + > for_each_net(net) { > while ((skb = skb_dequeue(&net->wext_nlevents))) > rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, > GFP_KERNEL); > } > - up_read(&net_rwsem); > } > EXPORT_SYMBOL_GPL(wireless_nlevent_flush); > > @@ -410,7 +410,9 @@ subsys_initcall(wireless_nlevent_init); > /* Process events generated by the wireless layer or the driver. */ > static void wireless_nlevent_process(struct work_struct *work) > { > + rtnl_lock(); > wireless_nlevent_flush(); > + rtnl_unlock(); > } > > static DECLARE_WORK(wireless_nlevent_work, wireless_nlevent_process); > diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h > index a0b465316292..1f173a7a4daa 100644 > --- a/security/selinux/include/xfrm.h > +++ b/security/selinux/include/xfrm.h > @@ -47,10 +47,10 @@ static inline void selinux_xfrm_notify_policyload(void) > { > struct net *net; > > - down_read(&net_rwsem); > + rtnl_lock(); > for_each_net(net) > rt_genid_bump_all(net); > - up_read(&net_rwsem); > + rtnl_unlock(); > } > #else > static inline int selinux_xfrm_enabled(void) >
