[PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Two first patches add a generic infrastructure, that will be used
to get tracking of refcount increments/decrements.

The general idea is to be able to precisely pair each decrement with
a corresponding prior increment. Both share a cookie, basically
a pointer to private data storing stack traces.

Then a series of 17 patches converts some dev_hold()/dev_put()
pairs to new hepers : dev_hold_track() and dev_put_track().

Hopefully this will be used by developpers and syzbot to
root cause bugs that cause netdevice dismantles freezes.

With CONFIG_PCPU_DEV_REFCNT=n option, we were able to detect
some class of bugs, but too late (when too many dev_put()
were happening).

Eric Dumazet (19):
  lib: add reference counting tracking infrastructure
  lib: add tests for reference tracker
  net: add dev_hold_track() and dev_put_track() helpers
  net: add net device refcount tracker to struct netdev_rx_queue
  net: add net device refcount tracker to struct netdev_queue
  net: add net device refcount tracker to ethtool_phys_id()
  net: add net device refcount tracker to dev_ifsioc()
  drop_monitor: add net device refcount tracker
  net: dst: add net device refcount tracking to dst_entry
  ipv6: add net device refcount tracker to rt6_probe_deferred()
  sit: add net device refcount tracking to ip_tunnel
  ipv6: add net device refcount tracker to struct ip6_tnl
  net: add net device refcount tracker to struct neighbour
  net: add net device refcount tracker to struct pneigh_entry
  net: add net device refcount tracker to struct neigh_parms
  net: add net device refcount tracker to struct netdev_adjacent
  ipv6: add net device refcount tracker to struct inet6_dev
  ipv4: add net device refcount tracker to struct in_device
  net/sched: add net device refcount tracker to struct Qdisc

 include/linux/inetdevice.h  |   2 +
 include/linux/netdevice.h   |  53 ++++++++++++++
 include/linux/ref_tracker.h |  73 +++++++++++++++++++
 include/net/devlink.h       |   3 +
 include/net/dst.h           |   1 +
 include/net/if_inet6.h      |   1 +
 include/net/ip6_tunnel.h    |   1 +
 include/net/ip_tunnels.h    |   3 +
 include/net/neighbour.h     |   3 +
 include/net/sch_generic.h   |   2 +-
 lib/Kconfig                 |   4 ++
 lib/Kconfig.debug           |  10 +++
 lib/Makefile                |   4 +-
 lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
 lib/test_ref_tracker.c      | 116 ++++++++++++++++++++++++++++++
 net/Kconfig                 |   8 +++
 net/core/dev.c              |  10 ++-
 net/core/dev_ioctl.c        |   5 +-
 net/core/drop_monitor.c     |   4 +-
 net/core/dst.c              |   8 +--
 net/core/neighbour.c        |  18 ++---
 net/core/net-sysfs.c        |   8 +--
 net/ethtool/ioctl.c         |   5 +-
 net/ipv4/devinet.c          |   4 +-
 net/ipv4/route.c            |   7 +-
 net/ipv6/addrconf.c         |   4 +-
 net/ipv6/addrconf_core.c    |   2 +-
 net/ipv6/ip6_gre.c          |   8 +--
 net/ipv6/ip6_tunnel.c       |   4 +-
 net/ipv6/ip6_vti.c          |   4 +-
 net/ipv6/route.c            |  10 +--
 net/ipv6/sit.c              |   4 +-
 net/sched/sch_generic.c     |   4 +-
 33 files changed, 481 insertions(+), 52 deletions(-)
 create mode 100644 include/linux/ref_tracker.h
 create mode 100644 lib/ref_tracker.c
 create mode 100644 lib/test_ref_tracker.c

-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 01/19] lib: add reference counting tracking infrastructure

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

It can be hard to track where references are taken and released.

In networking, we have annoying issues at device or netns dismantles,
and we had various proposals to ease root causing them.

This patch adds new infrastructure pairing refcount increases
and decreases. This will self document code, because programmers
will have to associate increments/decrements.

This is controled by CONFIG_REF_TRACKER which can be selected
by users of this feature.

This adds both cpu and memory costs, and thus should probably be
used with care.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/ref_tracker.h |  73 +++++++++++++++++++
 lib/Kconfig                 |   4 ++
 lib/Makefile                |   2 +
 lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
 4 files changed, 219 insertions(+)
 create mode 100644 include/linux/ref_tracker.h
 create mode 100644 lib/ref_tracker.c

diff --git a/include/linux/ref_tracker.h b/include/linux/ref_tracker.h
new file mode 100644
index 0000000000000000000000000000000000000000..c11c9db5825cf933acf529c83db441a818135f29
--- /dev/null
+++ b/include/linux/ref_tracker.h
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#ifndef _LINUX_REF_TRACKER_H
+#define _LINUX_REF_TRACKER_H
+#include <linux/refcount.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+
+struct ref_tracker;
+
+struct ref_tracker_dir {
+#ifdef CONFIG_REF_TRACKER
+	spinlock_t		lock;
+	unsigned int		quarantine_avail;
+	refcount_t		untracked;
+	struct list_head	list; /* List of active trackers */
+	struct list_head	quarantine; /* List of dead trackers */
+#endif
+};
+
+#ifdef CONFIG_REF_TRACKER
+static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
+					unsigned int quarantine_count)
+{
+	INIT_LIST_HEAD(&dir->list);
+	INIT_LIST_HEAD(&dir->quarantine);
+	spin_lock_init(&dir->lock);
+	dir->quarantine_avail = quarantine_count;
+	refcount_set(&dir->untracked, 1);
+}
+
+void ref_tracker_dir_exit(struct ref_tracker_dir *dir);
+
+void ref_tracker_dir_print(struct ref_tracker_dir *dir,
+			   unsigned int display_limit);
+
+int ref_tracker_alloc(struct ref_tracker_dir *dir,
+		      struct ref_tracker **trackerp, gfp_t gfp);
+
+int ref_tracker_free(struct ref_tracker_dir *dir,
+		     struct ref_tracker **trackerp);
+
+#else /* CONFIG_REF_TRACKER */
+
+static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
+					unsigned int quarantine_count)
+{
+}
+
+static inline void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
+{
+}
+
+static inline void ref_tracker_dir_print(struct ref_tracker_dir *dir,
+					 unsigned int display_limit)
+{
+}
+
+static inline int ref_tracker_alloc(struct ref_tracker_dir *dir,
+				    struct ref_tracker **trackerp,
+				    gfp_t gfp)
+{
+	return 0;
+}
+
+static inline int ref_tracker_free(struct ref_tracker_dir *dir,
+				   struct ref_tracker **trackerp)
+{
+	return 0;
+}
+
+#endif
+
+#endif /* _LINUX_REF_TRACKER_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 5e7165e6a346c9bec878b78c8c8c3d175fc98dfd..d01be8e9593992a7d94a46bd1716460bc33c3ae1 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -680,6 +680,10 @@ config STACK_HASH_ORDER
 	 Select the hash size as a power of 2 for the stackdepot hash table.
 	 Choose a lower value to reduce the memory impact.
 
+config REF_TRACKER
+	bool
+	select STACKDEPOT
+
 config SBITMAP
 	bool
 
diff --git a/lib/Makefile b/lib/Makefile
index 364c23f1557816f73aebd8304c01224a4846ac6c..c1fd9243ddb9cc1ac5252d7eb8009f9290782c4a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -270,6 +270,8 @@ obj-$(CONFIG_STACKDEPOT) += stackdepot.o
 KASAN_SANITIZE_stackdepot.o := n
 KCOV_INSTRUMENT_stackdepot.o := n
 
+obj-$(CONFIG_REF_TRACKER) += ref_tracker.o
+
 libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
 	       fdt_empty_tree.o fdt_addresses.o
 $(foreach file, $(libfdt_files), \
diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c
new file mode 100644
index 0000000000000000000000000000000000000000..0ae2e66dcf0fdb976f4cb99e747c9448b37f22cc
--- /dev/null
+++ b/lib/ref_tracker.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/export.h>
+#include <linux/ref_tracker.h>
+#include <linux/slab.h>
+#include <linux/stacktrace.h>
+#include <linux/stackdepot.h>
+
+#define REF_TRACKER_STACK_ENTRIES 16
+
+struct ref_tracker {
+	struct list_head	head;   /* anchor into dir->list or dir->quarantine */
+	bool			dead;
+	depot_stack_handle_t	alloc_stack_handle;
+	depot_stack_handle_t	free_stack_handle;
+};
+
+void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
+{
+	struct ref_tracker *tracker, *n;
+	unsigned long flags;
+	bool leak = false;
+
+	spin_lock_irqsave(&dir->lock, flags);
+	list_for_each_entry_safe(tracker, n, &dir->quarantine, head) {
+		list_del(&tracker->head);
+		kfree(tracker);
+		dir->quarantine_avail++;
+	}
+	list_for_each_entry_safe(tracker, n, &dir->list, head) {
+		pr_err("leaked reference.\n");
+		if (tracker->alloc_stack_handle)
+			stack_depot_print(tracker->alloc_stack_handle);
+		leak = true;
+		list_del(&tracker->head);
+		kfree(tracker);
+	}
+	spin_unlock_irqrestore(&dir->lock, flags);
+	WARN_ON_ONCE(leak);
+	WARN_ON_ONCE(refcount_read(&dir->untracked) != 1);
+}
+EXPORT_SYMBOL(ref_tracker_dir_exit);
+
+void ref_tracker_dir_print(struct ref_tracker_dir *dir,
+			   unsigned int display_limit)
+{
+	struct ref_tracker *tracker;
+	unsigned long flags;
+	unsigned int i = 0;
+
+	spin_lock_irqsave(&dir->lock, flags);
+	list_for_each_entry(tracker, &dir->list, head) {
+		if (i < display_limit) {
+			pr_err("leaked reference.\n");
+			if (tracker->alloc_stack_handle)
+				stack_depot_print(tracker->alloc_stack_handle);
+			i++;
+		} else {
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&dir->lock, flags);
+}
+EXPORT_SYMBOL(ref_tracker_dir_print);
+
+int ref_tracker_alloc(struct ref_tracker_dir *dir,
+		      struct ref_tracker **trackerp,
+		      gfp_t gfp)
+{
+	unsigned long entries[REF_TRACKER_STACK_ENTRIES];
+	struct ref_tracker *tracker;
+	unsigned int nr_entries;
+	unsigned long flags;
+
+	*trackerp = tracker = kzalloc(sizeof(*tracker), gfp | __GFP_NOFAIL);
+	if (unlikely(!tracker)) {
+		pr_err_once("memory allocation failure, unreliable refcount tracker.\n");
+		refcount_inc(&dir->untracked);
+		return -ENOMEM;
+	}
+	nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
+	nr_entries = filter_irq_stacks(entries, nr_entries);
+	tracker->alloc_stack_handle = stack_depot_save(entries, nr_entries, gfp);
+
+	spin_lock_irqsave(&dir->lock, flags);
+	list_add(&tracker->head, &dir->list);
+	spin_unlock_irqrestore(&dir->lock, flags);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ref_tracker_alloc);
+
+int ref_tracker_free(struct ref_tracker_dir *dir,
+		     struct ref_tracker **trackerp)
+{
+	unsigned long entries[REF_TRACKER_STACK_ENTRIES];
+	struct ref_tracker *tracker = *trackerp;
+	depot_stack_handle_t stack_handle;
+	unsigned int nr_entries;
+	unsigned long flags;
+
+	if (!tracker) {
+		refcount_dec(&dir->untracked);
+		return -EEXIST;
+	}
+	nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
+	nr_entries = filter_irq_stacks(entries, nr_entries);
+	stack_handle = stack_depot_save(entries, nr_entries, GFP_ATOMIC);
+
+	spin_lock_irqsave(&dir->lock, flags);
+	if (tracker->dead) {
+		pr_err("reference already released.\n");
+		if (tracker->alloc_stack_handle) {
+			pr_err("allocated in:\n");
+			stack_depot_print(tracker->alloc_stack_handle);
+		}
+		if (tracker->free_stack_handle) {
+			pr_err("freed in:\n");
+			stack_depot_print(tracker->free_stack_handle);
+		}
+		spin_unlock_irqrestore(&dir->lock, flags);
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
+	tracker->dead = true;
+
+	tracker->free_stack_handle = stack_handle;
+
+	list_move_tail(&tracker->head, &dir->quarantine);
+	if (!dir->quarantine_avail) {
+		tracker = list_first_entry(&dir->quarantine, struct ref_tracker, head);
+		list_del(&tracker->head);
+	} else {
+		dir->quarantine_avail--;
+		tracker = NULL;
+	}
+	spin_unlock_irqrestore(&dir->lock, flags);
+
+	kfree(tracker);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ref_tracker_free);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 02/19] lib: add tests for reference tracker

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This module uses reference tracker, forcing two issues.

1) Double free of a tracker

2) leak of two trackers, one being allocated from softirq context.

"modprobe test_ref_tracker" would emit the following traces.
(Use scripts/decode_stacktrace.sh if necessary)

[  171.648681] reference already released.
[  171.653213] allocated in:
[  171.656523]  alloctest_ref_tracker_alloc2+0x1c/0x20 [test_ref_tracker]
[  171.656526]  init_module+0x86/0x1000 [test_ref_tracker]
[  171.656528]  do_one_initcall+0x9c/0x220
[  171.656532]  do_init_module+0x60/0x240
[  171.656536]  load_module+0x32b5/0x3610
[  171.656538]  __do_sys_init_module+0x148/0x1a0
[  171.656540]  __x64_sys_init_module+0x1d/0x20
[  171.656542]  do_syscall_64+0x4a/0xb0
[  171.656546]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.656549] freed in:
[  171.659520]  alloctest_ref_tracker_free+0x13/0x20 [test_ref_tracker]
[  171.659522]  init_module+0xec/0x1000 [test_ref_tracker]
[  171.659523]  do_one_initcall+0x9c/0x220
[  171.659525]  do_init_module+0x60/0x240
[  171.659527]  load_module+0x32b5/0x3610
[  171.659529]  __do_sys_init_module+0x148/0x1a0
[  171.659532]  __x64_sys_init_module+0x1d/0x20
[  171.659534]  do_syscall_64+0x4a/0xb0
[  171.659536]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.659575] ------------[ cut here ]------------
[  171.659576] WARNING: CPU: 5 PID: 13016 at lib/ref_tracker.c:112 ref_tracker_free+0x224/0x270
[  171.659581] Modules linked in: test_ref_tracker(+)
[  171.659591] CPU: 5 PID: 13016 Comm: modprobe Tainted: G S                5.16.0-smp-DEV #290
[  171.659595] RIP: 0010:ref_tracker_free+0x224/0x270
[  171.659599] Code: 5e 41 5f 5d c3 48 c7 c7 04 9c 74 a6 31 c0 e8 62 ee 67 00 83 7b 14 00 75 1a 83 7b 18 00 75 30 4c 89 ff 4c 89 f6 e8 9c 00 69 00 <0f> 0b bb ea ff ff ff eb ae 48 c7 c7 3a 0a 77 a6 31 c0 e8 34 ee 67
[  171.659601] RSP: 0018:ffff89058ba0bbd0 EFLAGS: 00010286
[  171.659603] RAX: 0000000000000029 RBX: ffff890586b19780 RCX: 08895bff57c7d100
[  171.659604] RDX: c0000000ffff7fff RSI: 0000000000000282 RDI: ffffffffc0407000
[  171.659606] RBP: ffff89058ba0bc88 R08: 0000000000000000 R09: ffffffffa6f342e0
[  171.659607] R10: 00000000ffff7fff R11: 0000000000000000 R12: 000000008f000000
[  171.659608] R13: 0000000000000014 R14: 0000000000000282 R15: ffffffffc0407000
[  171.659609] FS:  00007f97ea29d740(0000) GS:ffff8923ff940000(0000) knlGS:0000000000000000
[  171.659611] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  171.659613] CR2: 00007f97ea299000 CR3: 0000000186b4a004 CR4: 00000000001706e0
[  171.659614] Call Trace:
[  171.659615]  <TASK>
[  171.659631]  ? alloctest_ref_tracker_free+0x13/0x20 [test_ref_tracker]
[  171.659633]  ? init_module+0x105/0x1000 [test_ref_tracker]
[  171.659636]  ? do_one_initcall+0x9c/0x220
[  171.659638]  ? do_init_module+0x60/0x240
[  171.659641]  ? load_module+0x32b5/0x3610
[  171.659644]  ? __do_sys_init_module+0x148/0x1a0
[  171.659646]  ? __x64_sys_init_module+0x1d/0x20
[  171.659649]  ? do_syscall_64+0x4a/0xb0
[  171.659652]  ? entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.659656]  ? 0xffffffffc040a000
[  171.659658]  alloctest_ref_tracker_free+0x13/0x20 [test_ref_tracker]
[  171.659660]  init_module+0x105/0x1000 [test_ref_tracker]
[  171.659663]  do_one_initcall+0x9c/0x220
[  171.659666]  do_init_module+0x60/0x240
[  171.659669]  load_module+0x32b5/0x3610
[  171.659672]  __do_sys_init_module+0x148/0x1a0
[  171.659676]  __x64_sys_init_module+0x1d/0x20
[  171.659678]  do_syscall_64+0x4a/0xb0
[  171.659694]  ? exc_page_fault+0x6e/0x140
[  171.659696]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.659698] RIP: 0033:0x7f97ea3dbe7a
[  171.659700] Code: 48 8b 0d 61 8d 06 00 f7 d8 64 89 01 48 83 c8 ff c3 cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 49 89 ca b8 af 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 2e 8d 06 00 f7 d8 64 89 01 48
[  171.659701] RSP: 002b:00007ffea67ce608 EFLAGS: 00000246 ORIG_RAX: 00000000000000af
[  171.659703] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f97ea3dbe7a
[  171.659704] RDX: 00000000013a0ba0 RSI: 0000000000002808 RDI: 00007f97ea299000
[  171.659705] RBP: 00007ffea67ce670 R08: 0000000000000003 R09: 0000000000000000
[  171.659706] R10: 0000000000000000 R11: 0000000000000246 R12: 00000000013a1048
[  171.659707] R13: 00000000013a0ba0 R14: 0000000001399930 R15: 00000000013a1030
[  171.659709]  </TASK>
[  171.659710] ---[ end trace f5dbd6afa41e60a9 ]---
[  171.659712] leaked reference.
[  171.663393]  alloctest_ref_tracker_alloc0+0x1c/0x20 [test_ref_tracker]
[  171.663395]  test_ref_tracker_timer_func+0x9/0x20 [test_ref_tracker]
[  171.663397]  call_timer_fn+0x31/0x140
[  171.663401]  expire_timers+0x46/0x110
[  171.663403]  __run_timers+0x16f/0x1b0
[  171.663404]  run_timer_softirq+0x1d/0x40
[  171.663406]  __do_softirq+0x148/0x2d3
[  171.663408] leaked reference.
[  171.667101]  alloctest_ref_tracker_alloc1+0x1c/0x20 [test_ref_tracker]
[  171.667103]  init_module+0x81/0x1000 [test_ref_tracker]
[  171.667104]  do_one_initcall+0x9c/0x220
[  171.667106]  do_init_module+0x60/0x240
[  171.667108]  load_module+0x32b5/0x3610
[  171.667111]  __do_sys_init_module+0x148/0x1a0
[  171.667113]  __x64_sys_init_module+0x1d/0x20
[  171.667115]  do_syscall_64+0x4a/0xb0
[  171.667117]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.667131] ------------[ cut here ]------------
[  171.667132] WARNING: CPU: 5 PID: 13016 at lib/ref_tracker.c:30 ref_tracker_dir_exit+0x104/0x130
[  171.667136] Modules linked in: test_ref_tracker(+)
[  171.667144] CPU: 5 PID: 13016 Comm: modprobe Tainted: G S      W         5.16.0-smp-DEV #290
[  171.667147] RIP: 0010:ref_tracker_dir_exit+0x104/0x130
[  171.667150] Code: 01 00 00 00 00 ad de 48 89 03 4c 89 63 08 48 89 df e8 20 a0 d5 ff 4c 89 f3 4d 39 ee 75 a8 4c 89 ff 48 8b 75 d0 e8 7c 05 69 00 <0f> 0b eb 0c 4c 89 ff 48 8b 75 d0 e8 6c 05 69 00 41 8b 47 08 83 f8
[  171.667151] RSP: 0018:ffff89058ba0bc68 EFLAGS: 00010286
[  171.667154] RAX: 08895bff57c7d100 RBX: ffffffffc0407010 RCX: 000000000000003b
[  171.667156] RDX: 000000000000003c RSI: 0000000000000282 RDI: ffffffffc0407000
[  171.667157] RBP: ffff89058ba0bc98 R08: 0000000000000000 R09: ffffffffa6f342e0
[  171.667159] R10: 00000000ffff7fff R11: 0000000000000000 R12: dead000000000122
[  171.667160] R13: ffffffffc0407010 R14: ffffffffc0407010 R15: ffffffffc0407000
[  171.667162] FS:  00007f97ea29d740(0000) GS:ffff8923ff940000(0000) knlGS:0000000000000000
[  171.667164] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  171.667166] CR2: 00007f97ea299000 CR3: 0000000186b4a004 CR4: 00000000001706e0
[  171.667169] Call Trace:
[  171.667170]  <TASK>
[  171.667171]  ? 0xffffffffc040a000
[  171.667173]  init_module+0x126/0x1000 [test_ref_tracker]
[  171.667175]  do_one_initcall+0x9c/0x220
[  171.667179]  do_init_module+0x60/0x240
[  171.667182]  load_module+0x32b5/0x3610
[  171.667186]  __do_sys_init_module+0x148/0x1a0
[  171.667189]  __x64_sys_init_module+0x1d/0x20
[  171.667192]  do_syscall_64+0x4a/0xb0
[  171.667194]  ? exc_page_fault+0x6e/0x140
[  171.667196]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  171.667199] RIP: 0033:0x7f97ea3dbe7a
[  171.667200] Code: 48 8b 0d 61 8d 06 00 f7 d8 64 89 01 48 83 c8 ff c3 cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 49 89 ca b8 af 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 2e 8d 06 00 f7 d8 64 89 01 48
[  171.667201] RSP: 002b:00007ffea67ce608 EFLAGS: 00000246 ORIG_RAX: 00000000000000af
[  171.667203] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f97ea3dbe7a
[  171.667204] RDX: 00000000013a0ba0 RSI: 0000000000002808 RDI: 00007f97ea299000
[  171.667205] RBP: 00007ffea67ce670 R08: 0000000000000003 R09: 0000000000000000
[  171.667206] R10: 0000000000000000 R11: 0000000000000246 R12: 00000000013a1048
[  171.667207] R13: 00000000013a0ba0 R14: 0000000001399930 R15: 00000000013a1030
[  171.667209]  </TASK>
[  171.667210] ---[ end trace f5dbd6afa41e60aa ]---

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 lib/Kconfig.debug      |  10 ++++
 lib/Makefile           |   2 +-
 lib/test_ref_tracker.c | 116 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_ref_tracker.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5c12bde10996cf97b5f075d318089b1be73f71d7..d005f555872147e15d3e0a65d2a03e1a5c44f5f0 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2106,6 +2106,16 @@ config BACKTRACE_SELF_TEST
 
 	  Say N if you are unsure.
 
+config TEST_REF_TRACKER
+	tristate "Self test for reference tracker"
+	depends on DEBUG_KERNEL
+	select REF_TRACKER
+	help
+	  This option provides a kernel module performing tests
+	  using reference tracker infrastructure.
+
+	  Say N if you are unsure.
+
 config RBTREE_TEST
 	tristate "Red-Black tree test"
 	depends on DEBUG_KERNEL
diff --git a/lib/Makefile b/lib/Makefile
index c1fd9243ddb9cc1ac5252d7eb8009f9290782c4a..b213a7bbf3fda2eb9f234fb7473b8f1b617bed6b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -101,7 +101,7 @@ obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
 obj-$(CONFIG_TEST_HMM) += test_hmm.o
 obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
-
+obj-$(CONFIG_TEST_REF_TRACKER) += test_ref_tracker.o
 #
 # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
 # off the generation of FPU/SSE* instructions for kernel proper but FPU_FLAGS
diff --git a/lib/test_ref_tracker.c b/lib/test_ref_tracker.c
new file mode 100644
index 0000000000000000000000000000000000000000..73bf9255e03790fa50491fe8e5cd411d54827c45
--- /dev/null
+++ b/lib/test_ref_tracker.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Referrence tracker self test.
+ *
+ * Copyright (c) 2021 Eric Dumazet <edumazet@google.com>
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/ref_tracker.h>
+#include <linux/slab.h>
+#include <linux/timer.h>
+
+static struct ref_tracker_dir ref_dir;
+static struct ref_tracker *tracker[20];
+
+#define TRT_ALLOC(X) static noinline void 					\
+	alloctest_ref_tracker_alloc##X(struct ref_tracker_dir *dir, 	\
+				    struct ref_tracker **trackerp)	\
+	{								\
+		ref_tracker_alloc(dir, trackerp, GFP_KERNEL);		\
+	}
+
+TRT_ALLOC(0)
+TRT_ALLOC(1)
+TRT_ALLOC(2)
+TRT_ALLOC(3)
+TRT_ALLOC(4)
+TRT_ALLOC(5)
+TRT_ALLOC(6)
+TRT_ALLOC(7)
+TRT_ALLOC(8)
+TRT_ALLOC(9)
+TRT_ALLOC(10)
+TRT_ALLOC(11)
+TRT_ALLOC(12)
+TRT_ALLOC(13)
+TRT_ALLOC(14)
+TRT_ALLOC(15)
+TRT_ALLOC(16)
+TRT_ALLOC(17)
+TRT_ALLOC(18)
+TRT_ALLOC(19)
+
+#undef TRT_ALLOC
+
+static noinline void
+alloctest_ref_tracker_free(struct ref_tracker_dir *dir,
+			   struct ref_tracker **trackerp)
+{
+	ref_tracker_free(dir, trackerp);
+}
+
+
+static struct timer_list test_ref_tracker_timer;
+static atomic_t test_ref_timer_done = ATOMIC_INIT(0);
+
+static void test_ref_tracker_timer_func(struct timer_list *t)
+{
+	alloctest_ref_tracker_alloc0(&ref_dir, &tracker[0]);
+	atomic_set(&test_ref_timer_done, 1);
+}
+
+static int __init test_ref_tracker_init(void)
+{
+	int i;
+
+	ref_tracker_dir_init(&ref_dir, 100);
+
+	timer_setup(&test_ref_tracker_timer, test_ref_tracker_timer_func, 0);
+	mod_timer(&test_ref_tracker_timer, jiffies + 1);
+
+	alloctest_ref_tracker_alloc1(&ref_dir, &tracker[1]);
+	alloctest_ref_tracker_alloc2(&ref_dir, &tracker[2]);
+	alloctest_ref_tracker_alloc3(&ref_dir, &tracker[3]);
+	alloctest_ref_tracker_alloc4(&ref_dir, &tracker[4]);
+	alloctest_ref_tracker_alloc5(&ref_dir, &tracker[5]);
+	alloctest_ref_tracker_alloc6(&ref_dir, &tracker[6]);
+	alloctest_ref_tracker_alloc7(&ref_dir, &tracker[7]);
+	alloctest_ref_tracker_alloc8(&ref_dir, &tracker[8]);
+	alloctest_ref_tracker_alloc9(&ref_dir, &tracker[9]);
+	alloctest_ref_tracker_alloc10(&ref_dir, &tracker[10]);
+	alloctest_ref_tracker_alloc11(&ref_dir, &tracker[11]);
+	alloctest_ref_tracker_alloc12(&ref_dir, &tracker[12]);
+	alloctest_ref_tracker_alloc13(&ref_dir, &tracker[13]);
+	alloctest_ref_tracker_alloc14(&ref_dir, &tracker[14]);
+	alloctest_ref_tracker_alloc15(&ref_dir, &tracker[15]);
+	alloctest_ref_tracker_alloc16(&ref_dir, &tracker[16]);
+	alloctest_ref_tracker_alloc17(&ref_dir, &tracker[17]);
+	alloctest_ref_tracker_alloc18(&ref_dir, &tracker[18]);
+	alloctest_ref_tracker_alloc19(&ref_dir, &tracker[19]);
+
+	/* free all trackers but first 0 and 1. */
+	for (i = 2; i < ARRAY_SIZE(tracker); i++)
+		alloctest_ref_tracker_free(&ref_dir, &tracker[i]);
+
+	/* Attempt to free an already freed tracker. */
+	alloctest_ref_tracker_free(&ref_dir, &tracker[2]);
+
+	while (!atomic_read(&test_ref_timer_done))
+		msleep(1);
+
+	/* This should warn about tracker[0] & tracker[1] being not freed. */
+	ref_tracker_dir_exit(&ref_dir);
+
+	return 0;
+}
+
+static void __exit test_ref_tracker_exit(void)
+{
+}
+
+module_init(test_ref_tracker_init);
+module_exit(test_ref_tracker_exit);
+
+MODULE_LICENSE("GPL v2");
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 03/19] net: add dev_hold_track() and dev_put_track() helpers

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

They should replace dev_hold() and dev_put().

To use these helpers, each data structure owning a refcount
should also use a "netdevice_tracker" to pair the hold and put.

Whenever a leak happens, we will get precise stack traces
of the point dev_hold_track() happened, at device dismantle phase.

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

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index db3bff1ae7fdf5ff0b0546cbd0102b86f04fa144..3ddced0fa20f5c7a4548c340788214ea909a265f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -48,6 +48,7 @@
 #include <uapi/linux/pkt_cls.h>
 #include <linux/hashtable.h>
 #include <linux/rbtree.h>
+#include <linux/ref_tracker.h>
 
 struct netpoll_info;
 struct device;
