netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
To: "David S. Miller" <davem@redhat.com>
Cc: Linux Networking Development Mailing List <netdev@oss.sgi.com>
Subject: [RFC] taking advantage of sk_{add,del}_node
Date: Tue, 17 Jun 2003 22:32:15 -0300	[thread overview]
Message-ID: <20030618013214.GC17694@conectiva.com.br> (raw)

Hi,

	While doing the hlist work I noticed that it may be a good idea to have
the sock_{hold,put} done automatically in most (all?) sk_{add,del}_node, as
we're adding the sock to a list, we must hold a reference, so take a look at
this (untested, with some issues) patch and please give comments about it.

	Doing this we'll have some protocols that doesn't do any refcounting
at all suck less, and make it easier to fill the gap, as I have an idea for
the next step for a sk_search(list, callback_cmp_func), done as a macro
and using the callback_cmp_func as an inline, that would do the sock_hold on
finding the sock when callback_cmp_func returns true (think about a variation
on bsearch(3) idea...).

	With those thingies in place writing a protocol will be a piece of
cake and we'll have less differences among network families implementations 8)

Thanks,

- Arnaldo

===== include/net/sock.h 1.45 vs edited =====
--- 1.45/include/net/sock.h	Tue Jun 17 13:35:27 2003
+++ edited/include/net/sock.h	Tue Jun 17 22:01:57 2003
@@ -289,7 +289,7 @@
 	node->pprev = NULL;
 }
 
-static __inline__ int sk_del_node_init(struct sock *sk)
+static __inline__ int __sk_del_node_init(struct sock *sk)
 {
 	if (sk_hashed(sk)) {
 		__hlist_del(&sk->sk_node);
@@ -299,8 +299,23 @@
 	return 0;
 }
 
+static __inline__ int sk_del_node_init(struct sock *sk)
+{
+	int rc = __sk_del_node_init(sk);
+
+	if (rc)
+		__sock_put(sk);
+	return rc;
+}
+
+static __inline__ void __sk_add_node(struct sock *sk, struct hlist_head *list)
+{
+	hlist_add_head(&sk->sk_node, list);
+}
+
 static __inline__ void sk_add_node(struct sock *sk, struct hlist_head *list)
 {
+	sock_hold(sk);
 	hlist_add_head(&sk->sk_node, list);
 }
 
===== net/bluetooth/af_bluetooth.c 1.20 vs edited =====
--- 1.20/net/bluetooth/af_bluetooth.c	Mon Jun 16 12:11:36 2003
+++ edited/net/bluetooth/af_bluetooth.c	Tue Jun 17 22:03:33 2003
@@ -143,15 +143,13 @@
 {
 	write_lock_bh(&l->lock);
 	sk_add_node(sk, &l->head);
-	sock_hold(sk);
 	write_unlock_bh(&l->lock);
 }
 
 void bt_sock_unlink(struct bt_sock_list *l, struct sock *sk)
 {
 	write_lock_bh(&l->lock);
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk)
 	write_unlock_bh(&l->lock);
 }
 
===== net/decnet/af_decnet.c 1.29 vs edited =====
--- 1.29/net/decnet/af_decnet.c	Mon Jun 16 12:11:36 2003
+++ edited/net/decnet/af_decnet.c	Tue Jun 17 22:04:17 2003
@@ -276,7 +276,7 @@
 		return;
 
 	write_lock_bh(&dn_hash_lock);
-	hlist_del(&sk->sk_node);
+	sk_del_node_init(&sk->sk_node);
 	DN_SK(sk)->addrloc = 0;
 	list = listen_hash(&DN_SK(sk)->addr);
 	sk_add_node(sk, list);
===== net/econet/af_econet.c 1.21 vs edited =====
--- 1.21/net/econet/af_econet.c	Mon Jun 16 12:11:36 2003
+++ edited/net/econet/af_econet.c	Tue Jun 17 22:04:31 2003
@@ -96,8 +96,7 @@
 static void econet_remove_socket(struct hlist_head *list, struct sock *sk)
 {
 	write_lock_bh(&econet_lock);
-	if (sk_del_node_init(sk))
-		sock_put(sk);
+	sk_del_node_init(sk)
 	write_unlock_bh(&econet_lock);
 }
 
