* Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
2023-02-14 4:14 [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature Jason Xing
@ 2023-02-15 4:56 ` Kuniyuki Iwashima
2023-02-15 6:32 ` Wenjia Zhang
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-15 4:56 UTC (permalink / raw)
To: kerneljasonxing
Cc: bjorn, bpf, davem, dsahern, edumazet, jaka, jonathan.lemon,
kernelxing, kgraul, kuba, kuniyu, linux-kernel, linux-s390,
linux-sctp, lucien.xin, maciej.fijalkowski, magnus.karlsson,
marcelo.leitner, matthieu.baerts, mptcp, netdev, nhorman, pabeni,
wenjia, willemdebruijn.kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
2023-02-14 4:14 [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature Jason Xing
2023-02-15 4:56 ` Kuniyuki Iwashima
@ 2023-02-15 6:32 ` Wenjia Zhang
2023-02-15 8:34 ` Eric Dumazet
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Wenjia Zhang @ 2023-02-15 6:32 UTC (permalink / raw)
To: Jason Xing, kuniyu, davem, edumazet, kuba, pabeni, dsahern,
matthieu.baerts, willemdebruijn.kernel, nhorman, marcelo.leitner,
lucien.xin, kgraul, jaka, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon
Cc: bpf, linux-s390, linux-sctp, mptcp, netdev, linux-kernel,
Jason Xing
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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
2023-02-14 4:14 [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature Jason Xing
2023-02-15 4:56 ` Kuniyuki Iwashima
2023-02-15 6:32 ` Wenjia Zhang
@ 2023-02-15 8:34 ` Eric Dumazet
2023-02-15 9:40 ` Matthieu Baerts
2023-02-15 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2023-02-15 8:34 UTC (permalink / raw)
To: Jason Xing
Cc: kuniyu, davem, kuba, pabeni, dsahern, matthieu.baerts,
willemdebruijn.kernel, nhorman, marcelo.leitner, lucien.xin,
kgraul, wenjia, jaka, bjorn, magnus.karlsson, maciej.fijalkowski,
jonathan.lemon, bpf, linux-s390, linux-sctp, mptcp, netdev,
linux-kernel, Jason Xing
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
2023-02-14 4:14 [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature Jason Xing
` (2 preceding siblings ...)
2023-02-15 8:34 ` Eric Dumazet
@ 2023-02-15 9:40 ` Matthieu Baerts
2023-02-15 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2023-02-15 9:40 UTC (permalink / raw)
To: Jason Xing, kuniyu, davem, edumazet, kuba, pabeni, dsahern,
willemdebruijn.kernel, nhorman, marcelo.leitner, lucien.xin,
kgraul, wenjia, jaka, bjorn, magnus.karlsson, maciej.fijalkowski,
jonathan.lemon
Cc: bpf, linux-s390, linux-sctp, mptcp, netdev, linux-kernel,
Jason Xing
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
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
2023-02-14 4:14 [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature Jason Xing
` (3 preceding siblings ...)
2023-02-15 9:40 ` Matthieu Baerts
@ 2023-02-15 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-02-15 10:30 UTC (permalink / raw)
To: Jason Xing
Cc: kuniyu, davem, edumazet, kuba, pabeni, dsahern, matthieu.baerts,
willemdebruijn.kernel, nhorman, marcelo.leitner, lucien.xin,
kgraul, wenjia, jaka, bjorn, magnus.karlsson, maciej.fijalkowski,
jonathan.lemon, bpf, linux-s390, linux-sctp, mptcp, netdev,
linux-kernel, kernelxing
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread