Re: [RFC net-next 1/2] net: add netdev_refs debug
From: Leon Romanovsky <leon@kernel.org>
Date: 2021-11-17 18:24:22
On Wed, Nov 17, 2021 at 09:47:22AM -0800, Jakub Kicinski wrote:
quoted hunk ↗ jump to hunk
Debugging netdev ref leaks is still pretty hard. Eric added optional use of a normal refcount which is useful for tracking abuse of existing users. For new code, however, it'd be great if we could actually track the refs per-user. Allowing us to detect leaks where they happen. This patch introduces a netdev_ref type and uses the debug_objects infra to track refs being lost or misused. In the future we can extend this structure to also catch those who fail to release the ref on unregistering notification. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- MAINTAINERS | 1 + include/linux/netdev_refs.h | 104 ++++++++++++++++++++++++++++++++++++ lib/Kconfig.debug | 7 +++ net/core/dev.c | 8 +++ 4 files changed, 120 insertions(+) create mode 100644 include/linux/netdev_refs.hdiff --git a/MAINTAINERS b/MAINTAINERS index 4c74516e4353..47fe27175c9f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -18482,6 +18482,7 @@ F: include/uapi/linux/pkt_sched.h F: include/uapi/linux/tc_act/ F: include/uapi/linux/tc_ematch/ F: net/sched/ +F: tools/testing/selftests/tc-testing/ TC90522 MEDIA DRIVER M: Akihiro Tsukada <tskd08@gmail.com>diff --git a/include/linux/netdev_refs.h b/include/linux/netdev_refs.h new file mode 100644 index 000000000000..326772ea0a63 --- /dev/null +++ b/include/linux/netdev_refs.h@@ -0,0 +1,104 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef _LINUX_NETDEV_REFS_H +#define _LINUX_NETDEV_REFS_H + +#include <linux/debugobjects.h> +#include <linux/netdevice.h> + +/* Explicit netdevice references + * struct netdev_ref is a storage for a reference. It's equivalent + * to a netdev pointer, but when debug is enabled it performs extra checks. + * Most users will want to take a reference with netdev_hold(), access it + * via netdev_ref_ptr() and release with netdev_put(). + */ + +struct netdev_ref { + struct net_device *dev; +#ifdef CONFIG_DEBUG_OBJECTS_NETDEV_REFS + refcount_t cnt; +#endif +}; + +extern const struct debug_obj_descr netdev_ref_debug_descr; + +/* Store a raw, unprotected pointer */ +static inline void __netdev_ref_store(struct netdev_ref *ref, + struct net_device *dev) +{ + ref->dev = dev; + +#ifdef CONFIG_DEBUG_OBJECTS_NETDEV_REFS + refcount_set(&ref->cnt, 0);
This is very uncommon pattern. I would expect that first pointer access will start from 1, like all refcount_t users. If you still prefer to start from 0, i suggest you to use atomic_t. IMHO, much better will be to use kref for this type of reference counting. Thanks