Skip to contentSkip to conversation

list_for_each_rcu must die: networking

PATCH New Paul E. McKenney 3 participants 4 messages Apr 24, 2008 – May 15, 2008

Subsystems (2)
Networking Networking drivers (wireless)
lore
Share & tools
lore

Paul E. McKenney

author
#0 lore raw

PATCH 1/1 list_for_each_rcu must die: networking

All uses of list_for_each_rcu() can be profitably replaced by the easier-to-use list_for_each_entry_rcu(). This patch makes this change for networking, in preparation for removing the list_for_each_rcu() API entirely.

Updated to remove the two now-unused variables as noted by Dave Miller.

Acked-by David S. Miller <[email protected]> (lkml.org/lkml/2008/4/23/448)

Signed_off_by: Paul E. McKenney <[email protected]>
---

 drivers/net/wireless/strip.c |    6 +++---
 net/802/psnap.c              |    4 +---
 net/ipv4/af_inet.c           |    4 +---
 net/ipv6/af_inet6.c          |    4 +---
 4 files changed, 6 insertions(+), 12 deletions(-)
drivers/net/wireless/strip.c +3 -3
diff -urpNa -X dontdiff linux-2.6.25/drivers/net/wireless/strip.c linux-2.6.25-lfer-net/drivers/net/wireless/strip.c
@@ -962,12 +962,12 @@ static char *time_delta(char buffer[], l
 /* get Nth element of the linked list */
 static struct strip *strip_get_idx(loff_t pos) 
 {
-	struct list_head *l;
+	struct strip *s;
 	int i = 0;
 
-	list_for_each_rcu(l, &strip_list) {
+	list_for_each_entry_rcu(s, &strip_list, list) {
 		if (pos == i)
-			return list_entry(l, struct strip, list);
+			return s;
 		++i;
 	}
 	return NULL;
net/802/psnap.c +1 -3
diff -urpNa -X dontdiff linux-2.6.25/net/802/psnap.c linux-2.6.25-lfer-net/net/802/psnap.c
@@ -30,11 +30,9 @@ static struct llc_sap *snap_sap;
  */
 static struct datalink_proto *find_snap_client(unsigned char *desc)
 {
-	struct list_head *entry;
 	struct datalink_proto *proto = NULL, *p;
 
-	list_for_each_rcu(entry, &snap_list) {
-		p = list_entry(entry, struct datalink_proto, node);
+	list_for_each_entry_rcu(p, &snap_list, node) {
 		if (!memcmp(p->type, desc, 5)) {
 			proto = p;
 			break;
net/ipv4/af_inet.c +1 -3
diff -urpNa -X dontdiff linux-2.6.25/net/ipv4/af_inet.c linux-2.6.25-lfer-net/net/ipv4/af_inet.c
@@ -250,7 +250,6 @@ EXPORT_SYMBOL(build_ehash_secret);
 static int inet_create(struct net *net, struct socket *sock, int protocol)
 {
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct inet_sock *inet;
 	struct proto *answer_prot;

@@ -274,8 +273,7 @@ static int inet_create(struct net *net, 
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw[sock->type], list) {
 
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
net/ipv6/af_inet6.c +1 -3
diff -urpNa -X dontdiff linux-2.6.25/net/ipv6/af_inet6.c linux-2.6.25-lfer-net/net/ipv6/af_inet6.c
@@ -84,7 +84,6 @@ static int inet6_create(struct net *net,
 	struct inet_sock *inet;
 	struct ipv6_pinfo *np;
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct proto *answer_prot;
 	unsigned char answer_flags;

@@ -105,8 +104,7 @@ static int inet6_create(struct net *net,
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw6[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw6[sock->type], list) {
 
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
author
#1 lore raw

PATCH 1/1 list_for_each_rcu must die: networking

All uses of list_for_each_rcu() can be profitably replaced by the easier-to-use list_for_each_entry_rcu(). This patch makes this change for networking, in preparation for removing the list_for_each_rcu() API entirely.

Updated to remove the two now-unused variables as noted by Dave Miller, and also to fix my bonehead error detected by Kamalesh Babulal and Alexey Dobriyan. It now passes LTP on POWER. And also has valid SOB. Some days it just doesn't pay to get out of bed...

Acked-by David S. Miller <[email protected]> (lkml.org/lkml/2008/4/23/448)
Signed-off-by Paul E. McKenney <[email protected]>

---

 802/psnap.c     |    4 +---
 ipv4/af_inet.c  |    9 +++------
 ipv6/af_inet6.c |    9 +++------
 3 files changed, 7 insertions(+), 15 deletions(-)
net/802/psnap.c +1 -3
diff -urpNa -X dontdiff linux-2.6.26-rc2/net/802/psnap.c linux-2.6.26-rc2-lfer-net/net/802/psnap.c
@@ -30,11 +30,9 @@ static struct llc_sap *snap_sap;
  */
 static struct datalink_proto *find_snap_client(unsigned char *desc)
 {
-	struct list_head *entry;
 	struct datalink_proto *proto = NULL, *p;
 
-	list_for_each_rcu(entry, &snap_list) {
-		p = list_entry(entry, struct datalink_proto, node);
+	list_for_each_entry_rcu(p, &snap_list, node) {
 		if (!memcmp(p->type, desc, 5)) {
 			proto = p;
 			break;
net/ipv4/af_inet.c +3 -6
diff -urpNa -X dontdiff linux-2.6.26-rc2/net/ipv4/af_inet.c linux-2.6.26-rc2-lfer-net/net/ipv4/af_inet.c
@@ -267,7 +267,6 @@ static inline int inet_netns_ok(struct n
 static int inet_create(struct net *net, struct socket *sock, int protocol)
 {
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct inet_sock *inet;
 	struct proto *answer_prot;

@@ -284,13 +283,12 @@ static int inet_create(struct net *net, 
 	sock->state = SS_UNCONNECTED;
 
 	/* Look for the requested type/protocol pair. */
-	answer = NULL;
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw[sock->type], list) {
 
+		err = 0;
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
 			if (protocol != IPPROTO_IP)

@@ -305,10 +303,9 @@ lookup_protocol:
 				break;
 		}
 		err = -EPROTONOSUPPORT;
-		answer = NULL;
 	}
 
-	if (unlikely(answer == NULL)) {
+	if (unlikely(err)) {
 		if (try_loading_module < 2) {
 			rcu_read_unlock();
 			/*
net/ipv6/af_inet6.c +3 -6
diff -urpNa -X dontdiff linux-2.6.26-rc2/net/ipv6/af_inet6.c linux-2.6.26-rc2-lfer-net/net/ipv6/af_inet6.c
@@ -87,7 +87,6 @@ static int inet6_create(struct net *net,
 	struct inet_sock *inet;
 	struct ipv6_pinfo *np;
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct proto *answer_prot;
 	unsigned char answer_flags;

@@ -101,13 +100,12 @@ static int inet6_create(struct net *net,
 		build_ehash_secret();
 
 	/* Look for the requested type/protocol pair. */
-	answer = NULL;
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw6[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw6[sock->type], list) {
 
+		err = 0;
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
 			if (protocol != IPPROTO_IP)

@@ -122,10 +120,9 @@ lookup_protocol:
 				break;
 		}
 		err = -EPROTONOSUPPORT;
-		answer = NULL;
 	}
 
-	if (!answer) {
+	if (err) {
 		if (try_loading_module < 2) {
 			rcu_read_unlock();
 			/*
Tested-by on v1 1/1
#2 lore raw
Quoted 9 lines · Paul E. McKenney

All uses of list_for_each_rcu() can be profitably replaced by the easier-to-use list_for_each_entry_rcu(). This patch makes this change for networking, in preparation for removing the list_for_each_rcu() API entirely.

Updated to remove the two now-unused variables as noted by Dave Miller, and also to fix my bonehead error detected by Kamalesh Babulal and Alexey Dobriyan. It now passes LTP on POWER. And also has valid SOB. Some days it just doesn't pay to get out of bed...

Hi Paul,

Thanks, the patch fixes the oops.

Tested-by Kamalesh Babulal <[email protected]>
Quoted 104 lines
Acked-by David S. Miller <[email protected]> (lkml.org/lkml/2008/4/23/448)
Signed-off-by Paul E. McKenney <[email protected]>

---

 802/psnap.c     |    4 +---
 ipv4/af_inet.c  |    9 +++------
 ipv6/af_inet6.c |    9 +++------
 3 files changed, 7 insertions(+), 15 deletions(-)

diff -urpNa -X dontdiff linux-2.6.26-rc2/net/802/psnap.c linux-2.6.26-rc2-lfer-net/net/802/psnap.c

net/802/psnap.c +1 -3
@@ -30,11 +30,9 @@ static struct llc_sap *snap_sap;
  */
 static struct datalink_proto *find_snap_client(unsigned char *desc)
 {
-	struct list_head *entry;
 	struct datalink_proto *proto = NULL, *p;
 
-	list_for_each_rcu(entry, &snap_list) {
-		p = list_entry(entry, struct datalink_proto, node);
+	list_for_each_entry_rcu(p, &snap_list, node) {
 		if (!memcmp(p->type, desc, 5)) {
 			proto = p;
 			break;

diff -urpNa -X dontdiff linux-2.6.26-rc2/net/ipv4/af_inet.c linux-2.6.26-rc2-lfer-net/net/ipv4/af_inet.c

net/ipv4/af_inet.c +3 -6
@@ -267,7 +267,6 @@ static inline int inet_netns_ok(struct n
 static int inet_create(struct net *net, struct socket *sock, int protocol)
 {
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct inet_sock *inet;
 	struct proto *answer_prot;

@@ -284,13 +283,12 @@ static int inet_create(struct net *net, 
 	sock->state = SS_UNCONNECTED;
 
 	/* Look for the requested type/protocol pair. */
-	answer = NULL;
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw[sock->type], list) {
 
+		err = 0;
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
 			if (protocol != IPPROTO_IP)

@@ -305,10 +303,9 @@ lookup_protocol:
 				break;
 		}
 		err = -EPROTONOSUPPORT;
-		answer = NULL;
 	}
 
-	if (unlikely(answer == NULL)) {
+	if (unlikely(err)) {
 		if (try_loading_module < 2) {
 			rcu_read_unlock();
 			/*

diff -urpNa -X dontdiff linux-2.6.26-rc2/net/ipv6/af_inet6.c linux-2.6.26-rc2-lfer-net/net/ipv6/af_inet6.c

net/ipv6/af_inet6.c +3 -7
@@ -87,7 +87,6 @@ static int inet6_create(struct net *net,
 	struct inet_sock *inet;
 	struct ipv6_pinfo *np;
 	struct sock *sk;
-	struct list_head *p;
 	struct inet_protosw *answer;
 	struct proto *answer_prot;
 	unsigned char answer_flags;

@@ -101,13 +100,12 @@ static int inet6_create(struct net *net,
 		build_ehash_secret();
 
 	/* Look for the requested type/protocol pair. */
-	answer = NULL;
 lookup_protocol:
 	err = -ESOCKTNOSUPPORT;
 	rcu_read_lock();
-	list_for_each_rcu(p, &inetsw6[sock->type]) {
-		answer = list_entry(p, struct inet_protosw, list);
+	list_for_each_entry_rcu(answer, &inetsw6[sock->type], list) {
 
+		err = 0;
 		/* Check the non-wild match. */
 		if (protocol == answer->protocol) {
 			if (protocol != IPPROTO_IP)

@@ -122,10 +120,9 @@ lookup_protocol:
 				break;
 		}
 		err = -EPROTONOSUPPORT;
-		answer = NULL;
 	}
 
-	if (!answer) {
+	if (err) {
 		if (try_loading_module < 2) {
 			rcu_read_unlock();
 			/*
--
Signature · 4 lines
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.
maintainer
#3 lore raw

From: Kamalesh Babulal <[email protected]>
Date: Thu, 15 May 2008 10:56:07 +0530

Quoted 11 lines
Quoted 9 lines · Paul E. McKenney

All uses of list_for_each_rcu() can be profitably replaced by the easier-to-use list_for_each_entry_rcu(). This patch makes this change for networking, in preparation for removing the list_for_each_rcu() API entirely.

Updated to remove the two now-unused variables as noted by Dave Miller, and also to fix my bonehead error detected by Kamalesh Babulal and Alexey Dobriyan. It now passes LTP on POWER. And also has valid SOB. Some days it just doesn't pay to get out of bed...

Quoted 7 lines

Hi Paul,

Thanks, the patch fixes the oops.

Tested-by Kamalesh Babulal <[email protected]>
Quoted 2 lines
Acked-by David S. Miller <[email protected]> (lkml.org/lkml/2008/4/23/448)
Signed-off-by Paul E. McKenney <[email protected]>

I'm fine with these changes, and I acked them already. They belong as a set with the other list_for_each_rcu killing patches, so I'm fine with them going all together in Andrew's tree.

Thanks!