[v2,net-next] net: no longer support SOCK_REFCNT_DEBUG feature
Commit Message
From: Jason Xing <kernelxing@tencent.com>
Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
Since we could track all of them through bpf and kprobe related tools
and the feature could print loads of information which might not be
that helpful even under a little bit pressure, the whole feature which
has been inactive for many years is no longer supported.
Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
v2:
1) change the title and body message.
2) remove the whole feature instead suggested by Kuniyuki Iwashima.
---
include/net/sock.h | 28 ----------------------------
net/core/sock.c | 13 -------------
net/ipv4/af_inet.c | 3 ---
net/ipv4/inet_connection_sock.c | 2 --
net/ipv4/inet_timewait_sock.c | 3 ---
net/ipv6/af_inet6.c | 10 ----------
net/ipv6/ipv6_sockglue.c | 12 ------------
net/mptcp/protocol.c | 1 -
net/packet/af_packet.c | 4 ----
net/sctp/ipv6.c | 2 --
net/sctp/protocol.c | 2 --
net/smc/af_smc.c | 3 ---
net/xdp/xsk.c | 4 ----
13 files changed, 87 deletions(-)
Comments
From: Jason Xing <kerneljasonxing@gmail.com>
Date: Tue, 14 Feb 2023 12:14:10 +0800
> From: Jason Xing <kernelxing@tencent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Thanks!
> ---
> v2:
> 1) change the title and body message.
> 2) remove the whole feature instead suggested by Kuniyuki Iwashima.
> ---
> include/net/sock.h | 28 ----------------------------
> net/core/sock.c | 13 -------------
> net/ipv4/af_inet.c | 3 ---
> net/ipv4/inet_connection_sock.c | 2 --
> net/ipv4/inet_timewait_sock.c | 3 ---
> net/ipv6/af_inet6.c | 10 ----------
> net/ipv6/ipv6_sockglue.c | 12 ------------
> net/mptcp/protocol.c | 1 -
> net/packet/af_packet.c | 4 ----
> net/sctp/ipv6.c | 2 --
> net/sctp/protocol.c | 2 --
> net/smc/af_smc.c | 3 ---
> net/xdp/xsk.c | 4 ----
> 13 files changed, 87 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index dcd72e6285b2..e6369068a7bb 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1349,9 +1349,6 @@ struct proto {
> char name[32];
>
> struct list_head node;
> -#ifdef SOCK_REFCNT_DEBUG
> - atomic_t socks;
> -#endif
> int (*diag_destroy)(struct sock *sk, int err);
> } __randomize_layout;
>
> @@ -1359,31 +1356,6 @@ 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
> -static inline void sk_refcnt_debug_inc(struct sock *sk)
> -{
> - atomic_inc(&sk->sk_prot->socks);
> -}
> -
> -static inline void sk_refcnt_debug_dec(struct sock *sk)
> -{
> - atomic_dec(&sk->sk_prot->socks);
> - printk(KERN_DEBUG "%s socket %p released, %d are still alive\n",
> - sk->sk_prot->name, sk, atomic_read(&sk->sk_prot->socks));
> -}
> -
> -static inline void sk_refcnt_debug_release(const struct sock *sk)
> -{
> - if (refcount_read(&sk->sk_refcnt) != 1)
> - 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 */
> -#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 */
> -
> INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));
>
> static inline int sk_forward_alloc_get(const struct sock *sk)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index f954d5893e79..be7b29d97637 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2338,17 +2338,6 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
> smp_wmb();
> refcount_set(&newsk->sk_refcnt, 2);
>
> - /* Increment the counter in the same struct proto as the master
> - * sock (sk_refcnt_debug_inc uses newsk->sk_prot->socks, that
> - * is the same as sk->sk_prot->socks, as this field was copied
> - * with memcpy).
> - *
> - * This _changes_ the previous behaviour, where
> - * tcp_create_openreq_child always was incrementing the
> - * equivalent to tcp_prot->socks (inet_sock_nr), so this have
> - * to be taken into account in all callers. -acme
> - */
> - sk_refcnt_debug_inc(newsk);
> sk_set_socket(newsk, NULL);
> sk_tx_queue_clear(newsk);
> RCU_INIT_POINTER(newsk->sk_wq, NULL);
> @@ -3696,8 +3685,6 @@ void sk_common_release(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> sock_put(sk);
> }
> EXPORT_SYMBOL(sk_common_release);
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 6c0ec2789943..f46a3924c440 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -156,7 +156,6 @@ void inet_sock_destruct(struct sock *sk)
> kfree(rcu_dereference_protected(inet->inet_opt, 1));
> dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
> dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
> - sk_refcnt_debug_dec(sk);
> }
> EXPORT_SYMBOL(inet_sock_destruct);
>
> @@ -356,8 +355,6 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
> inet->mc_list = NULL;
> inet->rcv_tos = 0;
>
> - sk_refcnt_debug_inc(sk);
> -
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> * the user to assign a number at socket
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index d1f837579398..64be59d93b04 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1178,8 +1178,6 @@ void inet_csk_destroy_sock(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> this_cpu_dec(*sk->sk_prot->orphan_count);
>
> sock_put(sk);
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index beed32fff484..40052414c7c7 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -77,9 +77,6 @@ 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
> - 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);
> module_put(owner);
> }
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index fee9163382c2..c93f2e865fea 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -238,16 +238,6 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
> inet->pmtudisc = IP_PMTUDISC_DONT;
> else
> inet->pmtudisc = IP_PMTUDISC_WANT;
> - /*
> - * Increment only the relevant sk_prot->socks debug field, this changes
> - * the previous behaviour of incrementing both the equivalent to
> - * answer->prot->socks (inet6_sock_nr) and inet_sock_nr.
> - *
> - * This allows better debug granularity as we'll know exactly how many
> - * UDPv6, TCPv6, etc socks were allocated, not the sum of all IPv6
> - * transport protocol socks. -acme
> - */
> - sk_refcnt_debug_inc(sk);
>
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 9ce51680290b..2917dd8d198c 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -464,13 +464,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> __ipv6_sock_mc_close(sk);
> __ipv6_sock_ac_close(sk);
>
> - /*
> - * Sock is moving from IPv6 to IPv4 (sk_prot), so
> - * remove it from the refcnt debug socks count in the
> - * original family...
> - */
> - sk_refcnt_debug_dec(sk);
> -
> if (sk->sk_protocol == IPPROTO_TCP) {
> struct inet_connection_sock *icsk = inet_csk(sk);
>
> @@ -507,11 +500,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>
> inet6_cleanup_sock(sk);
>
> - /*
> - * ... and add it to the refcnt debug socks count
> - * in the new family. -acme
> - */
> - sk_refcnt_debug_inc(sk);
> module_put(THIS_MODULE);
> retv = 0;
> break;
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8cd6cc67c2c5..e913752df112 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2876,7 +2876,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
> sk_stream_kill_queues(sk);
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
> }
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b5ab98ca2511..a4c8f86ac12a 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1335,8 +1335,6 @@ static void packet_sock_destruct(struct sock *sk)
> pr_err("Attempt to release alive packet socket: %p\n", sk);
> return;
> }
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static bool fanout_flow_is_huge(struct packet_sock *po, struct sk_buff *skb)
> @@ -3172,7 +3170,6 @@ static int packet_release(struct socket *sock)
>
> skb_queue_purge(&sk->sk_receive_queue);
> packet_free_pending(po);
> - sk_refcnt_debug_release(sk);
>
> sock_put(sk);
> return 0;
> @@ -3362,7 +3359,6 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> packet_cached_dev_reset(po);
>
> sk->sk_destruct = packet_sock_destruct;
> - sk_refcnt_debug_inc(sk);
>
> /*
> * Attach a protocol block
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 097bd60ce964..62b436a2c8fe 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -807,8 +807,6 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
>
> newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 909a89a1cff4..c365df24ad33 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -601,8 +601,6 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
>
> newinet->inet_daddr = asoc->peer.primary_addr.v4.sin_addr.s_addr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index e12d4fa5aece..c594312e22cd 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -359,8 +359,6 @@ static void smc_destruct(struct sock *sk)
> return;
> if (!sock_flag(sk, SOCK_DEAD))
> return;
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> @@ -389,7 +387,6 @@ static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> spin_lock_init(&smc->accept_q_lock);
> spin_lock_init(&smc->conn.send_lock);
> sk->sk_prot->hash(sk);
> - sk_refcnt_debug_inc(sk);
> mutex_init(&smc->clcsock_release_lock);
> smc_init_saved_callbacks(smc);
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 9f0561b67c12..a245c1b4a21b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -845,7 +845,6 @@ static int xsk_release(struct socket *sock)
> sock_orphan(sk);
> sock->sk = NULL;
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
>
> return 0;
> @@ -1396,8 +1395,6 @@ static void xsk_destruct(struct sock *sk)
>
> if (!xp_put_pool(xs->pool))
> xdp_put_umem(xs->umem, !xs->pool);
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static int xsk_create(struct net *net, struct socket *sock, int protocol,
> @@ -1427,7 +1424,6 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> sk->sk_family = PF_XDP;
>
> sk->sk_destruct = xsk_destruct;
> - sk_refcnt_debug_inc(sk);
>
> sock_set_flag(sk, SOCK_RCU_FREE);
>
> --
> 2.37.3
On 14.02.23 05:14, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> v2:
> 1) change the title and body message.
> 2) remove the whole feature instead suggested by Kuniyuki Iwashima.
> ---
> include/net/sock.h | 28 ----------------------------
> net/core/sock.c | 13 -------------
> net/ipv4/af_inet.c | 3 ---
> net/ipv4/inet_connection_sock.c | 2 --
> net/ipv4/inet_timewait_sock.c | 3 ---
> net/ipv6/af_inet6.c | 10 ----------
> net/ipv6/ipv6_sockglue.c | 12 ------------
> net/mptcp/protocol.c | 1 -
> net/packet/af_packet.c | 4 ----
> net/sctp/ipv6.c | 2 --
> net/sctp/protocol.c | 2 --
> net/smc/af_smc.c | 3 ---
> net/xdp/xsk.c | 4 ----
> 13 files changed, 87 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index dcd72e6285b2..e6369068a7bb 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1349,9 +1349,6 @@ struct proto {
> char name[32];
>
> struct list_head node;
> -#ifdef SOCK_REFCNT_DEBUG
> - atomic_t socks;
> -#endif
> int (*diag_destroy)(struct sock *sk, int err);
> } __randomize_layout;
>
> @@ -1359,31 +1356,6 @@ 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
> -static inline void sk_refcnt_debug_inc(struct sock *sk)
> -{
> - atomic_inc(&sk->sk_prot->socks);
> -}
> -
> -static inline void sk_refcnt_debug_dec(struct sock *sk)
> -{
> - atomic_dec(&sk->sk_prot->socks);
> - printk(KERN_DEBUG "%s socket %p released, %d are still alive\n",
> - sk->sk_prot->name, sk, atomic_read(&sk->sk_prot->socks));
> -}
> -
> -static inline void sk_refcnt_debug_release(const struct sock *sk)
> -{
> - if (refcount_read(&sk->sk_refcnt) != 1)
> - 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 */
> -#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 */
> -
> INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));
>
> static inline int sk_forward_alloc_get(const struct sock *sk)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index f954d5893e79..be7b29d97637 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2338,17 +2338,6 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
> smp_wmb();
> refcount_set(&newsk->sk_refcnt, 2);
>
> - /* Increment the counter in the same struct proto as the master
> - * sock (sk_refcnt_debug_inc uses newsk->sk_prot->socks, that
> - * is the same as sk->sk_prot->socks, as this field was copied
> - * with memcpy).
> - *
> - * This _changes_ the previous behaviour, where
> - * tcp_create_openreq_child always was incrementing the
> - * equivalent to tcp_prot->socks (inet_sock_nr), so this have
> - * to be taken into account in all callers. -acme
> - */
> - sk_refcnt_debug_inc(newsk);
> sk_set_socket(newsk, NULL);
> sk_tx_queue_clear(newsk);
> RCU_INIT_POINTER(newsk->sk_wq, NULL);
> @@ -3696,8 +3685,6 @@ void sk_common_release(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> sock_put(sk);
> }
> EXPORT_SYMBOL(sk_common_release);
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 6c0ec2789943..f46a3924c440 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -156,7 +156,6 @@ void inet_sock_destruct(struct sock *sk)
> kfree(rcu_dereference_protected(inet->inet_opt, 1));
> dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
> dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
> - sk_refcnt_debug_dec(sk);
> }
> EXPORT_SYMBOL(inet_sock_destruct);
>
> @@ -356,8 +355,6 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
> inet->mc_list = NULL;
> inet->rcv_tos = 0;
>
> - sk_refcnt_debug_inc(sk);
> -
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> * the user to assign a number at socket
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index d1f837579398..64be59d93b04 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1178,8 +1178,6 @@ void inet_csk_destroy_sock(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> this_cpu_dec(*sk->sk_prot->orphan_count);
>
> sock_put(sk);
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index beed32fff484..40052414c7c7 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -77,9 +77,6 @@ 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
> - 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);
> module_put(owner);
> }
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index fee9163382c2..c93f2e865fea 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -238,16 +238,6 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
> inet->pmtudisc = IP_PMTUDISC_DONT;
> else
> inet->pmtudisc = IP_PMTUDISC_WANT;
> - /*
> - * Increment only the relevant sk_prot->socks debug field, this changes
> - * the previous behaviour of incrementing both the equivalent to
> - * answer->prot->socks (inet6_sock_nr) and inet_sock_nr.
> - *
> - * This allows better debug granularity as we'll know exactly how many
> - * UDPv6, TCPv6, etc socks were allocated, not the sum of all IPv6
> - * transport protocol socks. -acme
> - */
> - sk_refcnt_debug_inc(sk);
>
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 9ce51680290b..2917dd8d198c 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -464,13 +464,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> __ipv6_sock_mc_close(sk);
> __ipv6_sock_ac_close(sk);
>
> - /*
> - * Sock is moving from IPv6 to IPv4 (sk_prot), so
> - * remove it from the refcnt debug socks count in the
> - * original family...
> - */
> - sk_refcnt_debug_dec(sk);
> -
> if (sk->sk_protocol == IPPROTO_TCP) {
> struct inet_connection_sock *icsk = inet_csk(sk);
>
> @@ -507,11 +500,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>
> inet6_cleanup_sock(sk);
>
> - /*
> - * ... and add it to the refcnt debug socks count
> - * in the new family. -acme
> - */
> - sk_refcnt_debug_inc(sk);
> module_put(THIS_MODULE);
> retv = 0;
> break;
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8cd6cc67c2c5..e913752df112 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2876,7 +2876,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
> sk_stream_kill_queues(sk);
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
> }
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b5ab98ca2511..a4c8f86ac12a 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1335,8 +1335,6 @@ static void packet_sock_destruct(struct sock *sk)
> pr_err("Attempt to release alive packet socket: %p\n", sk);
> return;
> }
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static bool fanout_flow_is_huge(struct packet_sock *po, struct sk_buff *skb)
> @@ -3172,7 +3170,6 @@ static int packet_release(struct socket *sock)
>
> skb_queue_purge(&sk->sk_receive_queue);
> packet_free_pending(po);
> - sk_refcnt_debug_release(sk);
>
> sock_put(sk);
> return 0;
> @@ -3362,7 +3359,6 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> packet_cached_dev_reset(po);
>
> sk->sk_destruct = packet_sock_destruct;
> - sk_refcnt_debug_inc(sk);
>
> /*
> * Attach a protocol block
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 097bd60ce964..62b436a2c8fe 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -807,8 +807,6 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
>
> newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 909a89a1cff4..c365df24ad33 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -601,8 +601,6 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
>
> newinet->inet_daddr = asoc->peer.primary_addr.v4.sin_addr.s_addr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index e12d4fa5aece..c594312e22cd 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -359,8 +359,6 @@ static void smc_destruct(struct sock *sk)
> return;
> if (!sock_flag(sk, SOCK_DEAD))
> return;
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> @@ -389,7 +387,6 @@ static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> spin_lock_init(&smc->accept_q_lock);
> spin_lock_init(&smc->conn.send_lock);
> sk->sk_prot->hash(sk);
> - sk_refcnt_debug_inc(sk);
> mutex_init(&smc->clcsock_release_lock);
> smc_init_saved_callbacks(smc);
>
Acked-by: Wenjia Zhang <wenjia@linux.ibm.com>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 9f0561b67c12..a245c1b4a21b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -845,7 +845,6 @@ static int xsk_release(struct socket *sock)
> sock_orphan(sk);
> sock->sk = NULL;
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
>
> return 0;
> @@ -1396,8 +1395,6 @@ static void xsk_destruct(struct sock *sk)
>
> if (!xp_put_pool(xs->pool))
> xdp_put_umem(xs->umem, !xs->pool);
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static int xsk_create(struct net *net, struct socket *sock, int protocol,
> @@ -1427,7 +1424,6 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> sk->sk_family = PF_XDP;
>
> sk->sk_destruct = xsk_destruct;
> - sk_refcnt_debug_inc(sk);
>
> sock_set_flag(sk, SOCK_RCU_FREE);
>
On Tue, Feb 14, 2023 at 5:15 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Hi Jason,
On 14/02/2023 05:14, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
(...)
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8cd6cc67c2c5..e913752df112 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2876,7 +2876,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
> sk_stream_kill_queues(sk);
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
> }
>
Thank you for the patch!
For the modification in MPTCP subtree:
Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Cheers,
Matt
Hello:
This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:
On Tue, 14 Feb 2023 12:14:10 +0800 you wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> [...]
Here is the summary with links:
- [v2,net-next] net: no longer support SOCK_REFCNT_DEBUG feature
https://git.kernel.org/netdev/net-next/c/fe33311c3e37
You are awesome, thank you!
@@ -1349,9 +1349,6 @@ struct proto {
char name[32];
struct list_head node;
-#ifdef SOCK_REFCNT_DEBUG
- atomic_t socks;
-#endif
int (*diag_destroy)(struct sock *sk, int err);
} __randomize_layout;
@@ -1359,31 +1356,6 @@ 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
-static inline void sk_refcnt_debug_inc(struct sock *sk)
-{
- atomic_inc(&sk->sk_prot->socks);
-}
-
-static inline void sk_refcnt_debug_dec(struct sock *sk)
-{
- atomic_dec(&sk->sk_prot->socks);
- printk(KERN_DEBUG "%s socket %p released, %d are still alive\n",
- sk->sk_prot->name, sk, atomic_read(&sk->sk_prot->socks));
-}
-
-static inline void sk_refcnt_debug_release(const struct sock *sk)
-{
- if (refcount_read(&sk->sk_refcnt) != 1)
- 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 */
-#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 */
-
INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));
static inline int sk_forward_alloc_get(const struct sock *sk)
@@ -2338,17 +2338,6 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
smp_wmb();
refcount_set(&newsk->sk_refcnt, 2);
- /* Increment the counter in the same struct proto as the master
- * sock (sk_refcnt_debug_inc uses newsk->sk_prot->socks, that
- * is the same as sk->sk_prot->socks, as this field was copied
- * with memcpy).
- *
- * This _changes_ the previous behaviour, where
- * tcp_create_openreq_child always was incrementing the
- * equivalent to tcp_prot->socks (inet_sock_nr), so this have
- * to be taken into account in all callers. -acme
- */
- sk_refcnt_debug_inc(newsk);
sk_set_socket(newsk, NULL);
sk_tx_queue_clear(newsk);
RCU_INIT_POINTER(newsk->sk_wq, NULL);
@@ -3696,8 +3685,6 @@ void sk_common_release(struct sock *sk)
xfrm_sk_free_policy(sk);
- sk_refcnt_debug_release(sk);
-
sock_put(sk);
}
EXPORT_SYMBOL(sk_common_release);
@@ -156,7 +156,6 @@ void inet_sock_destruct(struct sock *sk)
kfree(rcu_dereference_protected(inet->inet_opt, 1));
dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
- sk_refcnt_debug_dec(sk);
}
EXPORT_SYMBOL(inet_sock_destruct);
@@ -356,8 +355,6 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
inet->mc_list = NULL;
inet->rcv_tos = 0;
- sk_refcnt_debug_inc(sk);
-
if (inet->inet_num) {
/* It assumes that any protocol which allows
* the user to assign a number at socket
@@ -1178,8 +1178,6 @@ void inet_csk_destroy_sock(struct sock *sk)
xfrm_sk_free_policy(sk);
- sk_refcnt_debug_release(sk);
-
this_cpu_dec(*sk->sk_prot->orphan_count);
sock_put(sk);
@@ -77,9 +77,6 @@ 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
- 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);
module_put(owner);
}
@@ -238,16 +238,6 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
inet->pmtudisc = IP_PMTUDISC_DONT;
else
inet->pmtudisc = IP_PMTUDISC_WANT;
- /*
- * Increment only the relevant sk_prot->socks debug field, this changes
- * the previous behaviour of incrementing both the equivalent to
- * answer->prot->socks (inet6_sock_nr) and inet_sock_nr.
- *
- * This allows better debug granularity as we'll know exactly how many
- * UDPv6, TCPv6, etc socks were allocated, not the sum of all IPv6
- * transport protocol socks. -acme
- */
- sk_refcnt_debug_inc(sk);
if (inet->inet_num) {
/* It assumes that any protocol which allows
@@ -464,13 +464,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
__ipv6_sock_mc_close(sk);
__ipv6_sock_ac_close(sk);
- /*
- * Sock is moving from IPv6 to IPv4 (sk_prot), so
- * remove it from the refcnt debug socks count in the
- * original family...
- */
- sk_refcnt_debug_dec(sk);
-
if (sk->sk_protocol == IPPROTO_TCP) {
struct inet_connection_sock *icsk = inet_csk(sk);
@@ -507,11 +500,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
inet6_cleanup_sock(sk);
- /*
- * ... and add it to the refcnt debug socks count
- * in the new family. -acme
- */
- sk_refcnt_debug_inc(sk);
module_put(THIS_MODULE);
retv = 0;
break;
@@ -2876,7 +2876,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
sk_stream_kill_queues(sk);
xfrm_sk_free_policy(sk);
- sk_refcnt_debug_release(sk);
sock_put(sk);
}
@@ -1335,8 +1335,6 @@ static void packet_sock_destruct(struct sock *sk)
pr_err("Attempt to release alive packet socket: %p\n", sk);
return;
}
-
- sk_refcnt_debug_dec(sk);
}
static bool fanout_flow_is_huge(struct packet_sock *po, struct sk_buff *skb)
@@ -3172,7 +3170,6 @@ static int packet_release(struct socket *sock)
skb_queue_purge(&sk->sk_receive_queue);
packet_free_pending(po);
- sk_refcnt_debug_release(sk);
sock_put(sk);
return 0;
@@ -3362,7 +3359,6 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
packet_cached_dev_reset(po);
sk->sk_destruct = packet_sock_destruct;
- sk_refcnt_debug_inc(sk);
/*
* Attach a protocol block
@@ -807,8 +807,6 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
- sk_refcnt_debug_inc(newsk);
-
if (newsk->sk_prot->init(newsk)) {
sk_common_release(newsk);
newsk = NULL;
@@ -601,8 +601,6 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
newinet->inet_daddr = asoc->peer.primary_addr.v4.sin_addr.s_addr;
- sk_refcnt_debug_inc(newsk);
-
if (newsk->sk_prot->init(newsk)) {
sk_common_release(newsk);
newsk = NULL;
@@ -359,8 +359,6 @@ static void smc_destruct(struct sock *sk)
return;
if (!sock_flag(sk, SOCK_DEAD))
return;
-
- sk_refcnt_debug_dec(sk);
}
static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
@@ -389,7 +387,6 @@ static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
spin_lock_init(&smc->accept_q_lock);
spin_lock_init(&smc->conn.send_lock);
sk->sk_prot->hash(sk);
- sk_refcnt_debug_inc(sk);
mutex_init(&smc->clcsock_release_lock);
smc_init_saved_callbacks(smc);
@@ -845,7 +845,6 @@ static int xsk_release(struct socket *sock)
sock_orphan(sk);
sock->sk = NULL;
- sk_refcnt_debug_release(sk);
sock_put(sk);
return 0;
@@ -1396,8 +1395,6 @@ static void xsk_destruct(struct sock *sk)
if (!xp_put_pool(xs->pool))
xdp_put_umem(xs->umem, !xs->pool);
-
- sk_refcnt_debug_dec(sk);
}
static int xsk_create(struct net *net, struct socket *sock, int protocol,
@@ -1427,7 +1424,6 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
sk->sk_family = PF_XDP;
sk->sk_destruct = xsk_destruct;
- sk_refcnt_debug_inc(sk);
sock_set_flag(sk, SOCK_RCU_FREE);