@@ -105,7 +104,6 @@
 {
 	write_lock_bh(&econet_lock);
 	sk_add_node(sk, list);
-	sock_hold(sk);
 	write_unlock_bh(&econet_lock);
 }
 
===== net/ipv4/raw.c 1.34 vs edited =====
--- 1.34/net/ipv4/raw.c	Mon Jun 16 12:11:36 2003
+++ edited/net/ipv4/raw.c	Tue Jun 17 22:04:38 2003
@@ -91,17 +91,14 @@
 	write_lock_bh(&raw_v4_lock);
 	sk_add_node(sk, head);
 	sock_prot_inc_use(sk->sk_prot);
- 	sock_hold(sk);
 	write_unlock_bh(&raw_v4_lock);
 }
 
 static void raw_v4_unhash(struct sock *sk)
 {
  	write_lock_bh(&raw_v4_lock);
-	if (sk_del_node_init(sk)) {
+	if (sk_del_node_init(sk))
 		sock_prot_dec_use(sk->sk_prot);
-		__sock_put(sk);
-	}
 	write_unlock_bh(&raw_v4_lock);
 }
 
===== net/ipv4/tcp_ipv4.c 1.63 vs edited =====
--- 1.63/net/ipv4/tcp_ipv4.c	Tue Jun 17 13:35:27 2003
+++ edited/net/ipv4/tcp_ipv4.c	Tue Jun 17 22:05:38 2003
@@ -359,7 +359,7 @@
 		lock = &tcp_ehash[sk->sk_hashent].lock;
 		write_lock(lock);
 	}
-	sk_add_node(sk, list);
+	__sk_add_node(sk, list);
 	sock_prot_inc_use(sk->sk_prot);
 	write_unlock(lock);
 	if (listen_possible && sk->sk_state == TCP_LISTEN)
@@ -392,7 +392,7 @@
 		write_lock_bh(&head->lock);
 	}
 
-	if (sk_del_node_init(sk))
+	if (__sk_del_node_init(sk))
 		sock_prot_dec_use(sk->sk_prot);
 	write_unlock_bh(lock);
 
@@ -608,7 +608,7 @@
 	inet->sport = htons(lport);
 	sk->sk_hashent = hash;
 	BUG_TRAP(sk_unhashed(sk));
-	sk_add_node(sk, &head->chain);
+	__sk_add_node(sk, &head->chain);
 	sock_prot_inc_use(sk->sk_prot);
 	write_unlock(&head->lock);
 
===== net/ipv4/udp.c 1.43 vs edited =====
--- 1.43/net/ipv4/udp.c	Mon Jun 16 12:11:36 2003
+++ edited/net/ipv4/udp.c	Tue Jun 17 22:04:45 2003
@@ -189,7 +189,6 @@
 
 		sk_add_node(sk, h);
 		sock_prot_inc_use(sk->sk_prot);
-		sock_hold(sk);
 	}
 	write_unlock_bh(&udp_hash_lock);
 	return 0;
@@ -210,7 +209,6 @@
 	if (sk_del_node_init(sk)) {
 		inet_sk(sk)->num = 0;
 		sock_prot_dec_use(sk->sk_prot);
-		__sock_put(sk);
 	}
 	write_unlock_bh(&udp_hash_lock);
 }
===== net/ipv6/raw.c 1.31 vs edited =====
--- 1.31/net/ipv6/raw.c	Mon Jun 16 12:11:36 2003
+++ edited/net/ipv6/raw.c	Tue Jun 17 22:06:08 2003
@@ -64,17 +64,14 @@
 	write_lock_bh(&raw_v6_lock);
 	sk_add_node(sk, list);
 	sock_prot_inc_use(sk->sk_prot);
- 	sock_hold(sk);
  	write_unlock_bh(&raw_v6_lock);
 }
 
 static void raw_v6_unhash(struct sock *sk)
 {
  	write_lock_bh(&raw_v6_lock);
-	if (sk_del_node_init(sk)) {
+	if (sk_del_node_init(sk))
 		sock_prot_dec_use(sk->sk_prot);
-		__sock_put(sk);
-	}
 	write_unlock_bh(&raw_v6_lock);
 }
 
