From: Jason Xing <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
Date: Sat, 11 Feb 2023 14:51:53 +0800
> From: Jason Xing <[email protected]>
>
> 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.
>
> Signed-off-by: Jason Xing <[email protected]>
> ---
> 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
On Tue, Feb 14, 2023 at 1:30 AM Kuniyuki Iwashima <[email protected]> wrote:
>
> From: Jason Xing <[email protected]>
> Date: Sat, 11 Feb 2023 14:51:53 +0800
> > From: Jason Xing <[email protected]>
> >
> > 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.
>
>
> > 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
>
>
> >
> > Signed-off-by: Jason Xing <[email protected]>
> > ---
> > 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