@@ -300,6 +301,12 @@ enum netdev_state_t {
 };
 
 
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+typedef struct ref_tracker *netdevice_tracker;
+#else
+typedef struct {} netdevice_tracker;
+#endif
+
 struct gro_list {
 	struct list_head	list;
 	int			count;
@@ -2178,6 +2185,7 @@ struct net_device {
 #else
 	refcount_t		dev_refcnt;
 #endif
+	struct ref_tracker_dir	refcnt_tracker;
 
 	struct list_head	link_watch_list;
 
@@ -3805,6 +3813,7 @@ void netdev_run_todo(void);
  *	@dev: network device
  *
  * Release reference to device to allow it to be freed.
+ * Try using dev_put_track() instead.
  */
 static inline void dev_put(struct net_device *dev)
 {
@@ -3822,6 +3831,7 @@ static inline void dev_put(struct net_device *dev)
  *	@dev: network device
  *
  * Hold reference to device to keep it from being freed.
+ * Try using dev_hold_track() instead.
  */
 static inline void dev_hold(struct net_device *dev)
 {
@@ -3834,6 +3844,28 @@ static inline void dev_hold(struct net_device *dev)
 	}
 }
 
+static inline void dev_hold_track(struct net_device *dev,
+				  netdevice_tracker *tracker, gfp_t gfp)
+{
+	if (dev) {
+		dev_hold(dev);
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+		ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp);
+#endif
+	}
+}
+
+static inline void dev_put_track(struct net_device *dev,
+				 netdevice_tracker *tracker)
+{
+	if (dev) {
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+		ref_tracker_free(&dev->refcnt_tracker, tracker);
+#endif
+		dev_put(dev);
+	}
+}
+
 /* Carrier loss detection, dial on demand. The functions netif_carrier_on
  * and _off may be called from IRQ context, but it is caller
  * who is responsible for serialization of these calls.
diff --git a/net/Kconfig b/net/Kconfig
index 8a1f9d0287de3c32040eee03b60114c6e6d150bc..0b665e60b53490f44eeda1e5506d4e125ef4c53a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -253,6 +253,14 @@ config PCPU_DEV_REFCNT
 	  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 NET_DEV_REFCNT_TRACKER
+	bool "Enable tracking in dev_put_track() and dev_hold_track()"
+	select REF_TRACKER
+	default n
+	help
+	  Enable debugging feature to track leaked device references.
+	  This adds memory and cpu costs.
+
 config RPS
 	bool
 	depends on SMP && SYSFS
diff --git a/net/core/dev.c b/net/core/dev.c
index d30adecc2bb2b744b283dbda89d9488b8eb6d47e..bdfbfa4e291858f990f5e2c99c4ce0ee9ed687cd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9861,6 +9861,7 @@ static void netdev_wait_allrefs(struct net_device *dev)
 			       netdev_unregister_timeout_secs * HZ)) {
 			pr_emerg("unregister_netdevice: waiting for %s to become free. Usage count = %d\n",
 				 dev->name, refcnt);
+			ref_tracker_dir_print(&dev->refcnt_tracker, 10);
 			warning_time = jiffies;
 		}
 	}
@@ -10151,6 +10152,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	dev = PTR_ALIGN(p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
 
+	ref_tracker_dir_init(&dev->refcnt_tracker, 128);
 #ifdef CONFIG_PCPU_DEV_REFCNT
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
@@ -10267,6 +10269,7 @@ void free_netdev(struct net_device *dev)
 	list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
 		netif_napi_del(p);
 
+	ref_tracker_dir_exit(&dev->refcnt_tracker);
 #ifdef CONFIG_PCPU_DEV_REFCNT
 	free_percpu(dev->pcpu_refcnt);
 	dev->pcpu_refcnt = NULL;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 04/19] net: add net device refcount tracker to struct netdev_rx_queue

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This helps debugging net device refcount leaks.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h | 2 ++
 net/core/net-sysfs.c      | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ddced0fa20f5c7a4548c340788214ea909a265f..01006e02f8e66fc3ee16890d3bbdb49e3e7386b6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -741,6 +741,8 @@ struct netdev_rx_queue {
 #endif
 	struct kobject			kobj;
 	struct net_device		*dev;
+	netdevice_tracker		dev_tracker;
+
 #ifdef CONFIG_XDP_SOCKETS
 	struct xsk_buff_pool            *pool;
 #endif
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index affe34d71d313498d8fae4a319696250aa3aef0a..27a7ac2e516f65dbfdb2a2319e6faa27c7dd8f31 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1004,7 +1004,7 @@ static void rx_queue_release(struct kobject *kobj)
 #endif
 
 	memset(kobj, 0, sizeof(*kobj));
-	dev_put(queue->dev);
+	dev_put_track(queue->dev, &queue->dev_tracker);
 }
 
 static const void *rx_queue_namespace(struct kobject *kobj)
@@ -1044,7 +1044,7 @@ static int rx_queue_add_kobject(struct net_device *dev, int index)
 	/* Kobject_put later will trigger rx_queue_release call which
 	 * decreases dev refcount: Take that reference here
 	 */
-	dev_hold(queue->dev);
+	dev_hold_track(queue->dev, &queue->dev_tracker, GFP_KERNEL);
 
 	kobj->kset = dev->queues_kset;
 	error = kobject_init_and_add(kobj, &rx_queue_ktype, NULL,
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 05/19] net: add net device refcount tracker to struct netdev_queue

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This will help debugging pesky netdev reference leaks.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h | 2 ++
 net/core/net-sysfs.c      | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 01006e02f8e66fc3ee16890d3bbdb49e3e7386b6..b43102614696a97c919f601d1337746443d79557 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -586,6 +586,8 @@ struct netdev_queue {
  * read-mostly part
  */
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
+
 	struct Qdisc __rcu	*qdisc;
 	struct Qdisc		*qdisc_sleeping;
 #ifdef CONFIG_SYSFS
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 27a7ac2e516f65dbfdb2a2319e6faa27c7dd8f31..3b2cdbbdc858e06fb0a482a9b7fc778e501ba1e0 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1607,7 +1607,7 @@ static void netdev_queue_release(struct kobject *kobj)
 	struct netdev_queue *queue = to_netdev_queue(kobj);
 
 	memset(kobj, 0, sizeof(*kobj));
-	dev_put(queue->dev);
+	dev_put_track(queue->dev, &queue->dev_tracker);
 }
 
 static const void *netdev_queue_namespace(struct kobject *kobj)
@@ -1647,7 +1647,7 @@ static int netdev_queue_add_kobject(struct net_device *dev, int index)
 	/* Kobject_put later will trigger netdev_queue_release call
 	 * which decreases dev refcount: Take that reference here
 	 */
-	dev_hold(queue->dev);
+	dev_hold_track(queue->dev, &queue->dev_tracker, GFP_KERNEL);
 
 	kobj->kset = dev->queues_kset;
 	error = kobject_init_and_add(kobj, &netdev_queue_ktype, NULL,
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 06/19] net: add net device refcount tracker to ethtool_phys_id()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This helper might hold a netdev reference for a long time,
lets add reference tracking.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ethtool/ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index fa8aa5ec19ba1cfff611a159c5b8bcd02f74c5ca..9a113d89352123acf56ec598c191bb75985c6be5 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1989,6 +1989,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 	struct ethtool_value id;
 	static bool busy;
 	const struct ethtool_ops *ops = dev->ethtool_ops;
+	netdevice_tracker dev_tracker;
 	int rc;
 
 	if (!ops->set_phys_id)
@@ -2008,7 +2009,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 	 * removal of the device.
 	 */
 	busy = true;
-	dev_hold(dev);
+	dev_hold_track(dev, &dev_tracker, GFP_KERNEL);
 	rtnl_unlock();
 
 	if (rc == 0) {
@@ -2032,7 +2033,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 	}
 
 	rtnl_lock();
-	dev_put(dev);
+	dev_put_track(dev, &dev_tracker);
 	busy = false;
 
 	(void) ops->set_phys_id(dev, ETHTOOL_ID_INACTIVE);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 07/19] net: add net device refcount tracker to dev_ifsioc()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev_ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index cbab5fec64b12df9355b154bdcaa0f48701966e8..1d309a6669325ad26c59892c4e78ca14dec017d9 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -313,6 +313,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 	int err;
 	struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
 	const struct net_device_ops *ops;
+	netdevice_tracker dev_tracker;
 
 	if (!dev)
 		return -ENODEV;
@@ -381,10 +382,10 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 			return -ENODEV;
 		if (!netif_is_bridge_master(dev))
 			return -EOPNOTSUPP;
-		dev_hold(dev);
+		dev_hold_track(dev, &dev_tracker, GFP_KERNEL);
 		rtnl_unlock();
 		err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL);
-		dev_put(dev);
+		dev_put_track(dev, &dev_tracker);
 		rtnl_lock();
 		return err;
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 08/19] drop_monitor: add net device refcount tracker

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

We want to track all dev_hold()/dev_put() to ease leak hunting.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/devlink.h   | 3 +++
 net/core/drop_monitor.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 043fcec8b0aadf041aba35b8339c93ac9336b551..09b75fdfa74e268aaeb05ec640fd76ec5ba777ac 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -670,7 +670,10 @@ struct devlink_health_reporter_ops {
 struct devlink_trap_metadata {
 	const char *trap_name;
 	const char *trap_group_name;
+
 	struct net_device *input_dev;
+	netdevice_tracker dev_tracker;
+
 	const struct flow_action_cookie *fa_cookie;
 	enum devlink_trap_type trap_type;
 };
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 49442cae6f69d5e9d93d00b53ab8f5a0563c1d37..dff13c208c1dafac26c3180a37e6e3be5f8fa744 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -850,7 +850,7 @@ net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
 	}
 
 	hw_metadata->input_dev = metadata->input_dev;
-	dev_hold(hw_metadata->input_dev);
+	dev_hold_track(hw_metadata->input_dev, &hw_metadata->dev_tracker, GFP_ATOMIC);
 
 	return hw_metadata;
 
@@ -866,7 +866,7 @@ net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
 static void
 net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)
 {
-	dev_put(hw_metadata->input_dev);
+	dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
 	kfree(hw_metadata->fa_cookie);
 	kfree(hw_metadata->trap_name);
 	kfree(hw_metadata->trap_group_name);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 09/19] net: dst: add net device refcount tracking to dst_entry

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

We want to track all dev_hold()/dev_put() to ease leak hunting.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/netdevice.h | 17 +++++++++++++++++
 include/net/dst.h         |  1 +
 net/core/dst.c            |  8 ++++----
 net/ipv4/route.c          |  7 ++++---
 net/ipv6/route.c          |  5 +++--
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b43102614696a97c919f601d1337746443d79557..bfbf7c52688396093740c16aeb2e6947eaa9ac9c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3870,6 +3870,23 @@ static inline void dev_put_track(struct net_device *dev,
 	}
 }
 
+static inline void dev_replace_track(struct net_device *odev,
+				     struct net_device *ndev,
+				     netdevice_tracker *tracker,
+				     gfp_t gfp)
+{
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+	if (odev)
+		ref_tracker_free(&odev->refcnt_tracker, tracker);
+#endif
+	dev_hold(ndev);
+	dev_put(odev);
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+	if (ndev)
+		ref_tracker_alloc(&ndev->refcnt_tracker, tracker, gfp);
+#endif
+}
+
 /* Carrier loss detection, dial on demand. The functions netif_carrier_on
  * and _off may be called from IRQ context, but it is caller
  * who is responsible for serialization of these calls.
diff --git a/include/net/dst.h b/include/net/dst.h
index a057319aabefac52075edb038358439ceec23a60..6aa252c3fc55ccaee58faebf265510469e91d780 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -77,6 +77,7 @@ struct dst_entry {
 #ifndef CONFIG_64BIT
 	atomic_t		__refcnt;	/* 32-bit offset 64 */
 #endif
+	netdevice_tracker	dev_tracker;
 };
 
 struct dst_metrics {
diff --git a/net/core/dst.c b/net/core/dst.c
index 497ef9b3fc6abb5fd9c74e5730c3aedc3277f850..d16c2c9bfebd3dadd4c8dbc4f14836574bb52bbe 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -49,7 +49,7 @@ void dst_init(struct dst_entry *dst, struct dst_ops *ops,
 	      unsigned short flags)
 {
 	dst->dev = dev;
-	dev_hold(dev);
+	dev_hold_track(dev, &dst->dev_tracker, GFP_ATOMIC);
 	dst->ops = ops;
 	dst_init_metrics(dst, dst_default_metrics.metrics, true);
 	dst->expires = 0UL;
@@ -117,7 +117,7 @@ struct dst_entry *dst_destroy(struct dst_entry * dst)
 
 	if (dst->ops->destroy)
 		dst->ops->destroy(dst);
-	dev_put(dst->dev);
+	dev_put_track(dst->dev, &dst->dev_tracker);
 
 	lwtstate_put(dst->lwtstate);
 
@@ -159,8 +159,8 @@ void dst_dev_put(struct dst_entry *dst)
 	dst->input = dst_discard;
 	dst->output = dst_discard_out;
 	dst->dev = blackhole_netdev;
-	dev_hold(dst->dev);
-	dev_put(dev);
+	dev_replace_track(dev, blackhole_netdev, &dst->dev_tracker,
+			  GFP_ATOMIC);
 }
 EXPORT_SYMBOL(dst_dev_put);
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 243a0c52be42b60226a38dce980956b33e583d80..843a7a3699feeb24f3b9af5efaff87724214cbce 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1531,8 +1531,9 @@ void rt_flush_dev(struct net_device *dev)
 			if (rt->dst.dev != dev)
 				continue;
 			rt->dst.dev = blackhole_netdev;
-			dev_hold(rt->dst.dev);
-			dev_put(dev);
+			dev_replace_track(dev, blackhole_netdev,
+					  &rt->dst.dev_tracker,
+					  GFP_ATOMIC);
 		}
 		spin_unlock_bh(&ul->lock);
 	}
@@ -2819,7 +2820,7 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
 		new->output = dst_discard_out;
 
 		new->dev = net->loopback_dev;
-		dev_hold(new->dev);
+		dev_hold_track(new->dev, &new->dev_tracker, GFP_ATOMIC);
 
 		rt->rt_is_input = ort->rt_is_input;
 		rt->rt_iif = ort->rt_iif;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f0d29fcb20945b5a48097c89dc57daedeed9d177..ba4dc94d76d63c98ff49c41b712231f81eb8af40 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -182,8 +182,9 @@ static void rt6_uncached_list_flush_dev(struct net *net, struct net_device *dev)
 
 			if (rt_dev == dev) {
 				rt->dst.dev = blackhole_netdev;
-				dev_hold(rt->dst.dev);
-				dev_put(rt_dev);
+				dev_replace_track(rt_dev, blackhole_netdev,
+						  &rt->dst.dev_tracker,
+						  GFP_ATOMIC);
 			}
 		}
 		spin_unlock_bh(&ul->lock);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 10/19] ipv6: add net device refcount tracker to rt6_probe_deferred()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/route.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ba4dc94d76d63c98ff49c41b712231f81eb8af40..8d834f901b483edf75c493620d38f979a4bcbf69 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -593,6 +593,7 @@ struct __rt6_probe_work {
 	struct work_struct work;
 	struct in6_addr target;
 	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 };
 
 static void rt6_probe_deferred(struct work_struct *w)
@@ -603,7 +604,7 @@ static void rt6_probe_deferred(struct work_struct *w)
 
 	addrconf_addr_solict_mult(&work->target, &mcaddr);
 	ndisc_send_ns(work->dev, &work->target, &mcaddr, NULL, 0);
-	dev_put(work->dev);
+	dev_put_track(work->dev, &work->dev_tracker);
 	kfree(work);
 }
 
@@ -657,7 +658,7 @@ static void rt6_probe(struct fib6_nh *fib6_nh)
 	} else {
 		INIT_WORK(&work->work, rt6_probe_deferred);
 		work->target = *nh_gw;
-		dev_hold(dev);
+		dev_hold_track(dev, &work->dev_tracker, GFP_KERNEL);
 		work->dev = dev;
 		schedule_work(&work->work);
 	}
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 11/19] sit: add net device refcount tracking to ip_tunnel

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Note that other ip_tunnel users do not seem to hold a reference
on tunnel->dev. Probably needs some investigations.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ip_tunnels.h | 3 +++
 net/ipv6/sit.c           | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index bc3b13ec93c9dcd3f5d4c0ad8598200912172863..0219fe907b261952ec1f140d105cd74d7eda6b40 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -104,7 +104,10 @@ struct metadata_dst;
 struct ip_tunnel {
 	struct ip_tunnel __rcu	*next;
 	struct hlist_node hash_node;
+
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
+
 	struct net		*net;	/* netns for packet i/o */
 
 	unsigned long	err_time;	/* Time when the last ICMP error
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 1b57ee36d6682e04085aa271c6c5c09e6e3a7b7e..057c0f83c8007fb0756ca0d3a2231fab8eebaacb 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -521,7 +521,7 @@ static void ipip6_tunnel_uninit(struct net_device *dev)
 		ipip6_tunnel_del_prl(tunnel, NULL);
 	}
 	dst_cache_reset(&tunnel->dst_cache);
-	dev_put(dev);
+	dev_put_track(dev, &tunnel->dev_tracker);
 }
 
 static int ipip6_err(struct sk_buff *skb, u32 info)
@@ -1463,7 +1463,7 @@ static int ipip6_tunnel_init(struct net_device *dev)
 		dev->tstats = NULL;
 		return err;
 	}
-	dev_hold(dev);
+	dev_hold_track(dev, &tunnel->dev_tracker, GFP_KERNEL);
 	return 0;
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 12/19] ipv6: add net device refcount tracker to struct ip6_tnl

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ip6_tunnel.h | 1 +
 net/ipv6/ip6_gre.c       | 8 ++++----
 net/ipv6/ip6_tunnel.c    | 4 ++--
 net/ipv6/ip6_vti.c       | 4 ++--
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 028eaea1c854493fdab40f655216230e991e2fc5..a38c4f1e4e5c641dcede4d7fedfcdbfadbac430e 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -46,6 +46,7 @@ struct __ip6_tnl_parm {
 struct ip6_tnl {
 	struct ip6_tnl __rcu *next;	/* next tunnel in list */
 	struct net_device *dev;	/* virtual device associated with tunnel */
+	netdevice_tracker dev_tracker;
 	struct net *net;	/* netns for packet i/o */
 	struct __ip6_tnl_parm parms;	/* tunnel configuration parameters */
 	struct flowi fl;	/* flowi template for xmit */
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index d831d243969327b2cd4b539199e02a897a6c14f7..110839a88bc2ef098df38196f997335501305ad7 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -403,7 +403,7 @@ static void ip6erspan_tunnel_uninit(struct net_device *dev)
 	ip6erspan_tunnel_unlink_md(ign, t);
 	ip6gre_tunnel_unlink(ign, t);
 	dst_cache_reset(&t->dst_cache);
-	dev_put(dev);
+	dev_put_track(dev, &t->dev_tracker);
 }
 
 static void ip6gre_tunnel_uninit(struct net_device *dev)
@@ -416,7 +416,7 @@ static void ip6gre_tunnel_uninit(struct net_device *dev)
 	if (ign->fb_tunnel_dev == dev)
 		WRITE_ONCE(ign->fb_tunnel_dev, NULL);
 	dst_cache_reset(&t->dst_cache);
-	dev_put(dev);
+	dev_put_track(dev, &t->dev_tracker);
 }
 
 
@@ -1496,7 +1496,7 @@ static int ip6gre_tunnel_init_common(struct net_device *dev)
 	}
 	ip6gre_tnl_init_features(dev);
 
-	dev_hold(dev);
+	dev_hold_track(dev, &tunnel->dev_tracker, GFP_KERNEL);
 	return 0;
 
 cleanup_dst_cache_init:
@@ -1888,7 +1888,7 @@ static int ip6erspan_tap_init(struct net_device *dev)
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 	ip6erspan_tnl_link_config(tunnel, 1);
 
-	dev_hold(dev);
+	dev_hold_track(dev, &tunnel->dev_tracker, GFP_KERNEL);
 	return 0;
 
 cleanup_dst_cache_init:
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 484aca492cc06858319e87ba9c989de9f378e896..fe786df4f8493396b803002d25701affd59ee96c 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -383,7 +383,7 @@ ip6_tnl_dev_uninit(struct net_device *dev)
 	else
 		ip6_tnl_unlink(ip6n, t);
 	dst_cache_reset(&t->dst_cache);
-	dev_put(dev);
+	dev_put_track(dev, &t->dev_tracker);
 }
 
 /**
@@ -1883,7 +1883,7 @@ ip6_tnl_dev_init_gen(struct net_device *dev)
 	dev->min_mtu = ETH_MIN_MTU;
 	dev->max_mtu = IP6_MAX_MTU - dev->hard_header_len;
 
-	dev_hold(dev);
+	dev_hold_track(dev, &t->dev_tracker, GFP_KERNEL);
 	return 0;
 
 destroy_dst:
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 527e9ead7449e5229db11b73622ff723847ffc96..ed9b6d6ca65e0173c9f03e580cca2747ad023a99 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -293,7 +293,7 @@ static void vti6_dev_uninit(struct net_device *dev)
 		RCU_INIT_POINTER(ip6n->tnls_wc[0], NULL);
 	else
 		vti6_tnl_unlink(ip6n, t);
-	dev_put(dev);
+	dev_put_track(dev, &t->dev_tracker);
 }
 
 static int vti6_input_proto(struct sk_buff *skb, int nexthdr, __be32 spi,
@@ -934,7 +934,7 @@ static inline int vti6_dev_init_gen(struct net_device *dev)
 	dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
 	if (!dev->tstats)
 		return -ENOMEM;
-	dev_hold(dev);
+	dev_hold_track(dev, &t->dev_tracker, GFP_KERNEL);
 	return 0;
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 13/19] net: add net device refcount tracker to struct neighbour

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/neighbour.h | 1 +
 net/core/neighbour.c    | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 55af812c3ed5704813bd64aa760246255aa5f93f..190b07fe089ef5c900a0d97df0bc4d667d8bdcd6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -158,6 +158,7 @@ struct neighbour {
 	struct list_head	managed_list;
 	struct rcu_head		rcu;
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
 	u8			primary_key[0];
 } __randomize_layout;
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 72ba027c34cfea6f38a9e78927c35048ebfe7a7f..fb340347e4d88f0058383697071cfb5bfbd9f925 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -624,7 +624,7 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 
 	memcpy(n->primary_key, pkey, key_len);
 	n->dev = dev;
-	dev_hold(dev);
+	dev_hold_track(dev, &n->dev_tracker, GFP_ATOMIC);
 
 	/* Protocol specific setup. */
 	if (tbl->constructor &&	(error = tbl->constructor(n)) < 0) {
@@ -880,7 +880,7 @@ void neigh_destroy(struct neighbour *neigh)
 	if (dev->netdev_ops->ndo_neigh_destroy)
 		dev->netdev_ops->ndo_neigh_destroy(dev, neigh);
 
-	dev_put(dev);
+	dev_put_track(dev, &neigh->dev_tracker);
 	neigh_parms_put(neigh->parms);
 
 	neigh_dbg(2, "neigh %p is destroyed\n", neigh);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 14/19] net: add net device refcount tracker to struct pneigh_entry

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/neighbour.h | 1 +
 net/core/neighbour.c    | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 190b07fe089ef5c900a0d97df0bc4d667d8bdcd6..5fffb783670a6d2432896a06d3044f6ac83feaf4 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -174,6 +174,7 @@ struct pneigh_entry {
 	struct pneigh_entry	*next;
 	possible_net_t		net;
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
 	u32			flags;
 	u8			protocol;
 	u8			key[];
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index fb340347e4d88f0058383697071cfb5bfbd9f925..56de74f8d2b1c896c478ded2c659b0207d7b5c75 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -771,10 +771,10 @@ struct pneigh_entry * pneigh_lookup(struct neigh_table *tbl,
 	write_pnet(&n->net, net);
 	memcpy(n->key, pkey, key_len);
 	n->dev = dev;
-	dev_hold(dev);
+	dev_hold_track(dev, &n->dev_tracker, GFP_KERNEL);
 
 	if (tbl->pconstructor && tbl->pconstructor(n)) {
-		dev_put(dev);
+		dev_put_track(dev, &n->dev_tracker);
 		kfree(n);
 		n = NULL;
 		goto out;
@@ -806,7 +806,7 @@ int pneigh_delete(struct neigh_table *tbl, struct net *net, const void *pkey,
 			write_unlock_bh(&tbl->lock);
 			if (tbl->pdestructor)
 				tbl->pdestructor(n);
-			dev_put(n->dev);
+			dev_put_track(n->dev, &n->dev_tracker);
 			kfree(n);
 			return 0;
 		}
@@ -839,7 +839,7 @@ static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
 		n->next = NULL;
 		if (tbl->pdestructor)
 			tbl->pdestructor(n);
-		dev_put(n->dev);
+		dev_put_track(n->dev, &n->dev_tracker);
 		kfree(n);
 	}
 	return -ENOENT;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 15/19] net: add net device refcount tracker to struct neigh_parms

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/neighbour.h | 1 +
 net/core/neighbour.c    | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 5fffb783670a6d2432896a06d3044f6ac83feaf4..937389e04c8e26b6f5a4c1362a90930145671889 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -70,6 +70,7 @@ enum {
 struct neigh_parms {
 	possible_net_t net;
 	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 	struct list_head list;
 	int	(*neigh_setup)(struct neighbour *);
 	struct neigh_table *tbl;
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 56de74f8d2b1c896c478ded2c659b0207d7b5c75..dd271ffedf11cdd3a1b6bf5fad7b0ddcc5d41e80 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1666,13 +1666,13 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
 		refcount_set(&p->refcnt, 1);
 		p->reachable_time =
 				neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
-		dev_hold(dev);
+		dev_hold_track(dev, &p->dev_tracker, GFP_KERNEL);
 		p->dev = dev;
 		write_pnet(&p->net, net);
 		p->sysctl_table = NULL;
 
 		if (ops->ndo_neigh_setup && ops->ndo_neigh_setup(dev, p)) {
-			dev_put(dev);
+			dev_put_track(dev, &p->dev_tracker);
 			kfree(p);
 			return NULL;
 		}
@@ -1703,7 +1703,7 @@ void neigh_parms_release(struct neigh_table *tbl, struct neigh_parms *parms)
 	list_del(&parms->list);
 	parms->dead = 1;
 	write_unlock_bh(&tbl->lock);
-	dev_put(parms->dev);
+	dev_put_track(parms->dev, &parms->dev_tracker);
 	call_rcu(&parms->rcu_head, neigh_rcu_free_parms);
 }
 EXPORT_SYMBOL(neigh_parms_release);
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 16/19] net: add net device refcount tracker to struct netdev_adjacent

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index bdfbfa4e291858f990f5e2c99c4ce0ee9ed687cd..337d927500af3c6332d7e2635449f9d4a9be007a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6534,6 +6534,7 @@ static __latent_entropy void net_rx_action(struct softirq_action *h)
 
 struct netdev_adjacent {
 	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 
 	/* upper master flag, there can only be one master device per list */
 	bool master;
@@ -7298,7 +7299,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 	adj->ref_nr = 1;
 	adj->private = private;
 	adj->ignore = false;
-	dev_hold(adj_dev);
+	dev_hold_track(adj_dev, &adj->dev_tracker, GFP_KERNEL);
 
 	pr_debug("Insert adjacency: dev %s adj_dev %s adj->ref_nr %d; dev_hold on %s\n",
 		 dev->name, adj_dev->name, adj->ref_nr, adj_dev->name);
@@ -7327,8 +7328,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 	if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list))
 		netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list);
 free_adj:
+	dev_put_track(adj_dev, &adj->dev_tracker);
 	kfree(adj);
-	dev_put(adj_dev);
 
 	return ret;
 }
@@ -7369,7 +7370,7 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
 	list_del_rcu(&adj->list);
 	pr_debug("adjacency: dev_put for %s, because link removed from %s to %s\n",
 		 adj_dev->name, dev->name, adj_dev->name);
-	dev_put(adj_dev);
+	dev_put_track(adj_dev, &adj->dev_tracker);
 	kfree_rcu(adj, rcu);
 }
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 17/19] ipv6: add net device refcount tracker to struct inet6_dev

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/if_inet6.h   | 1 +
 net/ipv6/addrconf.c      | 4 ++--
 net/ipv6/addrconf_core.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 653e7d0f65cb7a5e7458daf860215d1873c532e7..f026cf08a8e86c54ea5d9f1abddd5f0e3caf402b 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -160,6 +160,7 @@ struct ipv6_devstat {
 
 struct inet6_dev {
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
 
 	struct list_head	addr_list;
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3445f8017430f145496bad78afd6378bf5cb1c02..3eee17790a82fe6c528db4e821b11444cfa26866 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -405,13 +405,13 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	if (ndev->cnf.forwarding)
 		dev_disable_lro(dev);
 	/* We refer to the device */
-	dev_hold(dev);
+	dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
 
 	if (snmp6_alloc_dev(ndev) < 0) {
 		netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
 			   __func__);
 		neigh_parms_release(&nd_tbl, ndev->nd_parms);
-		dev_put(dev);
+		dev_put_track(dev, &ndev->dev_tracker);
 		kfree(ndev);
 		return ERR_PTR(err);
 	}
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index 1d4054bb345b72204179c17b4ebc69e11e3faf53..881d1477d24ad5af79fd744bee1e0792fcfa483d 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -263,7 +263,7 @@ void in6_dev_finish_destroy(struct inet6_dev *idev)
 #ifdef NET_REFCNT_DEBUG
 	pr_debug("%s: %s\n", __func__, dev ? dev->name : "NIL");
 #endif
-	dev_put(dev);
+	dev_put_track(dev, &idev->dev_tracker);
 	if (!idev->dead) {
 		pr_warn("Freeing alive inet6 device %p\n", idev);
 		return;
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 18/19] ipv4: add net device refcount tracker to struct in_device

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/inetdevice.h | 2 ++
 net/ipv4/devinet.c         | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 518b484a7f07ebfa75cb8e4b41d78c15157f9aac..674aeead626089c2740ef844a7c8c8b799ee79dd 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -24,6 +24,8 @@ struct ipv4_devconf {
 
 struct in_device {
 	struct net_device	*dev;
+	netdevice_tracker	dev_tracker;
+
 	refcount_t		refcnt;
 	int			dead;
 	struct in_ifaddr	__rcu *ifa_list;/* IP ifaddr chain		*/
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 323e622ff9b745350a0ce63a238774281ab326e4..fba2bffd65f7967f390dcaf5183994af1ae5493b 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -243,7 +243,7 @@ void in_dev_finish_destroy(struct in_device *idev)
 #ifdef NET_REFCNT_DEBUG
 	pr_debug("%s: %p=%s\n", __func__, idev, dev ? dev->name : "NIL");
 #endif
-	dev_put(dev);
+	dev_put_track(dev, &idev->dev_tracker);
 	if (!idev->dead)
 		pr_err("Freeing alive in_device %p\n", idev);
 	else
@@ -271,7 +271,7 @@ static struct in_device *inetdev_init(struct net_device *dev)
 	if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
 		dev_disable_lro(dev);
 	/* Reference in_dev->dev */
-	dev_hold(dev);
+	dev_hold_track(dev, &in_dev->dev_tracker, GFP_KERNEL);
 	/* Account for reference dev->ip_ptr (below) */
 	refcount_set(&in_dev->refcnt, 1);
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH net-next 19/19] net/sched: add net device refcount tracker to struct Qdisc

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/sch_generic.h | 2 +-
 net/sched/sch_generic.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 22179b2fda72a0a1c2fddc07eeda40716a443a46..dbf202bb1a0edad582dbaefe72aac1632e219fbd 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -125,7 +125,7 @@ struct Qdisc {
 	spinlock_t		seqlock;
 
 	struct rcu_head		rcu;
-
+	netdevice_tracker	dev_tracker;
 	/* private data */
 	long privdata[] ____cacheline_aligned;
 };
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index d33804d41c5c5a9047c808fd37ba65ae8875fc79..8c8fbf2e385679e46a1b7af47eeac059fb8468cc 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -973,7 +973,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 	sch->enqueue = ops->enqueue;
 	sch->dequeue = ops->dequeue;
 	sch->dev_queue = dev_queue;
-	dev_hold(dev);
+	dev_hold_track(dev, &sch->dev_tracker, GFP_KERNEL);
 	refcount_set(&sch->refcnt, 1);
 
 	return sch;
@@ -1073,7 +1073,7 @@ static void qdisc_destroy(struct Qdisc *qdisc)
 		ops->destroy(qdisc);
 
 	module_put(ops->owner);
-	dev_put(qdisc_dev(qdisc));
+	dev_put_track(qdisc_dev(qdisc), &qdisc->dev_tracker);
 
 	trace_qdisc_destroy(qdisc);
 
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[Eric Dumazet]

On Wed, Dec 1, 2021 at 7:21 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> This module uses reference tracker, forcing two issues.
>
> 1) Double free of a tracker
>
> 2) leak of two trackers, one being allocated from softirq context.
>
> "modprobe test_ref_tracker" would emit the following traces.
> (Use scripts/decode_stacktrace.sh if necessary)
>
> [

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  lib/Kconfig.debug      |  10 ++++
>  lib/Makefile           |   2 +-
>  lib/test_ref_tracker.c | 116 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 127 insertions(+), 1 deletion(-)
>  create mode 100644 lib/test_ref_tracker.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5c12bde10996cf97b5f075d318089b1be73f71d7..d005f555872147e15d3e0a65d2a03e1a5c44f5f0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2106,6 +2106,16 @@ config BACKTRACE_SELF_TEST
>
>           Say N if you are unsure.
>
> +config TEST_REF_TRACKER
> +       tristate "Self test for reference tracker"
> +       depends on DEBUG_KERNEL
> +       select REF_TRACKER
> +       help
> +         This option provides a kernel module performing tests
> +         using reference tracker infrastructure.
> +
> +         Say N if you are unsure.
> +
>  config RBTREE_TEST
>         tristate "Red-Black tree test"
>         depends on DEBUG_KERNEL
> diff --git a/lib/Makefile b/lib/Makefile
> index c1fd9243ddb9cc1ac5252d7eb8009f9290782c4a..b213a7bbf3fda2eb9f234fb7473b8f1b617bed6b 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -101,7 +101,7 @@ obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
>  obj-$(CONFIG_TEST_HMM) += test_hmm.o
>  obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
>  obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
> -
> +obj-$(CONFIG_TEST_REF_TRACKER) += test_ref_tracker.o
>  #
>  # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
>  # off the generation of FPU/SSE* instructions for kernel proper but FPU_FLAGS
> diff --git a/lib/test_ref_tracker.c b/lib/test_ref_tracker.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..73bf9255e03790fa50491fe8e5cd411d54827c45
> --- /dev/null
> +++ b/lib/test_ref_tracker.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Referrence tracker self test.
> + *
> + * Copyright (c) 2021 Eric Dumazet <edumazet@google.com>
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/ref_tracker.h>
> +#include <linux/slab.h>
> +#include <linux/timer.h>
> +
> +static struct ref_tracker_dir ref_dir;
> +static struct ref_tracker *tracker[20];
> +
> +#define TRT_ALLOC(X) static noinline void                                      \
> +       alloctest_ref_tracker_alloc##X(struct ref_tracker_dir *dir,     \
> +                                   struct ref_tracker **trackerp)      \
> +       {                                                               \
> +               ref_tracker_alloc(dir, trackerp, GFP_KERNEL);           \
> +       }
> +
> +TRT_ALLOC(0)
> +TRT_ALLOC(1)
> +TRT_ALLOC(2)
> +TRT_ALLOC(3)
> +TRT_ALLOC(4)
> +TRT_ALLOC(5)
> +TRT_ALLOC(6)
> +TRT_ALLOC(7)
> +TRT_ALLOC(8)
> +TRT_ALLOC(9)
> +TRT_ALLOC(10)
> +TRT_ALLOC(11)
> +TRT_ALLOC(12)
> +TRT_ALLOC(13)
> +TRT_ALLOC(14)
> +TRT_ALLOC(15)
> +TRT_ALLOC(16)
> +TRT_ALLOC(17)
> +TRT_ALLOC(18)
> +TRT_ALLOC(19)
> +
> +#undef TRT_ALLOC
> +
> +static noinline void
> +alloctest_ref_tracker_free(struct ref_tracker_dir *dir,
> +                          struct ref_tracker **trackerp)
> +{
> +       ref_tracker_free(dir, trackerp);
> +}
> +
> +
> +static struct timer_list test_ref_tracker_timer;
> +static atomic_t test_ref_timer_done = ATOMIC_INIT(0);
> +
> +static void test_ref_tracker_timer_func(struct timer_list *t)
> +{
> +       alloctest_ref_tracker_alloc0(&ref_dir, &tracker[0]);

This will be changed to use GFP_ATOMIC instead of GFP_KERNEL

          ref_tracker_alloc(&ref_dir, ..., GFP_ATOMIC);

> +       atomic_set(&test_ref_timer_done, 1);
> +}
> +
>