===== net/ipv6/udp.c 1.41 vs edited =====
--- 1.41/net/ipv6/udp.c	Mon Jun 16 12:24:58 2003
+++ edited/net/ipv6/udp.c	Tue Jun 17 22:06:13 2003
@@ -160,7 +160,6 @@
 	if (sk_unhashed(sk)) {
 		sk_add_node(sk, &udp_hash[snum & (UDP_HTABLE_SIZE - 1)]);
 		sock_prot_inc_use(sk->sk_prot);
-		sock_hold(sk);
 	}
 	write_unlock_bh(&udp_hash_lock);
 	return 0;
@@ -181,7 +180,6 @@
 	if (sk_del_node_init(sk)) {
 		inet_sk(sk)->num = 0;
 		sock_prot_dec_use(sk->sk_prot);
-		__sock_put(sk);
 	}
 	write_unlock_bh(&udp_hash_lock);
 }
===== net/ipx/af_ipx.c 1.38 vs edited =====
--- 1.38/net/ipx/af_ipx.c	Tue Jun 17 14:11:58 2003
+++ edited/net/ipx/af_ipx.c	Tue Jun 17 22:06:30 2003
@@ -142,7 +142,6 @@
 	spin_lock_bh(&intrfc->if_sklist_lock);
 	sk_del_node_init(sk);
 	spin_unlock_bh(&intrfc->if_sklist_lock);
-	sock_put(sk);
 	ipxitf_put(intrfc);
 out:
 	return;
@@ -229,7 +228,6 @@
 static void ipxitf_insert_socket(struct ipx_interface *intrfc, struct sock *sk)
 {
 	ipxitf_hold(intrfc);
-	sock_hold(sk);
 	spin_lock_bh(&intrfc->if_sklist_lock);
 	ipx_sk(sk)->intrfc = intrfc;
 	sk_add_node(sk, &intrfc->if_sklist);
===== net/key/af_key.c 1.42 vs edited =====
--- 1.42/net/key/af_key.c	Mon Jun 16 12:11:36 2003
+++ edited/net/key/af_key.c	Tue Jun 17 22:06:34 2003
@@ -115,15 +115,13 @@
 {
 	pfkey_table_grab();
 	sk_add_node(sk, &pfkey_table);
-	sock_hold(sk);
 	pfkey_table_ungrab();
 }
 
 static void pfkey_remove(struct sock *sk)
 {
 	pfkey_table_grab();
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	pfkey_table_ungrab();
 }
 
===== net/llc/llc_sap.c 1.20 vs edited =====
--- 1.20/net/llc/llc_sap.c	Mon Jun 16 12:11:36 2003
+++ edited/net/llc/llc_sap.c	Tue Jun 17 22:06:40 2003
@@ -35,7 +35,6 @@
 	write_lock_bh(&sap->sk_list.lock);
 	llc_sk(sk)->sap = sap;
 	sk_add_node(sk, &sap->sk_list.list);
-	sock_hold(sk);
 	write_unlock_bh(&sap->sk_list.lock);
 }
 
@@ -50,8 +49,7 @@
 void llc_sap_unassign_sock(struct llc_sap *sap, struct sock *sk)
 {
 	write_lock_bh(&sap->sk_list.lock);
-	if (sk_del_node_init(sk))
-		sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock_bh(&sap->sk_list.lock);
 }
 
===== net/netlink/af_netlink.c 1.28 vs edited =====
--- 1.28/net/netlink/af_netlink.c	Mon Jun 16 12:11:36 2003
+++ edited/net/netlink/af_netlink.c	Tue Jun 17 22:06:45 2003
@@ -193,7 +193,6 @@
 		if (nlk_sk(sk)->pid == 0) {
 			nlk_sk(sk)->pid = pid;
 			sk_add_node(sk, &nl_table[sk->sk_protocol]);
-			sock_hold(sk);
 			err = 0;
 		}
 	}
@@ -204,8 +203,7 @@
 static void netlink_remove(struct sock *sk)
 {
 	netlink_table_grab();
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	netlink_table_ungrab();
 }
 
