[PATCH net-next] net: Kconfig.debug: wrap socket refcnt debug into an option

From: Jason Xing <kernelxing@tencent.com>

Since commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock")
commented out the definition of SOCK_REFCNT_DEBUG and later another
patch deleted it, we need to enable it through defining it manually
somewhere. Wrapping it into an option in Kconfig.debug could make
it much clearer and easier for some developers to do things based
on this change.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 include/net/sock.h            | 8 ++++----
 net/Kconfig.debug             | 8 ++++++++
 net/ipv4/inet_timewait_sock.c | 2 +-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index dcd72e6285b2..1b001efeb9b5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h

@@ -1349,7 +1349,7 @@ struct proto {
 	char			name[32];
 
 	struct list_head	node;
-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
 	atomic_t		socks;
 #endif
 	int			(*diag_destroy)(struct sock *sk, int err);

@@ -1359,7 +1359,7 @@ int proto_register(struct proto *prot, int alloc_slab);
 void proto_unregister(struct proto *prot);
 int sock_load_diag_module(int family, int protocol);
 
-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
 static inline void sk_refcnt_debug_inc(struct sock *sk)
 {
 	atomic_inc(&sk->sk_prot->socks);

@@ -1378,11 +1378,11 @@ static inline void sk_refcnt_debug_release(const struct sock *sk)
 		printk(KERN_DEBUG "Destruction of the %s socket %p delayed, refcnt=%d\n",
 		       sk->sk_prot->name, sk, refcount_read(&sk->sk_refcnt));
 }
-#else /* SOCK_REFCNT_DEBUG */
+#else /* CONFIG_SOCK_REFCNT_DEBUG */
 #define sk_refcnt_debug_inc(sk) do { } while (0)
 #define sk_refcnt_debug_dec(sk) do { } while (0)
 #define sk_refcnt_debug_release(sk) do { } while (0)
-#endif /* SOCK_REFCNT_DEBUG */
+#endif /* CONFIG_SOCK_REFCNT_DEBUG */
 
 INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));
 

diff --git a/net/Kconfig.debug b/net/Kconfig.debug
index 5e3fffe707dd..667396d70e10 100644
--- a/net/Kconfig.debug
+++ b/net/Kconfig.debug

@@ -18,6 +18,14 @@ config NET_NS_REFCNT_TRACKER
 	  Enable debugging feature to track netns references.
 	  This adds memory and cpu costs.
 
+config SOCK_REFCNT_DEBUG
+	bool "Enable socket refcount debug"
+	depends on DEBUG_KERNEL && NET
+	default n
+	help
+	  Enable debugging feature to track socket references.
+	  This adds memory and cpu costs.
+
 config DEBUG_NET
 	bool "Add generic networking debug"
 	depends on DEBUG_KERNEL && NET

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index beed32fff484..e313516b64ce 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c