Re: [PATCH net-next 08/19] drop_monitor: add net device refcount tracker

[kernel test robot]

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-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20211202/202112021345.IlporM5t-lkp@intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/6b336d0b301ebb1097132101a9e3bd01f71c40d4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 6b336d0b301ebb1097132101a9e3bd01f71c40d4
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash net/core/

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/drop_monitor.c: In function 'net_dm_hw_metadata_free':
>> net/core/drop_monitor.c:869:47: warning: passing argument 2 of 'dev_put_track' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     869 |         dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
         |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from net/core/drop_monitor.c:10:
   include/linux/netdevice.h:3863:53: note: expected 'struct ref_tracker **' but argument is of type 'struct ref_tracker * const*'
    3863 |                                  netdevice_tracker *tracker)
         |                                  ~~~~~~~~~~~~~~~~~~~^~~~~~~


vim +869 net/core/drop_monitor.c

   865	
   866	static void
   867	net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)
   868	{
 > 869		dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
   870		kfree(hw_metadata->fa_cookie);
   871		kfree(hw_metadata->trap_name);
   872		kfree(hw_metadata->trap_group_name);
   873		kfree(hw_metadata);
   874	}
   875	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH net-next 08/19] drop_monitor: add net device refcount tracker

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

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-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20211202/202112021345.IlporM5t-lkp(a)intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/6b336d0b301ebb1097132101a9e3bd01f71c40d4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 6b336d0b301ebb1097132101a9e3bd01f71c40d4
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash net/core/

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/drop_monitor.c: In function 'net_dm_hw_metadata_free':
>> net/core/drop_monitor.c:869:47: warning: passing argument 2 of 'dev_put_track' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     869 |         dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
         |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from net/core/drop_monitor.c:10:
   include/linux/netdevice.h:3863:53: note: expected 'struct ref_tracker **' but argument is of type 'struct ref_tracker * const*'
    3863 |                                  netdevice_tracker *tracker)
         |                                  ~~~~~~~~~~~~~~~~~~~^~~~~~~


vim +869 net/core/drop_monitor.c

   865	
   866	static void
   867	net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)
   868	{
 > 869		dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
   870		kfree(hw_metadata->fa_cookie);
   871		kfree(hw_metadata->trap_name);
   872		kfree(hw_metadata->trap_group_name);
   873		kfree(hw_metadata);
   874	}
   875	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH net-next 08/19] drop_monitor: add net device refcount tracker

[Eric Dumazet]

On Wed, Dec 1, 2021 at 9:41 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-preliminary-netdev-refcount-tracking/20211202-112353
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20211202/202112021345.IlporM5t-lkp@intel.com/config)
> compiler: nds32le-linux-gcc (GCC) 11.2.0
> 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
>         # https://github.com/0day-ci/linux/commit/6b336d0b301ebb1097132101a9e3bd01f71c40d4
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
>         git checkout 6b336d0b301ebb1097132101a9e3bd01f71c40d4
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash net/core/
>
> 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/drop_monitor.c: In function 'net_dm_hw_metadata_free':
> >> net/core/drop_monitor.c:869:47: warning: passing argument 2 of 'dev_put_track' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      869 |         dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
>          |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
>    In file included from net/core/drop_monitor.c:10:
>    include/linux/netdevice.h:3863:53: note: expected 'struct ref_tracker **' but argument is of type 'struct ref_tracker * const*'
>     3863 |                                  netdevice_tracker *tracker)
>          |                                  ~~~~~~~~~~~~~~~~~~~^~~~~~~
>
>
> vim +869 net/core/drop_monitor.c
>
>    865
>    866  static void
>    867  net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)

Yep, I have removed this not really useful  const qualifier

net_dm_hw_metadata_free(struct devlink_trap_metadata *hw_metadata)
...


>    868  {
>  > 869          dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
>    870          kfree(hw_metadata->fa_cookie);
>    871          kfree(hw_metadata->trap_name);
>    872          kfree(hw_metadata->trap_group_name);
>    873          kfree(hw_metadata);
>    874  }
>    875
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH net-next 08/19] drop_monitor: add net device refcount tracker

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 3005 bytes --]

On Wed, Dec 1, 2021 at 9:41 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-preliminary-netdev-refcount-tracking/20211202-112353
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20211202/202112021345.IlporM5t-lkp(a)intel.com/config)
> compiler: nds32le-linux-gcc (GCC) 11.2.0
> 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
>         # https://github.com/0day-ci/linux/commit/6b336d0b301ebb1097132101a9e3bd01f71c40d4
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
>         git checkout 6b336d0b301ebb1097132101a9e3bd01f71c40d4
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash net/core/
>
> 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/drop_monitor.c: In function 'net_dm_hw_metadata_free':
> >> net/core/drop_monitor.c:869:47: warning: passing argument 2 of 'dev_put_track' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      869 |         dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
>          |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
>    In file included from net/core/drop_monitor.c:10:
>    include/linux/netdevice.h:3863:53: note: expected 'struct ref_tracker **' but argument is of type 'struct ref_tracker * const*'
>     3863 |                                  netdevice_tracker *tracker)
>          |                                  ~~~~~~~~~~~~~~~~~~~^~~~~~~
>
>
> vim +869 net/core/drop_monitor.c
>
>    865
>    866  static void
>    867  net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)

Yep, I have removed this not really useful  const qualifier

net_dm_hw_metadata_free(struct devlink_trap_metadata *hw_metadata)
...


>    868  {
>  > 869          dev_put_track(hw_metadata->input_dev, &hw_metadata->dev_tracker);
>    870          kfree(hw_metadata->fa_cookie);
>    871          kfree(hw_metadata->trap_name);
>    872          kfree(hw_metadata->trap_group_name);
>    873          kfree(hw_metadata);
>    874  }
>    875
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH net-next 01/19] lib: add reference counting tracking infrastructure

[Dmitry Vyukov]

On Thu, 2 Dec 2021 at 04:21, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> It can be hard to track where references are taken and released.
>
> In networking, we have annoying issues at device or netns dismantles,
> and we had various proposals to ease root causing them.
>
> This patch adds new infrastructure pairing refcount increases
> and decreases. This will self document code, because programmers
> will have to associate increments/decrements.
>
> This is controled by CONFIG_REF_TRACKER which can be selected
> by users of this feature.
>
> This adds both cpu and memory costs, and thus should probably be
> used with care.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>


Reviewed-by: Dmitry Vyukov <dvyukov@google.com>