===== net/packet/af_packet.c 1.30 vs edited =====
--- 1.30/net/packet/af_packet.c	Mon Jun 16 12:11:36 2003
+++ edited/net/packet/af_packet.c	Tue Jun 17 22:06:51 2003
@@ -758,8 +758,7 @@
 		return 0;
 
 	write_lock_bh(&packet_sklist_lock);
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock_bh(&packet_sklist_lock);
 
 	/*
@@ -984,7 +983,6 @@
 
 	write_lock_bh(&packet_sklist_lock);
 	sk_add_node(sk, &packet_sklist);
-	sock_hold(sk);
 	write_unlock_bh(&packet_sklist_lock);
 	return(0);
 
===== net/unix/af_unix.c 1.48 vs edited =====
--- 1.48/net/unix/af_unix.c	Mon Jun 16 16:46:51 2003
+++ edited/net/unix/af_unix.c	Tue Jun 17 22:06:58 2003
@@ -211,15 +211,13 @@
 
 static void __unix_remove_socket(struct sock *sk)
 {
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 }
 
 static void __unix_insert_socket(struct hlist_head *list, struct sock *sk)
 {
 	BUG_TRAP(sk_unhashed(sk));
 	sk_add_node(sk, list);
-	sock_hold(sk);
 }
 
 static inline void unix_remove_socket(struct sock *sk)
===== net/wanrouter/af_wanpipe.c 1.27 vs edited =====
--- 1.27/net/wanrouter/af_wanpipe.c	Mon Jun 16 12:11:36 2003
+++ edited/net/wanrouter/af_wanpipe.c	Tue Jun 17 22:07:04 2003
@@ -982,8 +982,7 @@
 
 	set_bit(1,&wanpipe_tx_critical);
 	write_lock(&wanpipe_sklist_lock);
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock(&wanpipe_sklist_lock);
 	clear_bit(1,&wanpipe_tx_critical);
 
@@ -1143,8 +1142,7 @@
 	}
 	
 	write_lock(&wanpipe_sklist_lock);
-	if (sk_del_node_init(sk))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock(&wanpipe_sklist_lock);
 
 
@@ -1206,8 +1204,7 @@
 	 * appropriate locks */
 	
 	write_lock(&wanpipe_sklist_lock);
-	if (sk_del_node_init(init))
-		__sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock(&wanpipe_sklist_lock);
 
 	sk->sk_socket = NULL;
@@ -1536,7 +1533,6 @@
 	set_bit(1,&wanpipe_tx_critical);
 	write_lock(&wanpipe_sklist_lock);
 	sk_add_node(sk, &wanpipe_sklist);
-	sock_hold(sk);
 	write_unlock(&wanpipe_sklist_lock);
 	clear_bit(1,&wanpipe_tx_critical);
 
@@ -2434,7 +2430,6 @@
 	set_bit(1,&wanpipe_tx_critical);
 	write_lock(&wanpipe_sklist_lock);
 	sk_add_node(newsk, &wanpipe_sklist);
-	sock_hold(sk);
 	write_unlock(&wanpipe_sklist_lock);
 	clear_bit(1,&wanpipe_tx_critical);
 
===== net/x25/af_x25.c 1.28 vs edited =====
--- 1.28/net/x25/af_x25.c	Mon Jun 16 12:11:36 2003
+++ edited/net/x25/af_x25.c	Tue Jun 17 22:07:26 2003
@@ -154,8 +154,7 @@
 static void x25_remove_socket(struct sock *sk)
 {
 	write_lock_bh(&x25_list_lock);
-	if (sk_del_node_init(sk))
-		sock_put(sk);
+	sk_del_node_init(sk);
 	write_unlock_bh(&x25_list_lock);
 }
 
@@ -219,7 +218,6 @@
 {
 	write_lock_bh(&x25_list_lock);
 	sk_add_node(sk, &x25_list);
-	sock_hold(sk);
 	write_unlock_bh(&x25_list_lock);
 }
 

                 reply	other threads:[~2003-06-18  1:32 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20030618013214.GC17694@conectiva.com.br \
    --to=acme@conectiva.com.br \
    --cc=davem@redhat.com \
    --cc=netdev@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).