@@ -77,7 +77,7 @@ void inet_twsk_free(struct inet_timewait_sock *tw)
 {
 	struct module *owner = tw->tw_prot->owner;
 	twsk_destructor((struct sock *)tw);
-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
 	pr_debug("%s timewait_sock %p released\n", tw->tw_prot->name, tw);
 #endif
 	kmem_cache_free(tw->tw_prot->twsk_prot->twsk_slab, tw);

-- 
2.37.3

From:   Jason Xing <redacted>
Date:   Sat, 11 Feb 2023 14:51:53 +0800

From: Jason Xing <kernelxing@tencent.com>

Since commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock")
commented out the definition of SOCK_REFCNT_DEBUG and later another
patch deleted it,

e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
is the commit which commented out SOCK_REFCNT_DEBUG, and 463c84b97f24
removed it.

we need to enable it through defining it manually
somewhere. Wrapping it into an option in Kconfig.debug could make
it much clearer and easier for some developers to do things based
on this change.

Considering SOCK_REFCNT_DEBUG is removed in 2005, how about removing
the whole feature?  I think we can track the same info easily with
bpftrace + kprobe.

On Tue, Feb 14, 2023 at 1:30 AM Kuniyuki Iwashima [off-list ref] wrote:

From:   Jason Xing <redacted>
Date:   Sat, 11 Feb 2023 14:51:53 +0800

quoted

From: Jason Xing <kernelxing@tencent.com>

Since commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock")
commented out the definition of SOCK_REFCNT_DEBUG and later another
patch deleted it,

e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
is the commit which commented out SOCK_REFCNT_DEBUG, and 463c84b97f24
removed it.

Yes.

quoted

we need to enable it through defining it manually
somewhere. Wrapping it into an option in Kconfig.debug could make
it much clearer and easier for some developers to do things based
on this change.

Considering SOCK_REFCNT_DEBUG is removed in 2005, how about removing
the whole feature?  I think we can track the same info easily with
bpftrace + kprobe.

I agree with you since it seems no one is developing more features
based on it.  I just did check every caller which may call those debug
interfaces and now confirm we surely can use bpf/kprobe related tools
to track. So I decided to remove the whole feature in the next
submission.

Thanks,
Jason

quoted

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 include/net/sock.h            | 8 ++++----
 net/Kconfig.debug             | 8 ++++++++
 net/ipv4/inet_timewait_sock.c | 2 +-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index dcd72e6285b2..1b001efeb9b5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h

@@ -1349,7 +1349,7 @@ struct proto {
      char                    name[32];

      struct list_head        node;
-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
      atomic_t                socks;
 #endif
      int                     (*diag_destroy)(struct sock *sk, int err);

@@ -1359,7 +1359,7 @@ int proto_register(struct proto *prot, int alloc_slab);
 void proto_unregister(struct proto *prot);
 int sock_load_diag_module(int family, int protocol);

-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
 static inline void sk_refcnt_debug_inc(struct sock *sk)
 {
      atomic_inc(&sk->sk_prot->socks);

@@ -1378,11 +1378,11 @@ static inline void sk_refcnt_debug_release(const struct sock *sk)
              printk(KERN_DEBUG "Destruction of the %s socket %p delayed, refcnt=%d\n",
                     sk->sk_prot->name, sk, refcount_read(&sk->sk_refcnt));
 }
-#else /* SOCK_REFCNT_DEBUG */
+#else /* CONFIG_SOCK_REFCNT_DEBUG */
 #define sk_refcnt_debug_inc(sk) do { } while (0)
 #define sk_refcnt_debug_dec(sk) do { } while (0)
 #define sk_refcnt_debug_release(sk) do { } while (0)
-#endif /* SOCK_REFCNT_DEBUG */
+#endif /* CONFIG_SOCK_REFCNT_DEBUG */

 INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));

diff --git a/net/Kconfig.debug b/net/Kconfig.debug
index 5e3fffe707dd..667396d70e10 100644
--- a/net/Kconfig.debug
+++ b/net/Kconfig.debug

@@ -18,6 +18,14 @@ config NET_NS_REFCNT_TRACKER
        Enable debugging feature to track netns references.
        This adds memory and cpu costs.

+config SOCK_REFCNT_DEBUG
+     bool "Enable socket refcount debug"
+     depends on DEBUG_KERNEL && NET
+     default n
+     help
+       Enable debugging feature to track socket references.
+       This adds memory and cpu costs.
+
 config DEBUG_NET
      bool "Add generic networking debug"
      depends on DEBUG_KERNEL && NET

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index beed32fff484..e313516b64ce 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c

@@ -77,7 +77,7 @@ void inet_twsk_free(struct inet_timewait_sock *tw)
 {
      struct module *owner = tw->tw_prot->owner;
      twsk_destructor((struct sock *)tw);
-#ifdef SOCK_REFCNT_DEBUG
+#ifdef CONFIG_SOCK_REFCNT_DEBUG
      pr_debug("%s timewait_sock %p released\n", tw->tw_prot->name, tw);
 #endif
      kmem_cache_free(tw->tw_prot->twsk_prot->twsk_slab, tw);
--

2.37.3