diff mbox series

[net-next] net: add CONFIG_PCPU_DEV_REFCNT

Message ID 20210319150232.354084-1-eric.dumazet@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: add CONFIG_PCPU_DEV_REFCNT | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 7 maintainers not CCed: daniel@iogearbox.net cong.wang@bytedance.com andriin@fb.com ast@kernel.org ap420073@gmail.com atenart@kernel.org weiwan@google.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 6874 this patch: 6874
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: please write a paragraph that describes the config symbol fully
netdev/build_allmodconfig_warn success Errors and warnings before: 7088 this patch: 7088
netdev/header_inline success Link

Commit Message

Eric Dumazet March 19, 2021, 3:02 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

I was working on a syzbot issue, claiming one device could not be
dismantled because its refcount was -1

unregister_netdevice: waiting for sit0 to become free. Usage count = -1

It would be nice if syzbot could trigger a warning at the time
this reference count became negative.

This patch adds CONFIG_PCPU_DEV_REFCNT options which defaults
to per cpu variables (as before this patch) on SMP builds.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h | 13 +++++++++++++
 net/Kconfig               |  8 ++++++++
 net/core/dev.c            | 10 ++++++++++
 3 files changed, 31 insertions(+)

Comments

kernel test robot March 19, 2021, 5:11 p.m. UTC | #1 
Hi Eric,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-CONFIG_PCPU_DEV_REFCNT/20210319-230417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 38cb57602369cf194556460a52bd18e53c76e13d
config: arm-randconfig-r014-20210318 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/684c34243e0c84e496aa426734df321b7ebc088b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-CONFIG_PCPU_DEV_REFCNT/20210319-230417
        git checkout 684c34243e0c84e496aa426734df321b7ebc088b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/core/dev.c:10752:1: warning: unused label 'free_dev' [-Wunused-label]
   free_dev:
   ^~~~~~~~~
   net/core/dev.c:4993:1: warning: unused function 'sch_handle_ingress' [-Wunused-function]
   sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
   ^
   net/core/dev.c:5143:19: warning: unused function 'nf_ingress' [-Wunused-function]
   static inline int nf_ingress(struct sk_buff *skb, struct packet_type **pt_prev,
                     ^
   3 warnings generated.


vim +/free_dev +10752 net/core/dev.c

8d3bdbd55a7e2a David S. Miller        2011-02-08  10703  
8d3bdbd55a7e2a David S. Miller        2011-02-08  10704  	INIT_LIST_HEAD(&dev->napi_list);
8d3bdbd55a7e2a David S. Miller        2011-02-08  10705  	INIT_LIST_HEAD(&dev->unreg_list);
5cde282938915f Eric W. Biederman      2013-10-05  10706  	INIT_LIST_HEAD(&dev->close_list);
8d3bdbd55a7e2a David S. Miller        2011-02-08  10707  	INIT_LIST_HEAD(&dev->link_watch_list);
2f268f129c2d1a Veaceslav Falico       2013-09-25  10708  	INIT_LIST_HEAD(&dev->adj_list.upper);
2f268f129c2d1a Veaceslav Falico       2013-09-25  10709  	INIT_LIST_HEAD(&dev->adj_list.lower);
7866a621043fba Salam Noureddine       2015-01-27  10710  	INIT_LIST_HEAD(&dev->ptype_all);
7866a621043fba Salam Noureddine       2015-01-27  10711  	INIT_LIST_HEAD(&dev->ptype_specific);
93642e14bd50e5 Jiri Pirko             2020-01-25  10712  	INIT_LIST_HEAD(&dev->net_notifier_list);
59cc1f61f09c26 Jiri Kosina            2016-08-10  10713  #ifdef CONFIG_NET_SCHED
59cc1f61f09c26 Jiri Kosina            2016-08-10  10714  	hash_init(dev->qdisc_hash);
59cc1f61f09c26 Jiri Kosina            2016-08-10  10715  #endif
0287587884b150 Eric Dumazet           2014-10-05  10716  	dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
8d3bdbd55a7e2a David S. Miller        2011-02-08  10717  	setup(dev);
8d3bdbd55a7e2a David S. Miller        2011-02-08  10718  
a813104d923339 Phil Sutter            2016-02-17  10719  	if (!dev->tx_queue_len) {
f84bb1eac02752 Phil Sutter            2015-08-27  10720  		dev->priv_flags |= IFF_NO_QUEUE;
1159708432f706 Jesper Dangaard Brouer 2016-11-03  10721  		dev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
a813104d923339 Phil Sutter            2016-02-17  10722  	}
906470c19da771 Phil Sutter            2015-08-18  10723  
36909ea43814cb Tom Herbert            2011-01-09  10724  	dev->num_tx_queues = txqs;
36909ea43814cb Tom Herbert            2011-01-09  10725  	dev->real_num_tx_queues = txqs;
ed9af2e839c06c Tom Herbert            2010-11-09  10726  	if (netif_alloc_netdev_queues(dev))
8d3bdbd55a7e2a David S. Miller        2011-02-08  10727  		goto free_all;
e8a0464cc95097 David S. Miller        2008-07-17  10728  
36909ea43814cb Tom Herbert            2011-01-09  10729  	dev->num_rx_queues = rxqs;
36909ea43814cb Tom Herbert            2011-01-09  10730  	dev->real_num_rx_queues = rxqs;
fe8222406c8277 Tom Herbert            2010-11-09  10731  	if (netif_alloc_rx_queues(dev))
8d3bdbd55a7e2a David S. Miller        2011-02-08  10732  		goto free_all;
0a9627f2649a02 Tom Herbert            2010-03-16  10733  
^1da177e4c3f41 Linus Torvalds         2005-04-16  10734  	strcpy(dev->name, name);
c835a677331495 Tom Gundersen          2014-07-14  10735  	dev->name_assign_type = name_assign_type;
cbda10fa97d72c Vlad Dogaru            2011-01-13  10736  	dev->group = INIT_NETDEV_GROUP;
2c60db037034d2 Eric Dumazet           2012-09-16  10737  	if (!dev->ethtool_ops)
2c60db037034d2 Eric Dumazet           2012-09-16  10738  		dev->ethtool_ops = &default_ethtool_ops;
e687ad60af0901 Pablo Neira            2015-05-13  10739  
357b6cc5834eab Daniel Borkmann        2020-03-18  10740  	nf_hook_ingress_init(dev);
e687ad60af0901 Pablo Neira            2015-05-13  10741  
^1da177e4c3f41 Linus Torvalds         2005-04-16  10742  	return dev;
ab9c73ccb52f40 Jiri Pirko             2009-05-08  10743  
8d3bdbd55a7e2a David S. Miller        2011-02-08  10744  free_all:
8d3bdbd55a7e2a David S. Miller        2011-02-08  10745  	free_netdev(dev);
8d3bdbd55a7e2a David S. Miller        2011-02-08  10746  	return NULL;
8d3bdbd55a7e2a David S. Miller        2011-02-08  10747  
29b4433d991c88 Eric Dumazet           2010-10-11  10748  free_pcpu:
684c34243e0c84 Eric Dumazet           2021-03-19  10749  #ifdef CONFIG_PCPU_DEV_REFCNT
29b4433d991c88 Eric Dumazet           2010-10-11  10750  	free_percpu(dev->pcpu_refcnt);
684c34243e0c84 Eric Dumazet           2021-03-19  10751  #endif
74d332c13b2148 Eric Dumazet           2013-10-30 @10752  free_dev:
74d332c13b2148 Eric Dumazet           2013-10-30  10753  	netdev_freemem(dev);
ab9c73ccb52f40 Jiri Pirko             2009-05-08  10754  	return NULL;
^1da177e4c3f41 Linus Torvalds         2005-04-16  10755  }
36909ea43814cb Tom Herbert            2011-01-09  10756  EXPORT_SYMBOL(alloc_netdev_mqs);
^1da177e4c3f41 Linus Torvalds         2005-04-16  10757  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Eric Dumazet March 19, 2021, 5:28 p.m. UTC | #2 
On Fri, Mar 19, 2021 at 6:12 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Eric,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
>
> url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-CONFIG_PCPU_DEV_REFCNT/20210319-230417
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 38cb57602369cf194556460a52bd18e53c76e13d
> config: arm-randconfig-r014-20210318 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://github.com/0day-ci/linux/commit/684c34243e0c84e496aa426734df321b7ebc088b
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Dumazet/net-add-CONFIG_PCPU_DEV_REFCNT/20210319-230417
>         git checkout 684c34243e0c84e496aa426734df321b7ebc088b
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> net/core/dev.c:10752:1: warning: unused label 'free_dev' [-Wunused-label]
>    free_dev:
>    ^~~~~~~~~

Great, I will add the following diff to v2

diff --git a/net/core/dev.c b/net/core/dev.c
index edde830df1a483535372014034d5b1ee1ff6210a..be941ed754ac71d0839604bcfdd8ab67c339d27f
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10748,8 +10748,8 @@ struct net_device *alloc_netdev_mqs(int
sizeof_priv, const char *name,
 free_pcpu:
 #ifdef CONFIG_PCPU_DEV_REFCNT
        free_percpu(dev->pcpu_refcnt);
-#endif
 free_dev:
+#endif
        netdev_freemem(dev);
        return NULL;
 }
diff mbox series

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4940509999beeb93ca4ad214e65d68e622a484cb..8f003955c485b81210ed56f7e1c24080b4bb46eb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2092,7 +2092,12 @@  struct net_device {
 	u32                     proto_down_reason;
 
 	struct list_head	todo_list;
+
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	int __percpu		*pcpu_refcnt;
+#else
+	refcount_t		dev_refcnt;
+#endif
 
 	struct list_head	link_watch_list;
 
@@ -4044,7 +4049,11 @@  void netdev_run_todo(void);
  */
 static inline void dev_put(struct net_device *dev)
 {
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	this_cpu_dec(*dev->pcpu_refcnt);
+#else
+	refcount_dec(&dev->dev_refcnt);
+#endif
 }
 
 /**
@@ -4055,7 +4064,11 @@  static inline void dev_put(struct net_device *dev)
  */
 static inline void dev_hold(struct net_device *dev)
 {
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	this_cpu_inc(*dev->pcpu_refcnt);
+#else
+	refcount_inc(&dev->dev_refcnt);
+#endif
 }
 
 /* Carrier loss detection, dial on demand. The functions netif_carrier_on
diff --git a/net/Kconfig b/net/Kconfig
index 0ead7ec0d2bd9ccbbfeab32f1892a9afd2a3eb77..9c456acc379e78caa9e45d4f1335802a05663a0f 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -245,6 +245,14 @@  source "net/l3mdev/Kconfig"
 source "net/qrtr/Kconfig"
 source "net/ncsi/Kconfig"
 
+config PCPU_DEV_REFCNT
+	bool "Use percpu variables to maintain network device refcount"
+	depends on SMP
+	default y
+	help
+	  network device refcount are using per cpu variables if this option is set.
+	  This can be forced to N to detect underflows (with a performance drop).
+
 config RPS
 	bool
 	depends on SMP && SYSFS
diff --git a/net/core/dev.c b/net/core/dev.c
index 4961fc2e9b1967c0d63b53cd809d52b43be0ed4b..edde830df1a483535372014034d5b1ee1ff6210a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10312,11 +10312,15 @@  EXPORT_SYMBOL(register_netdev);
 
 int netdev_refcnt_read(const struct net_device *dev)
 {
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	int i, refcnt = 0;
 
 	for_each_possible_cpu(i)
 		refcnt += *per_cpu_ptr(dev->pcpu_refcnt, i);
 	return refcnt;
+#else
+	return refcount_read(&dev->dev_refcnt);
+#endif
 }
 EXPORT_SYMBOL(netdev_refcnt_read);
 
@@ -10674,9 +10678,11 @@  struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	dev = PTR_ALIGN(p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
 
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
 		goto free_dev;
+#endif
 
 	if (dev_addr_init(dev))
 		goto free_pcpu;
@@ -10740,7 +10746,9 @@  struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	return NULL;
 
 free_pcpu:
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	free_percpu(dev->pcpu_refcnt);
+#endif
 free_dev:
 	netdev_freemem(dev);
 	return NULL;
@@ -10783,8 +10791,10 @@  void free_netdev(struct net_device *dev)
 	list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
 		netif_napi_del(p);
 
+#ifdef CONFIG_PCPU_DEV_REFCNT
 	free_percpu(dev->pcpu_refcnt);
 	dev->pcpu_refcnt = NULL;
+#endif
 	free_percpu(dev->xdp_bulkq);
 	dev->xdp_bulkq = NULL;