> ---
>  include/linux/ref_tracker.h |  73 +++++++++++++++++++
>  lib/Kconfig                 |   4 ++
>  lib/Makefile                |   2 +
>  lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 219 insertions(+)
>  create mode 100644 include/linux/ref_tracker.h
>  create mode 100644 lib/ref_tracker.c
>
> diff --git a/include/linux/ref_tracker.h b/include/linux/ref_tracker.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..c11c9db5825cf933acf529c83db441a818135f29
> --- /dev/null
> +++ b/include/linux/ref_tracker.h
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#ifndef _LINUX_REF_TRACKER_H
> +#define _LINUX_REF_TRACKER_H
> +#include <linux/refcount.h>
> +#include <linux/types.h>
> +#include <linux/spinlock.h>
> +
> +struct ref_tracker;
> +
> +struct ref_tracker_dir {
> +#ifdef CONFIG_REF_TRACKER
> +       spinlock_t              lock;
> +       unsigned int            quarantine_avail;
> +       refcount_t              untracked;
> +       struct list_head        list; /* List of active trackers */
> +       struct list_head        quarantine; /* List of dead trackers */
> +#endif
> +};
> +
> +#ifdef CONFIG_REF_TRACKER
> +static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
> +                                       unsigned int quarantine_count)
> +{
> +       INIT_LIST_HEAD(&dir->list);
> +       INIT_LIST_HEAD(&dir->quarantine);
> +       spin_lock_init(&dir->lock);
> +       dir->quarantine_avail = quarantine_count;
> +       refcount_set(&dir->untracked, 1);
> +}
> +
> +void ref_tracker_dir_exit(struct ref_tracker_dir *dir);
> +
> +void ref_tracker_dir_print(struct ref_tracker_dir *dir,
> +                          unsigned int display_limit);
> +
> +int ref_tracker_alloc(struct ref_tracker_dir *dir,
> +                     struct ref_tracker **trackerp, gfp_t gfp);
> +
> +int ref_tracker_free(struct ref_tracker_dir *dir,
> +                    struct ref_tracker **trackerp);
> +
> +#else /* CONFIG_REF_TRACKER */
> +
> +static inline void ref_tracker_dir_init(struct ref_tracker_dir *dir,
> +                                       unsigned int quarantine_count)
> +{
> +}
> +
> +static inline void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
> +{
> +}
> +
> +static inline void ref_tracker_dir_print(struct ref_tracker_dir *dir,
> +                                        unsigned int display_limit)
> +{
> +}
> +
> +static inline int ref_tracker_alloc(struct ref_tracker_dir *dir,
> +                                   struct ref_tracker **trackerp,
> +                                   gfp_t gfp)
> +{
> +       return 0;
> +}
> +
> +static inline int ref_tracker_free(struct ref_tracker_dir *dir,
> +                                  struct ref_tracker **trackerp)
> +{
> +       return 0;
> +}
> +
> +#endif
> +
> +#endif /* _LINUX_REF_TRACKER_H */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 5e7165e6a346c9bec878b78c8c8c3d175fc98dfd..d01be8e9593992a7d94a46bd1716460bc33c3ae1 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -680,6 +680,10 @@ config STACK_HASH_ORDER
>          Select the hash size as a power of 2 for the stackdepot hash table.
>          Choose a lower value to reduce the memory impact.
>
> +config REF_TRACKER
> +       bool
> +       select STACKDEPOT
> +
>  config SBITMAP
>         bool
>
> diff --git a/lib/Makefile b/lib/Makefile
> index 364c23f1557816f73aebd8304c01224a4846ac6c..c1fd9243ddb9cc1ac5252d7eb8009f9290782c4a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -270,6 +270,8 @@ obj-$(CONFIG_STACKDEPOT) += stackdepot.o
>  KASAN_SANITIZE_stackdepot.o := n
>  KCOV_INSTRUMENT_stackdepot.o := n
>
> +obj-$(CONFIG_REF_TRACKER) += ref_tracker.o
> +
>  libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
>                fdt_empty_tree.o fdt_addresses.o
>  $(foreach file, $(libfdt_files), \
> diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..0ae2e66dcf0fdb976f4cb99e747c9448b37f22cc
> --- /dev/null
> +++ b/lib/ref_tracker.c
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <linux/export.h>
> +#include <linux/ref_tracker.h>
> +#include <linux/slab.h>
> +#include <linux/stacktrace.h>
> +#include <linux/stackdepot.h>
> +
> +#define REF_TRACKER_STACK_ENTRIES 16
> +
> +struct ref_tracker {
> +       struct list_head        head;   /* anchor into dir->list or dir->quarantine */
> +       bool                    dead;
> +       depot_stack_handle_t    alloc_stack_handle;
> +       depot_stack_handle_t    free_stack_handle;
> +};
> +
> +void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
> +{
> +       struct ref_tracker *tracker, *n;
> +       unsigned long flags;
> +       bool leak = false;
> +
> +       spin_lock_irqsave(&dir->lock, flags);
> +       list_for_each_entry_safe(tracker, n, &dir->quarantine, head) {
> +               list_del(&tracker->head);
> +               kfree(tracker);
> +               dir->quarantine_avail++;
> +       }
> +       list_for_each_entry_safe(tracker, n, &dir->list, head) {
> +               pr_err("leaked reference.\n");
> +               if (tracker->alloc_stack_handle)
> +                       stack_depot_print(tracker->alloc_stack_handle);
> +               leak = true;
> +               list_del(&tracker->head);
> +               kfree(tracker);
> +       }
> +       spin_unlock_irqrestore(&dir->lock, flags);
> +       WARN_ON_ONCE(leak);
> +       WARN_ON_ONCE(refcount_read(&dir->untracked) != 1);
> +}
> +EXPORT_SYMBOL(ref_tracker_dir_exit);
> +
> +void ref_tracker_dir_print(struct ref_tracker_dir *dir,
> +                          unsigned int display_limit)
> +{
> +       struct ref_tracker *tracker;
> +       unsigned long flags;
> +       unsigned int i = 0;
> +
> +       spin_lock_irqsave(&dir->lock, flags);
> +       list_for_each_entry(tracker, &dir->list, head) {
> +               if (i < display_limit) {
> +                       pr_err("leaked reference.\n");
> +                       if (tracker->alloc_stack_handle)
> +                               stack_depot_print(tracker->alloc_stack_handle);
> +                       i++;
> +               } else {
> +                       break;
> +               }
> +       }
> +       spin_unlock_irqrestore(&dir->lock, flags);
> +}
> +EXPORT_SYMBOL(ref_tracker_dir_print);
> +
> +int ref_tracker_alloc(struct ref_tracker_dir *dir,
> +                     struct ref_tracker **trackerp,
> +                     gfp_t gfp)
> +{
> +       unsigned long entries[REF_TRACKER_STACK_ENTRIES];
> +       struct ref_tracker *tracker;
> +       unsigned int nr_entries;
> +       unsigned long flags;
> +
> +       *trackerp = tracker = kzalloc(sizeof(*tracker), gfp | __GFP_NOFAIL);
> +       if (unlikely(!tracker)) {
> +               pr_err_once("memory allocation failure, unreliable refcount tracker.\n");
> +               refcount_inc(&dir->untracked);
> +               return -ENOMEM;
> +       }
> +       nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
> +       nr_entries = filter_irq_stacks(entries, nr_entries);
> +       tracker->alloc_stack_handle = stack_depot_save(entries, nr_entries, gfp);
> +
> +       spin_lock_irqsave(&dir->lock, flags);
> +       list_add(&tracker->head, &dir->list);
> +       spin_unlock_irqrestore(&dir->lock, flags);
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(ref_tracker_alloc);
> +
> +int ref_tracker_free(struct ref_tracker_dir *dir,
> +                    struct ref_tracker **trackerp)
> +{
> +       unsigned long entries[REF_TRACKER_STACK_ENTRIES];
> +       struct ref_tracker *tracker = *trackerp;
> +       depot_stack_handle_t stack_handle;
> +       unsigned int nr_entries;
> +       unsigned long flags;
> +
> +       if (!tracker) {
> +               refcount_dec(&dir->untracked);

Nice approach.

> +               return -EEXIST;
> +       }
> +       nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
> +       nr_entries = filter_irq_stacks(entries, nr_entries);

Marco sent a patch to do this as part of stack_depot_save() as we spoke:
https://lore.kernel.org/lkml/20211130095727.2378739-1-elver@google.com/
I am not sure in what order these patches will reach trees, but
ultimately filter_irq_stacks() won't be needed here anymore...

> +       stack_handle = stack_depot_save(entries, nr_entries, GFP_ATOMIC);
> +
> +       spin_lock_irqsave(&dir->lock, flags);
> +       if (tracker->dead) {
> +               pr_err("reference already released.\n");
> +               if (tracker->alloc_stack_handle) {
> +                       pr_err("allocated in:\n");
> +                       stack_depot_print(tracker->alloc_stack_handle);
> +               }
> +               if (tracker->free_stack_handle) {
> +                       pr_err("freed in:\n");
> +                       stack_depot_print(tracker->free_stack_handle);
> +               }
> +               spin_unlock_irqrestore(&dir->lock, flags);
> +               WARN_ON_ONCE(1);
> +               return -EINVAL;
> +       }
> +       tracker->dead = true;
> +
> +       tracker->free_stack_handle = stack_handle;
> +
> +       list_move_tail(&tracker->head, &dir->quarantine);
> +       if (!dir->quarantine_avail) {
> +               tracker = list_first_entry(&dir->quarantine, struct ref_tracker, head);
> +               list_del(&tracker->head);
> +       } else {
> +               dir->quarantine_avail--;
> +               tracker = NULL;
> +       }
> +       spin_unlock_irqrestore(&dir->lock, flags);
> +
> +       kfree(tracker);
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(ref_tracker_free);
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[kernel test robot]

Hi Eric,

I love your patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: sparc-buildonly-randconfig-r005-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021600.8HBrwOX4-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash arch/sparc/

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

All errors (new ones prefixed by >>):

   In file included from arch/sparc/kernel/stacktrace.c:11:
   arch/sparc/kernel/kstack.h: In function 'kstack_valid':
>> arch/sparc/kernel/kstack.h:23:13: error: 'hardirq_stack' undeclared (first use in this function)
      23 |         if (hardirq_stack[tp->cpu]) {
         |             ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h:23:13: note: each undeclared identifier is reported only once for each function it appears in
>> arch/sparc/kernel/kstack.h:28:40: error: 'softirq_stack' undeclared (first use in this function)
      28 |                 base = (unsigned long) softirq_stack[tp->cpu];
         |                                        ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h: In function 'kstack_is_trap_frame':
   arch/sparc/kernel/kstack.h:46:13: error: 'hardirq_stack' undeclared (first use in this function)
      46 |         if (hardirq_stack[tp->cpu]) {
         |             ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h:51:40: error: 'softirq_stack' undeclared (first use in this function)
      51 |                 base = (unsigned long) softirq_stack[tp->cpu];
         |                                        ^~~~~~~~~~~~~
>> arch/sparc/kernel/kstack.h:59:18: error: 'struct pt_regs' has no member named 'magic'
      59 |         if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
         |                  ^~
>> arch/sparc/kernel/kstack.h:59:39: error: 'PT_REGS_MAGIC' undeclared (first use in this function); did you mean 'PT_V9_MAGIC'?
      59 |         if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
         |                                       ^~~~~~~~~~~~~
         |                                       PT_V9_MAGIC
   arch/sparc/kernel/kstack.h: In function 'set_hardirq_stack':
   arch/sparc/kernel/kstack.h:67:30: error: 'hardirq_stack' undeclared (first use in this function); did you mean 'set_hardirq_stack'?
      67 |         void *orig_sp, *sp = hardirq_stack[smp_processor_id()];
         |                              ^~~~~~~~~~~~~
         |                              set_hardirq_stack
   arch/sparc/kernel/stacktrace.c: In function '__save_stack_trace':
>> arch/sparc/kernel/stacktrace.c:46:35: error: 'struct pt_regs' has no member named 'tstate'
      46 |                         if (!(regs->tstate & TSTATE_PRIV))
         |                                   ^~
>> arch/sparc/kernel/stacktrace.c:46:46: error: 'TSTATE_PRIV' undeclared (first use in this function)
      46 |                         if (!(regs->tstate & TSTATE_PRIV))
         |                                              ^~~~~~~~~~~
>> arch/sparc/kernel/stacktrace.c:48:36: error: 'struct pt_regs' has no member named 'tpc'; did you mean 'pc'?
      48 |                         pc = regs->tpc;
         |                                    ^~~
         |                                    pc

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for STACKTRACE
   Depends on STACKTRACE_SUPPORT
   Selected by
   - STACKDEPOT


vim +/hardirq_stack +23 arch/sparc/kernel/kstack.h

4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12   9  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  10  /* SP must be STACK_BIAS adjusted already.  */
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  11  static inline bool kstack_valid(struct thread_info *tp, unsigned long sp)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  12  {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  13  	unsigned long base = (unsigned long) tp;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  14  
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  15  	/* Stack pointer must be 16-byte aligned.  */
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  16  	if (sp & (16UL - 1))
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  17  		return false;
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  18  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  19  	if (sp >= (base + sizeof(struct thread_info)) &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  20  	    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  21  		return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  22  
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13 @23  	if (hardirq_stack[tp->cpu]) {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  24  		base = (unsigned long) hardirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  25  		if (sp >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  26  		    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  27  			return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12 @28  		base = (unsigned long) softirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  29  		if (sp >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  30  		    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  31  			return true;
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  32  	}
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  33  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  34  }
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  35  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  36  /* Does "regs" point to a valid pt_regs trap frame?  */
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  37  static inline bool kstack_is_trap_frame(struct thread_info *tp, struct pt_regs *regs)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  38  {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  39  	unsigned long base = (unsigned long) tp;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  40  	unsigned long addr = (unsigned long) regs;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  41  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  42  	if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  43  	    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  44  		goto check_magic;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  45  
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  46  	if (hardirq_stack[tp->cpu]) {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  47  		base = (unsigned long) hardirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  48  		if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  49  		    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  50  			goto check_magic;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  51  		base = (unsigned long) softirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  52  		if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  53  		    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  54  			goto check_magic;
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  55  	}
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  56  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  57  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  58  check_magic:
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12 @59  	if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  60  		return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  61  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  62  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10263 bytes --]

Hi Eric,

I love your patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: sparc-buildonly-randconfig-r005-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021600.8HBrwOX4-lkp(a)intel.com/config)
compiler: sparc-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash arch/sparc/

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

All errors (new ones prefixed by >>):

   In file included from arch/sparc/kernel/stacktrace.c:11:
   arch/sparc/kernel/kstack.h: In function 'kstack_valid':
>> arch/sparc/kernel/kstack.h:23:13: error: 'hardirq_stack' undeclared (first use in this function)
      23 |         if (hardirq_stack[tp->cpu]) {
         |             ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h:23:13: note: each undeclared identifier is reported only once for each function it appears in
>> arch/sparc/kernel/kstack.h:28:40: error: 'softirq_stack' undeclared (first use in this function)
      28 |                 base = (unsigned long) softirq_stack[tp->cpu];
         |                                        ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h: In function 'kstack_is_trap_frame':
   arch/sparc/kernel/kstack.h:46:13: error: 'hardirq_stack' undeclared (first use in this function)
      46 |         if (hardirq_stack[tp->cpu]) {
         |             ^~~~~~~~~~~~~
   arch/sparc/kernel/kstack.h:51:40: error: 'softirq_stack' undeclared (first use in this function)
      51 |                 base = (unsigned long) softirq_stack[tp->cpu];
         |                                        ^~~~~~~~~~~~~
>> arch/sparc/kernel/kstack.h:59:18: error: 'struct pt_regs' has no member named 'magic'
      59 |         if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
         |                  ^~
>> arch/sparc/kernel/kstack.h:59:39: error: 'PT_REGS_MAGIC' undeclared (first use in this function); did you mean 'PT_V9_MAGIC'?
      59 |         if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
         |                                       ^~~~~~~~~~~~~
         |                                       PT_V9_MAGIC
   arch/sparc/kernel/kstack.h: In function 'set_hardirq_stack':
   arch/sparc/kernel/kstack.h:67:30: error: 'hardirq_stack' undeclared (first use in this function); did you mean 'set_hardirq_stack'?
      67 |         void *orig_sp, *sp = hardirq_stack[smp_processor_id()];
         |                              ^~~~~~~~~~~~~
         |                              set_hardirq_stack
   arch/sparc/kernel/stacktrace.c: In function '__save_stack_trace':
>> arch/sparc/kernel/stacktrace.c:46:35: error: 'struct pt_regs' has no member named 'tstate'
      46 |                         if (!(regs->tstate & TSTATE_PRIV))
         |                                   ^~
>> arch/sparc/kernel/stacktrace.c:46:46: error: 'TSTATE_PRIV' undeclared (first use in this function)
      46 |                         if (!(regs->tstate & TSTATE_PRIV))
         |                                              ^~~~~~~~~~~
>> arch/sparc/kernel/stacktrace.c:48:36: error: 'struct pt_regs' has no member named 'tpc'; did you mean 'pc'?
      48 |                         pc = regs->tpc;
         |                                    ^~~
         |                                    pc

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for STACKTRACE
   Depends on STACKTRACE_SUPPORT
   Selected by
   - STACKDEPOT


vim +/hardirq_stack +23 arch/sparc/kernel/kstack.h

4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12   9  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  10  /* SP must be STACK_BIAS adjusted already.  */
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  11  static inline bool kstack_valid(struct thread_info *tp, unsigned long sp)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  12  {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  13  	unsigned long base = (unsigned long) tp;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  14  
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  15  	/* Stack pointer must be 16-byte aligned.  */
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  16  	if (sp & (16UL - 1))
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  17  		return false;
232486e1e9f348 arch/sparc/kernel/kstack.h   David S. Miller 2010-02-12  18  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  19  	if (sp >= (base + sizeof(struct thread_info)) &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  20  	    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  21  		return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  22  
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13 @23  	if (hardirq_stack[tp->cpu]) {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  24  		base = (unsigned long) hardirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  25  		if (sp >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  26  		    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  27  			return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12 @28  		base = (unsigned long) softirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  29  		if (sp >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  30  		    sp <= (base + THREAD_SIZE - sizeof(struct sparc_stackf)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  31  			return true;
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  32  	}
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  33  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  34  }
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  35  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  36  /* Does "regs" point to a valid pt_regs trap frame?  */
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  37  static inline bool kstack_is_trap_frame(struct thread_info *tp, struct pt_regs *regs)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  38  {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  39  	unsigned long base = (unsigned long) tp;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  40  	unsigned long addr = (unsigned long) regs;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  41  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  42  	if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  43  	    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  44  		goto check_magic;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  45  
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  46  	if (hardirq_stack[tp->cpu]) {
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  47  		base = (unsigned long) hardirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  48  		if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  49  		    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  50  			goto check_magic;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  51  		base = (unsigned long) softirq_stack[tp->cpu];
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  52  		if (addr >= base &&
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  53  		    addr <= (base + THREAD_SIZE - sizeof(*regs)))
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  54  			goto check_magic;
6f63e781eaf6a7 arch/sparc64/kernel/kstack.h David S. Miller 2008-08-13  55  	}
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  56  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  57  
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  58  check_magic:
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12 @59  	if ((regs->magic & ~0x1ff) == PT_REGS_MAGIC)
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  60  		return true;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  61  	return false;
4f70f7a91bffdc arch/sparc64/kernel/kstack.h David S. Miller 2008-08-12  62  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[dust.li]

On Wed, Dec 01, 2021 at 07:21:20PM -0800, Eric Dumazet wrote:
>From: Eric Dumazet <edumazet@google.com>
>
>Two first patches add a generic infrastructure, that will be used
>to get tracking of refcount increments/decrements.
>
>The general idea is to be able to precisely pair each decrement with
>a corresponding prior increment. Both share a cookie, basically
>a pointer to private data storing stack traces.
>
>Then a series of 17 patches converts some dev_hold()/dev_put()
>pairs to new hepers : dev_hold_track() and dev_put_track().
>
>Hopefully this will be used by developpers and syzbot to
>root cause bugs that cause netdevice dismantles freezes.

Hi Eric:

I really like the idea of adding debug informantion for debugging
refcnt problems, we have met some bugs of leaking netdev refcnt in
the past and debugging them is really HARD !!

Recently, when investigating a sk_refcnt double free bug in SMC,
I added some debug code a bit similar like this. I'm curious have
you considered expanding the ref tracker infrastructure into other
places like sock_hold/put() or even some hot path ?

AFAIU, ref tracker add a tracker inside each object who want to
hold a refcnt, and stored the callstack into the tracker.
I have 2 questions here:
1. If we want to use this in the hot path, looks like the overhead
   is a bit heavy ?
2. Since we only store 1 callstack in 1 tracker, what if some object
   want to hold and put refcnt in different places ?

Thanks.

>
>With CONFIG_PCPU_DEV_REFCNT=n option, we were able to detect
>some class of bugs, but too late (when too many dev_put()
>were happening).
>
>Eric Dumazet (19):
>  lib: add reference counting tracking infrastructure
>  lib: add tests for reference tracker
>  net: add dev_hold_track() and dev_put_track() helpers
>  net: add net device refcount tracker to struct netdev_rx_queue
>  net: add net device refcount tracker to struct netdev_queue
>  net: add net device refcount tracker to ethtool_phys_id()
>  net: add net device refcount tracker to dev_ifsioc()
>  drop_monitor: add net device refcount tracker
>  net: dst: add net device refcount tracking to dst_entry
>  ipv6: add net device refcount tracker to rt6_probe_deferred()
>  sit: add net device refcount tracking to ip_tunnel
>  ipv6: add net device refcount tracker to struct ip6_tnl
>  net: add net device refcount tracker to struct neighbour
>  net: add net device refcount tracker to struct pneigh_entry
>  net: add net device refcount tracker to struct neigh_parms
>  net: add net device refcount tracker to struct netdev_adjacent
>  ipv6: add net device refcount tracker to struct inet6_dev
>  ipv4: add net device refcount tracker to struct in_device
>  net/sched: add net device refcount tracker to struct Qdisc
>
> include/linux/inetdevice.h  |   2 +
> include/linux/netdevice.h   |  53 ++++++++++++++
> include/linux/ref_tracker.h |  73 +++++++++++++++++++
> include/net/devlink.h       |   3 +
> include/net/dst.h           |   1 +
> include/net/if_inet6.h      |   1 +
> include/net/ip6_tunnel.h    |   1 +
> include/net/ip_tunnels.h    |   3 +
> include/net/neighbour.h     |   3 +
> include/net/sch_generic.h   |   2 +-
> lib/Kconfig                 |   4 ++
> lib/Kconfig.debug           |  10 +++
> lib/Makefile                |   4 +-
> lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
> lib/test_ref_tracker.c      | 116 ++++++++++++++++++++++++++++++
> net/Kconfig                 |   8 +++
> net/core/dev.c              |  10 ++-
> net/core/dev_ioctl.c        |   5 +-
> net/core/drop_monitor.c     |   4 +-
> net/core/dst.c              |   8 +--
> net/core/neighbour.c        |  18 ++---
> net/core/net-sysfs.c        |   8 +--
> net/ethtool/ioctl.c         |   5 +-
> net/ipv4/devinet.c          |   4 +-
> net/ipv4/route.c            |   7 +-
> net/ipv6/addrconf.c         |   4 +-
> net/ipv6/addrconf_core.c    |   2 +-
> net/ipv6/ip6_gre.c          |   8 +--
> net/ipv6/ip6_tunnel.c       |   4 +-
> net/ipv6/ip6_vti.c          |   4 +-
> net/ipv6/route.c            |  10 +--
> net/ipv6/sit.c              |   4 +-
> net/sched/sch_generic.c     |   4 +-
> 33 files changed, 481 insertions(+), 52 deletions(-)
> create mode 100644 include/linux/ref_tracker.h
> create mode 100644 lib/ref_tracker.c
> create mode 100644 lib/test_ref_tracker.c
>
>-- 
>2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[Eric Dumazet]

On Thu, Dec 2, 2021 at 4:49 AM dust.li <dust.li@linux.alibaba.com> wrote:
>
> On Wed, Dec 01, 2021 at 07:21:20PM -0800, Eric Dumazet wrote:
> >From: Eric Dumazet <edumazet@google.com>
> >
> >Two first patches add a generic infrastructure, that will be used
> >to get tracking of refcount increments/decrements.
> >
> >The general idea is to be able to precisely pair each decrement with
> >a corresponding prior increment. Both share a cookie, basically
> >a pointer to private data storing stack traces.
> >
> >Then a series of 17 patches converts some dev_hold()/dev_put()
> >pairs to new hepers : dev_hold_track() and dev_put_track().
> >
> >Hopefully this will be used by developpers and syzbot to
> >root cause bugs that cause netdevice dismantles freezes.
>
> Hi Eric:
>
> I really like the idea of adding debug informantion for debugging
> refcnt problems, we have met some bugs of leaking netdev refcnt in
> the past and debugging them is really HARD !!
>
> Recently, when investigating a sk_refcnt double free bug in SMC,
> I added some debug code a bit similar like this. I'm curious have
> you considered expanding the ref tracker infrastructure into other
> places like sock_hold/put() or even some hot path ?

Sure, this should be absolutely doable with this generic infra.

>
> AFAIU, ref tracker add a tracker inside each object who want to
> hold a refcnt, and stored the callstack into the tracker.
> I have 2 questions here:
> 1. If we want to use this in the hot path, looks like the overhead
>    is a bit heavy ?

Much less expensive than lockdep (we use one spinlock per struct
ref_tracker_dir), so I think this is something doable.

> 2. Since we only store 1 callstack in 1 tracker, what if some object
>    want to hold and put refcnt in different places ?


You can use a tracker on the stack (patch 6/19 net: add net device
refcount tracker to ethtool_phys_id())

For generic uses of dev_hold(), like in dev_get_by_index(), we will
probably have new helpers
so that callers can provide where the tracker is put.

Or simply use this sequence to convert a generic/untracked reference
to a tracked one.

dev = dev_get_by_index(),;
...

p->dev = dev;
dev_hold_track(dev, &p->dev_tracker, GFP...)
dev_put(dev);


>
> Thanks.
>
> >
> >With CONFIG_PCPU_DEV_REFCNT=n option, we were able to detect
> >some class of bugs, but too late (when too many dev_put()
> >were happening).
> >
> >Eric Dumazet (19):
> >  lib: add reference counting tracking infrastructure
> >  lib: add tests for reference tracker
> >  net: add dev_hold_track() and dev_put_track() helpers
> >  net: add net device refcount tracker to struct netdev_rx_queue
> >  net: add net device refcount tracker to struct netdev_queue
> >  net: add net device refcount tracker to ethtool_phys_id()
> >  net: add net device refcount tracker to dev_ifsioc()
> >  drop_monitor: add net device refcount tracker
> >  net: dst: add net device refcount tracking to dst_entry
> >  ipv6: add net device refcount tracker to rt6_probe_deferred()
> >  sit: add net device refcount tracking to ip_tunnel
> >  ipv6: add net device refcount tracker to struct ip6_tnl
> >  net: add net device refcount tracker to struct neighbour
> >  net: add net device refcount tracker to struct pneigh_entry
> >  net: add net device refcount tracker to struct neigh_parms
> >  net: add net device refcount tracker to struct netdev_adjacent
> >  ipv6: add net device refcount tracker to struct inet6_dev
> >  ipv4: add net device refcount tracker to struct in_device
> >  net/sched: add net device refcount tracker to struct Qdisc
> >
> > include/linux/inetdevice.h  |   2 +
> > include/linux/netdevice.h   |  53 ++++++++++++++
> > include/linux/ref_tracker.h |  73 +++++++++++++++++++
> > include/net/devlink.h       |   3 +
> > include/net/dst.h           |   1 +
> > include/net/if_inet6.h      |   1 +
> > include/net/ip6_tunnel.h    |   1 +
> > include/net/ip_tunnels.h    |   3 +
> > include/net/neighbour.h     |   3 +
> > include/net/sch_generic.h   |   2 +-
> > lib/Kconfig                 |   4 ++
> > lib/Kconfig.debug           |  10 +++
> > lib/Makefile                |   4 +-
> > lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
> > lib/test_ref_tracker.c      | 116 ++++++++++++++++++++++++++++++
> > net/Kconfig                 |   8 +++
> > net/core/dev.c              |  10 ++-
> > net/core/dev_ioctl.c        |   5 +-
> > net/core/drop_monitor.c     |   4 +-
> > net/core/dst.c              |   8 +--
> > net/core/neighbour.c        |  18 ++---
> > net/core/net-sysfs.c        |   8 +--
> > net/ethtool/ioctl.c         |   5 +-
> > net/ipv4/devinet.c          |   4 +-
> > net/ipv4/route.c            |   7 +-
> > net/ipv6/addrconf.c         |   4 +-
> > net/ipv6/addrconf_core.c    |   2 +-
> > net/ipv6/ip6_gre.c          |   8 +--
> > net/ipv6/ip6_tunnel.c       |   4 +-
> > net/ipv6/ip6_vti.c          |   4 +-
> > net/ipv6/route.c            |  10 +--
> > net/ipv6/sit.c              |   4 +-
> > net/sched/sch_generic.c     |   4 +-
> > 33 files changed, 481 insertions(+), 52 deletions(-)
> > create mode 100644 include/linux/ref_tracker.h
> > create mode 100644 lib/ref_tracker.c
> > create mode 100644 lib/test_ref_tracker.c
> >
> >--
> >2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH net-next 01/19] lib: add reference counting tracking infrastructure

[Eric Dumazet]

On Thu, Dec 2, 2021 at 12:13 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Thu, 2 Dec 2021 at 04:21, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > From: Eric Dumazet <edumazet@google.com>
> >
> > It can be hard to track where references are taken and released.
> >
> > In networking, we have annoying issues at device or netns dismantles,
> > and we had various proposals to ease root causing them.
> >
> > This patch adds new infrastructure pairing refcount increases
> > and decreases. This will self document code, because programmers
> > will have to associate increments/decrements.
> >
> > This is controled by CONFIG_REF_TRACKER which can be selected
> > by users of this feature.
> >
> > This adds both cpu and memory costs, and thus should probably be
> > used with care.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
>
> Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
>

Thanks !

> > +
> > +       if (!tracker) {
> > +               refcount_dec(&dir->untracked);
>
> Nice approach.

Yes, I found that a boolean was weak, we can do a full refcounted way,

Transition from 1 to 0 will generate a warning/stacktrace

>
> > +               return -EEXIST;
> > +       }
> > +       nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
> > +       nr_entries = filter_irq_stacks(entries, nr_entries);
>
> Marco sent a patch to do this as part of stack_depot_save() as we spoke:
> https://lore.kernel.org/lkml/20211130095727.2378739-1-elver@google.com/
> I am not sure in what order these patches will reach trees, but
> ultimately filter_irq_stacks() won't be needed here anymore...
>

Great, I will keep this in mind, thanks.

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[kernel test robot]

Hi Eric,

I love your patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
>> (.text+0x2e4): undefined reference to `save_stack_trace'
   (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for STACKTRACE
   Depends on STACKTRACE_SUPPORT
   Selected by
   - STACKDEPOT

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

Hi Eric,

I love your patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp(a)intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
        git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
>> (.text+0x2e4): undefined reference to `save_stack_trace'
   (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for STACKTRACE
   Depends on STACKTRACE_SUPPORT
   Selected by
   - STACKDEPOT

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[Eric Dumazet]

On Thu, Dec 2, 2021 at 11:25 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Eric,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on net-next/master]
>
> url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp@intel.com/config)
> compiler: nios2-linux-gcc (GCC) 11.2.0
> 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
>         # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
>         git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
> >> (.text+0x2e4): undefined reference to `save_stack_trace'
>    (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'
>
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for STACKTRACE
>    Depends on STACKTRACE_SUPPORT
>    Selected by
>    - STACKDEPOT
>

I am not sure I understand this.

Dmitry, do I need to add a depends on STACKTRACE_SUPPORT.

Thanks !

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On Thu, Dec 2, 2021 at 11:25 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Eric,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on net-next/master]
>
> url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp(a)intel.com/config)
> compiler: nios2-linux-gcc (GCC) 11.2.0
> 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
>         # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
>         git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
> >> (.text+0x2e4): undefined reference to `save_stack_trace'
>    (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'
>
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for STACKTRACE
>    Depends on STACKTRACE_SUPPORT
>    Selected by
>    - STACKDEPOT
>

I am not sure I understand this.

Dmitry, do I need to add a depends on STACKTRACE_SUPPORT.

Thanks !

Re: [PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[dust.li]

On Thu, Dec 02, 2021 at 07:13:12AM -0800, Eric Dumazet wrote:
>On Thu, Dec 2, 2021 at 4:49 AM dust.li <dust.li@linux.alibaba.com> wrote:
>>
>> On Wed, Dec 01, 2021 at 07:21:20PM -0800, Eric Dumazet wrote:
>> >From: Eric Dumazet <edumazet@google.com>
>> >
>> >Two first patches add a generic infrastructure, that will be used
>> >to get tracking of refcount increments/decrements.
>> >
>> >The general idea is to be able to precisely pair each decrement with
>> >a corresponding prior increment. Both share a cookie, basically
>> >a pointer to private data storing stack traces.
>> >
>> >Then a series of 17 patches converts some dev_hold()/dev_put()
>> >pairs to new hepers : dev_hold_track() and dev_put_track().
>> >
>> >Hopefully this will be used by developpers and syzbot to
>> >root cause bugs that cause netdevice dismantles freezes.
>>
>> Hi Eric:
>>
>> I really like the idea of adding debug informantion for debugging
>> refcnt problems, we have met some bugs of leaking netdev refcnt in
>> the past and debugging them is really HARD !!
>>
>> Recently, when investigating a sk_refcnt double free bug in SMC,
>> I added some debug code a bit similar like this. I'm curious have
>> you considered expanding the ref tracker infrastructure into other
>> places like sock_hold/put() or even some hot path ?
>
>Sure, this should be absolutely doable with this generic infra.
>
>>
>> AFAIU, ref tracker add a tracker inside each object who want to
>> hold a refcnt, and stored the callstack into the tracker.
>> I have 2 questions here:
>> 1. If we want to use this in the hot path, looks like the overhead
>>    is a bit heavy ?
>
>Much less expensive than lockdep (we use one spinlock per struct
>ref_tracker_dir), so I think this is something doable.

Yeah, I'm thinking enable this feature by default in our kernel
so I care about this won't bring regression.
I did a simple test in case this might be helpful to other.

Testing was triggered by test_ref_tracker.ko run on a Intel Xeon server.
Added the following debug code:

@@ -12,8 +13,14 @@ struct ref_tracker {
        bool                    dead;
        depot_stack_handle_t    alloc_stack_handle;
        depot_stack_handle_t    free_stack_handle;
+       unsigned long long      lat;
 };

+static void dump_tracker_latency(struct ref_tracker *tracker)
+{
+       printk("tracker %pK alloc cost(ns): %llu\n", tracker, tracker->lat);
+}
+
 void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
 {
        struct ref_tracker *tracker, *n;
@@ -23,6 +30,7 @@ void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
        spin_lock_irqsave(&dir->lock, flags);
        list_for_each_entry_safe(tracker, n, &dir->quarantine, head) {
                list_del(&tracker->head);
+               dump_tracker_latency(tracker);
                kfree(tracker);
                dir->quarantine_avail++;
        }
 ...
 }

@@ -70,7 +78,9 @@ int ref_tracker_alloc(struct ref_tracker_dir *dir,
        struct ref_tracker *tracker;
        unsigned int nr_entries;
        unsigned long flags;
+       unsigned long long t;

+       t = sched_clock();
        *trackerp = tracker = kzalloc(sizeof(*tracker), gfp | __GFP_NOFAIL);
        if (unlikely(!tracker)) {
                pr_err_once("memory allocation failure, unreliable refcount tracker.\n");
@@ -84,6 +94,7 @@ int ref_tracker_alloc(struct ref_tracker_dir *dir,
        spin_lock_irqsave(&dir->lock, flags);
        list_add(&tracker->head, &dir->list);
        spin_unlock_irqrestore(&dir->lock, flags);
+       tracker->lat = sched_clock() - t;
        return 0;
 }

result:
[   13.187325] tracker ffff888113285980 alloc cost(ns): 3488
[   13.188050] tracker ffff888113285800 alloc cost(ns): 2612
[   13.188760] tracker ffff888113285940 alloc cost(ns): 2763
[   13.189482] tracker ffff888113285bc0 alloc cost(ns): 1063
[   13.190227] tracker ffff888113285340 alloc cost(ns): 982
[   13.190933] tracker ffff888113285f80 alloc cost(ns): 883
[   13.191638] tracker ffff888113285080 alloc cost(ns): 1006
[   13.192355] tracker ffff888113285cc0 alloc cost(ns): 1007
[   13.193105] tracker ffff888113285dc0 alloc cost(ns): 901
[   13.193811] tracker ffff8881132858c0 alloc cost(ns): 882
[   13.194522] tracker ffff888113285e40 alloc cost(ns): 1000
[   13.195253] tracker ffff888113285a00 alloc cost(ns): 896
[   13.195973] tracker ffff888113285ac0 alloc cost(ns): 903
[   13.196673] tracker ffff888113285c80 alloc cost(ns): 893
[   13.197384] tracker ffff888113285d00 alloc cost(ns): 1220
[   13.198130] tracker ffff888113285140 alloc cost(ns): 979
[   13.198858] tracker ffff888113285180 alloc cost(ns): 893
[   13.199571] tracker ffff8881132850c0 alloc cost(ns): 893

The average cost for a ref_tracker_alloc() is about 0.9us
And most of the time is spent in stack_trace_save().

I'm not sure whether __builtin_return_address(0) is enough ?


>
>> 2. Since we only store 1 callstack in 1 tracker, what if some object
>>    want to hold and put refcnt in different places ?
>
>
>You can use a tracker on the stack (patch 6/19 net: add net device
>refcount tracker to ethtool_phys_id())
>
>For generic uses of dev_hold(), like in dev_get_by_index(), we will
>probably have new helpers
>so that callers can provide where the tracker is put.
>
>Or simply use this sequence to convert a generic/untracked reference
>to a tracked one.
>
>dev = dev_get_by_index(),;
>...
>
>p->dev = dev;
>dev_hold_track(dev, &p->dev_tracker, GFP...)
>dev_put(dev);

Yeah, I see. This looks good for netdev.
For sock_hold/put, I feel it's more complicate, I'm not sure if we
need add lots of tracker.

>
>
>>
>> Thanks.
>>
>> >
>> >With CONFIG_PCPU_DEV_REFCNT=n option, we were able to detect
>> >some class of bugs, but too late (when too many dev_put()
>> >were happening).
>> >
>> >Eric Dumazet (19):
>> >  lib: add reference counting tracking infrastructure
>> >  lib: add tests for reference tracker
>> >  net: add dev_hold_track() and dev_put_track() helpers
>> >  net: add net device refcount tracker to struct netdev_rx_queue
>> >  net: add net device refcount tracker to struct netdev_queue
>> >  net: add net device refcount tracker to ethtool_phys_id()
>> >  net: add net device refcount tracker to dev_ifsioc()
>> >  drop_monitor: add net device refcount tracker
>> >  net: dst: add net device refcount tracking to dst_entry
>> >  ipv6: add net device refcount tracker to rt6_probe_deferred()
>> >  sit: add net device refcount tracking to ip_tunnel
>> >  ipv6: add net device refcount tracker to struct ip6_tnl
>> >  net: add net device refcount tracker to struct neighbour
>> >  net: add net device refcount tracker to struct pneigh_entry
>> >  net: add net device refcount tracker to struct neigh_parms
>> >  net: add net device refcount tracker to struct netdev_adjacent
>> >  ipv6: add net device refcount tracker to struct inet6_dev
>> >  ipv4: add net device refcount tracker to struct in_device
>> >  net/sched: add net device refcount tracker to struct Qdisc
>> >
>> > include/linux/inetdevice.h  |   2 +
>> > include/linux/netdevice.h   |  53 ++++++++++++++
>> > include/linux/ref_tracker.h |  73 +++++++++++++++++++
>> > include/net/devlink.h       |   3 +
>> > include/net/dst.h           |   1 +
>> > include/net/if_inet6.h      |   1 +
>> > include/net/ip6_tunnel.h    |   1 +
>> > include/net/ip_tunnels.h    |   3 +
>> > include/net/neighbour.h     |   3 +
>> > include/net/sch_generic.h   |   2 +-
>> > lib/Kconfig                 |   4 ++
>> > lib/Kconfig.debug           |  10 +++
>> > lib/Makefile                |   4 +-
>> > lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
>> > lib/test_ref_tracker.c      | 116 ++++++++++++++++++++++++++++++
>> > net/Kconfig                 |   8 +++
>> > net/core/dev.c              |  10 ++-
>> > net/core/dev_ioctl.c        |   5 +-
>> > net/core/drop_monitor.c     |   4 +-
>> > net/core/dst.c              |   8 +--
>> > net/core/neighbour.c        |  18 ++---
>> > net/core/net-sysfs.c        |   8 +--
>> > net/ethtool/ioctl.c         |   5 +-
>> > net/ipv4/devinet.c          |   4 +-
>> > net/ipv4/route.c            |   7 +-
>> > net/ipv6/addrconf.c         |   4 +-
>> > net/ipv6/addrconf_core.c    |   2 +-
>> > net/ipv6/ip6_gre.c          |   8 +--
>> > net/ipv6/ip6_tunnel.c       |   4 +-
>> > net/ipv6/ip6_vti.c          |   4 +-
>> > net/ipv6/route.c            |  10 +--
>> > net/ipv6/sit.c              |   4 +-
>> > net/sched/sch_generic.c     |   4 +-
>> > 33 files changed, 481 insertions(+), 52 deletions(-)
>> > create mode 100644 include/linux/ref_tracker.h
>> > create mode 100644 lib/ref_tracker.c
>> > create mode 100644 lib/test_ref_tracker.c
>> >
>> >--
>> >2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH net-next 00/19] net: add preliminary netdev refcount tracking

[Eric Dumazet]

On Thu, Dec 2, 2021 at 7:46 PM dust.li <dust.li@linux.alibaba.com> wrote:
>
> On Thu, Dec 02, 2021 at 07:13:12AM -0800, Eric Dumazet wrote:
> >On Thu, Dec 2, 2021 at 4:49 AM dust.li <dust.li@linux.alibaba.com> wrote:
> >>
> >> On Wed, Dec 01, 2021 at 07:21:20PM -0800, Eric Dumazet wrote:
> >> >From: Eric Dumazet <edumazet@google.com>
> >> >
> >> >Two first patches add a generic infrastructure, that will be used
> >> >to get tracking of refcount increments/decrements.
> >> >
> >> >The general idea is to be able to precisely pair each decrement with
> >> >a corresponding prior increment. Both share a cookie, basically
> >> >a pointer to private data storing stack traces.
> >> >
> >> >Then a series of 17 patches converts some dev_hold()/dev_put()
> >> >pairs to new hepers : dev_hold_track() and dev_put_track().
> >> >
> >> >Hopefully this will be used by developpers and syzbot to
> >> >root cause bugs that cause netdevice dismantles freezes.
> >>
> >> Hi Eric:
> >>
> >> I really like the idea of adding debug informantion for debugging
> >> refcnt problems, we have met some bugs of leaking netdev refcnt in
> >> the past and debugging them is really HARD !!
> >>
> >> Recently, when investigating a sk_refcnt double free bug in SMC,
> >> I added some debug code a bit similar like this. I'm curious have
> >> you considered expanding the ref tracker infrastructure into other
> >> places like sock_hold/put() or even some hot path ?
> >
> >Sure, this should be absolutely doable with this generic infra.
> >
> >>
> >> AFAIU, ref tracker add a tracker inside each object who want to
> >> hold a refcnt, and stored the callstack into the tracker.
> >> I have 2 questions here:
> >> 1. If we want to use this in the hot path, looks like the overhead
> >>    is a bit heavy ?
> >
> >Much less expensive than lockdep (we use one spinlock per struct
> >ref_tracker_dir), so I think this is something doable.
>
> Yeah, I'm thinking enable this feature by default in our kernel
> so I care about this won't bring regression.

I doubt it, especially if some workload depended on device refcount being percpu
(CONFIG_PCPU_DEV_REFCNT=y)

> I did a simple test in case this might be helpful to other.
>
> Testing was triggered by test_ref_tracker.ko run on a Intel Xeon server.
> Added the following debug code:
>
> @@ -12,8 +13,14 @@ struct ref_tracker {
>         bool                    dead;
>         depot_stack_handle_t    alloc_stack_handle;
>         depot_stack_handle_t    free_stack_handle;
> +       unsigned long long      lat;
>  };
>
> +static void dump_tracker_latency(struct ref_tracker *tracker)
> +{
> +       printk("tracker %pK alloc cost(ns): %llu\n", tracker, tracker->lat);
> +}
> +
>  void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
>  {
>         struct ref_tracker *tracker, *n;
> @@ -23,6 +30,7 @@ void ref_tracker_dir_exit(struct ref_tracker_dir *dir)
>         spin_lock_irqsave(&dir->lock, flags);
>         list_for_each_entry_safe(tracker, n, &dir->quarantine, head) {
>                 list_del(&tracker->head);
> +               dump_tracker_latency(tracker);
>                 kfree(tracker);
>                 dir->quarantine_avail++;
>         }
>  ...
>  }
>
> @@ -70,7 +78,9 @@ int ref_tracker_alloc(struct ref_tracker_dir *dir,
>         struct ref_tracker *tracker;
>         unsigned int nr_entries;
>         unsigned long flags;
> +       unsigned long long t;
>
> +       t = sched_clock();
>         *trackerp = tracker = kzalloc(sizeof(*tracker), gfp | __GFP_NOFAIL);
>         if (unlikely(!tracker)) {
>                 pr_err_once("memory allocation failure, unreliable refcount tracker.\n");
> @@ -84,6 +94,7 @@ int ref_tracker_alloc(struct ref_tracker_dir *dir,
>         spin_lock_irqsave(&dir->lock, flags);
>         list_add(&tracker->head, &dir->list);
>         spin_unlock_irqrestore(&dir->lock, flags);
> +       tracker->lat = sched_clock() - t;
>         return 0;
>  }
>
> result:
> [   13.187325] tracker ffff888113285980 alloc cost(ns): 3488
> [   13.188050] tracker ffff888113285800 alloc cost(ns): 2612
> [   13.188760] tracker ffff888113285940 alloc cost(ns): 2763
> [   13.189482] tracker ffff888113285bc0 alloc cost(ns): 1063
> [   13.190227] tracker ffff888113285340 alloc cost(ns): 982
> [   13.190933] tracker ffff888113285f80 alloc cost(ns): 883
> [   13.191638] tracker ffff888113285080 alloc cost(ns): 1006
> [   13.192355] tracker ffff888113285cc0 alloc cost(ns): 1007
> [   13.193105] tracker ffff888113285dc0 alloc cost(ns): 901
> [   13.193811] tracker ffff8881132858c0 alloc cost(ns): 882
> [   13.194522] tracker ffff888113285e40 alloc cost(ns): 1000
> [   13.195253] tracker ffff888113285a00 alloc cost(ns): 896
> [   13.195973] tracker ffff888113285ac0 alloc cost(ns): 903
> [   13.196673] tracker ffff888113285c80 alloc cost(ns): 893
> [   13.197384] tracker ffff888113285d00 alloc cost(ns): 1220
> [   13.198130] tracker ffff888113285140 alloc cost(ns): 979
> [   13.198858] tracker ffff888113285180 alloc cost(ns): 893
> [   13.199571] tracker ffff8881132850c0 alloc cost(ns): 893
>
> The average cost for a ref_tracker_alloc() is about 0.9us
> And most of the time is spent in stack_trace_save().
>
> I'm not sure whether __builtin_return_address(0) is enough ?

Probably not.

Perhaps we can make the depth of the stack trace configurable,
but  this seems premature right now.


>
>
> >
> >> 2. Since we only store 1 callstack in 1 tracker, what if some object
> >>    want to hold and put refcnt in different places ?
> >
> >
> >You can use a tracker on the stack (patch 6/19 net: add net device
> >refcount tracker to ethtool_phys_id())
> >
> >For generic uses of dev_hold(), like in dev_get_by_index(), we will
> >probably have new helpers
> >so that callers can provide where the tracker is put.
> >
> >Or simply use this sequence to convert a generic/untracked reference
> >to a tracked one.
> >
> >dev = dev_get_by_index(),;
> >...
> >
> >p->dev = dev;
> >dev_hold_track(dev, &p->dev_tracker, GFP...)
> >dev_put(dev);
>
> Yeah, I see. This looks good for netdev.
> For sock_hold/put, I feel it's more complicate, I'm not sure if we
> need add lots of tracker.

I fail to see why there would be a lot of trackers.

I am betting less than 50, if you only focus on the usual suspects.

>
> >
> >
> >>
> >> Thanks.
> >>
> >> >
> >> >With CONFIG_PCPU_DEV_REFCNT=n option, we were able to detect
> >> >some class of bugs, but too late (when too many dev_put()
> >> >were happening).
> >> >
> >> >Eric Dumazet (19):
> >> >  lib: add reference counting tracking infrastructure
> >> >  lib: add tests for reference tracker
> >> >  net: add dev_hold_track() and dev_put_track() helpers
> >> >  net: add net device refcount tracker to struct netdev_rx_queue
> >> >  net: add net device refcount tracker to struct netdev_queue
> >> >  net: add net device refcount tracker to ethtool_phys_id()
> >> >  net: add net device refcount tracker to dev_ifsioc()
> >> >  drop_monitor: add net device refcount tracker
> >> >  net: dst: add net device refcount tracking to dst_entry
> >> >  ipv6: add net device refcount tracker to rt6_probe_deferred()
> >> >  sit: add net device refcount tracking to ip_tunnel
> >> >  ipv6: add net device refcount tracker to struct ip6_tnl
> >> >  net: add net device refcount tracker to struct neighbour
> >> >  net: add net device refcount tracker to struct pneigh_entry
> >> >  net: add net device refcount tracker to struct neigh_parms
> >> >  net: add net device refcount tracker to struct netdev_adjacent
> >> >  ipv6: add net device refcount tracker to struct inet6_dev
> >> >  ipv4: add net device refcount tracker to struct in_device
> >> >  net/sched: add net device refcount tracker to struct Qdisc
> >> >
> >> > include/linux/inetdevice.h  |   2 +
> >> > include/linux/netdevice.h   |  53 ++++++++++++++
> >> > include/linux/ref_tracker.h |  73 +++++++++++++++++++
> >> > include/net/devlink.h       |   3 +
> >> > include/net/dst.h           |   1 +
> >> > include/net/if_inet6.h      |   1 +
> >> > include/net/ip6_tunnel.h    |   1 +
> >> > include/net/ip_tunnels.h    |   3 +
> >> > include/net/neighbour.h     |   3 +
> >> > include/net/sch_generic.h   |   2 +-
> >> > lib/Kconfig                 |   4 ++
> >> > lib/Kconfig.debug           |  10 +++
> >> > lib/Makefile                |   4 +-
> >> > lib/ref_tracker.c           | 140 ++++++++++++++++++++++++++++++++++++
> >> > lib/test_ref_tracker.c      | 116 ++++++++++++++++++++++++++++++
> >> > net/Kconfig                 |   8 +++
> >> > net/core/dev.c              |  10 ++-
> >> > net/core/dev_ioctl.c        |   5 +-
> >> > net/core/drop_monitor.c     |   4 +-
> >> > net/core/dst.c              |   8 +--
> >> > net/core/neighbour.c        |  18 ++---
> >> > net/core/net-sysfs.c        |   8 +--
> >> > net/ethtool/ioctl.c         |   5 +-
> >> > net/ipv4/devinet.c          |   4 +-
> >> > net/ipv4/route.c            |   7 +-
> >> > net/ipv6/addrconf.c         |   4 +-
> >> > net/ipv6/addrconf_core.c    |   2 +-
> >> > net/ipv6/ip6_gre.c          |   8 +--
> >> > net/ipv6/ip6_tunnel.c       |   4 +-
> >> > net/ipv6/ip6_vti.c          |   4 +-
> >> > net/ipv6/route.c            |  10 +--
> >> > net/ipv6/sit.c              |   4 +-
> >> > net/sched/sch_generic.c     |   4 +-
> >> > 33 files changed, 481 insertions(+), 52 deletions(-)
> >> > create mode 100644 include/linux/ref_tracker.h
> >> > create mode 100644 lib/ref_tracker.c
> >> > create mode 100644 lib/test_ref_tracker.c
> >> >
> >> >--
> >> >2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[Dmitry Vyukov]

On Thu, 2 Dec 2021 at 20:46, Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Dec 2, 2021 at 11:25 AM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Eric,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on net-next/master]
> >
> > url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> > config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp@intel.com/config)
> > compiler: nios2-linux-gcc (GCC) 11.2.0
> > 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
> >         # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
> >         git remote add linux-review https://github.com/0day-ci/linux
> >         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> >         git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
> >         # save the config file to linux build tree
> >         mkdir build_dir
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
> > >> (.text+0x2e4): undefined reference to `save_stack_trace'
> >    (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'
> >
> > Kconfig warnings: (for reference only)
> >    WARNING: unmet direct dependencies detected for STACKTRACE
> >    Depends on STACKTRACE_SUPPORT
> >    Selected by
> >    - STACKDEPOT
> >
>
> I am not sure I understand this.
>
> Dmitry, do I need to add a depends on STACKTRACE_SUPPORT.

Humm... There is something strange about nios2 arch.

KASAN depends on ARCH_HAVE_KASAN which is not selected for nios2, so
it implicitly avoids this issue.

But I see PAGE_OWNER that also uses STACKDEPOT has "Depends on
STACKTRACE_SUPPORT".
So I guess yes.

I am not sure how Kconfig will reach if we make STACKDEPOT depend on
STACKTRACE_SUPPORT  and your config  says "select STACKDEPOT"...

Re: [PATCH net-next 02/19] lib: add tests for reference tracker

[Dmitry Vyukov]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

On Thu, 2 Dec 2021 at 20:46, Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Dec 2, 2021 at 11:25 AM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Eric,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on net-next/master]
> >
> > url:    https://github.com/0day-ci/linux/commits/Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8057cbb8335cf6d419866737504473833e1d128a
> > config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20211203/202112030323.z9IhC2B3-lkp(a)intel.com/config)
> > compiler: nios2-linux-gcc (GCC) 11.2.0
> > 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
> >         # https://github.com/0day-ci/linux/commit/5da0cdb1848fae9fb2d9d749bb94e568e2493df8
> >         git remote add linux-review https://github.com/0day-ci/linux
> >         git fetch --no-tags linux-review Eric-Dumazet/net-add-preliminary-netdev-refcount-tracking/20211202-112353
> >         git checkout 5da0cdb1848fae9fb2d9d749bb94e568e2493df8
> >         # save the config file to linux build tree
> >         mkdir build_dir
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    nios2-linux-ld: kernel/stacktrace.o: in function `stack_trace_save':
> > >> (.text+0x2e4): undefined reference to `save_stack_trace'
> >    (.text+0x2e4): relocation truncated to fit: R_NIOS2_CALL26 against `save_stack_trace'
> >
> > Kconfig warnings: (for reference only)
> >    WARNING: unmet direct dependencies detected for STACKTRACE
> >    Depends on STACKTRACE_SUPPORT
> >    Selected by
> >    - STACKDEPOT
> >
>
> I am not sure I understand this.
>
> Dmitry, do I need to add a depends on STACKTRACE_SUPPORT.

Humm... There is something strange about nios2 arch.

KASAN depends on ARCH_HAVE_KASAN which is not selected for nios2, so
it implicitly avoids this issue.

But I see PAGE_OWNER that also uses STACKDEPOT has "Depends on
STACKTRACE_SUPPORT".
So I guess yes.

I am not sure how Kconfig will reach if we make STACKDEPOT depend on
STACKTRACE_SUPPORT  and your config  says "select STACKDEPOT"...

Re: [PATCH net-next 03/19] net: add dev_hold_track() and dev_put_track() helpers

[Jakub Kicinski]

On Wed,  1 Dec 2021 19:21:23 -0800 Eric Dumazet wrote:
> +static inline void dev_hold_track(struct net_device *dev,
> +				  netdevice_tracker *tracker, gfp_t gfp)
> +{
> +	if (dev) {
> +		dev_hold(dev);
> +#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
> +		ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp);
> +#endif
> +	}
> +}
> +
> +static inline void dev_put_track(struct net_device *dev,
> +				 netdevice_tracker *tracker)
> +{
> +	if (dev) {
> +#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
> +		ref_tracker_free(&dev->refcnt_tracker, tracker);
> +#endif
> +		dev_put(dev);
> +	}
> +}

Hi Eric, how bad would it be if we renamed dev_hold/put_track() to
netdev_hold/put()? IIUC we use the dev_ prefix for "historic reasons"
could this be an opportunity to stop doing that?

Re: [PATCH net-next 03/19] net: add dev_hold_track() and dev_put_track() helpers

[Eric Dumazet]

On Mon, May 23, 2022 at 11:03 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed,  1 Dec 2021 19:21:23 -0800 Eric Dumazet wrote:
> > +static inline void dev_hold_track(struct net_device *dev,
> > +                               netdevice_tracker *tracker, gfp_t gfp)
> > +{
> > +     if (dev) {
> > +             dev_hold(dev);
> > +#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
> > +             ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp);
> > +#endif
> > +     }
> > +}
> > +
> > +static inline void dev_put_track(struct net_device *dev,
> > +                              netdevice_tracker *tracker)
> > +{
> > +     if (dev) {
> > +#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
> > +             ref_tracker_free(&dev->refcnt_tracker, tracker);
> > +#endif
> > +             dev_put(dev);
> > +     }
> > +}
>
> Hi Eric, how bad would it be if we renamed dev_hold/put_track() to
> netdev_hold/put()? IIUC we use the dev_ prefix for "historic reasons"
> could this be an opportunity to stop doing that?

Sure, we can do that, thanks.

Re: [PATCH net-next 03/19] net: add dev_hold_track() and dev_put_track() helpers

[Jakub Kicinski]

On Mon, 23 May 2022 11:14:27 -0700 Eric Dumazet wrote:
> > Hi Eric, how bad would it be if we renamed dev_hold/put_track() to
> > netdev_hold/put()? IIUC we use the dev_ prefix for "historic reasons"
> > could this be an opportunity to stop doing that?  
> 
> Sure, we can do that, thanks.

Thanks! I'll send a rename after the merge window.