[patch 1/7] net_device list cleanup: core

[patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

Cleanup of net_device list use in net_dev core and IP.
The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
Signed-off-by: Kirill Korotaev <dev@openvz.org>
---
 include/linux/netdevice.h |   29 ++++++++++++++++++++++++++-
 net/core/dev.c            |   48 +++++++++++++++++++++++++---------------------
 net/ipv4/devinet.c        |    6 ++---
 net/ipv6/addrconf.c       |    8 +++----
 net/ipv6/anycast.c        |   10 +++++----
 5 files changed, 68 insertions(+), 33 deletions(-)

--- ./include/linux/netdevice.h.vedevbase-core	Mon Jul  3 15:14:15 2006
+++ ./include/linux/netdevice.h	Mon Jul  3 16:09:11 2006
@@ -290,7 +290,8 @@ struct net_device
 	unsigned long		state;
 
 	struct net_device	*next;
-	
+	struct list_head	dev_list;
+
 	/* The device initialization function. Called only once. */
 	int			(*init)(struct net_device *dev);
 
@@ -558,8 +559,34 @@ struct packet_type {
 
 extern struct net_device		loopback_dev;		/* The loopback */
 extern struct net_device		*dev_base;		/* All devices */
+extern struct list_head			dev_base_head;		/* All devices */
 extern rwlock_t				dev_base_lock;		/* Device list lock */
 
+#define for_each_netdev(p)	list_for_each_entry(p, &dev_base_head, dev_list)
+
+/*
+ * When possible, it is preferrable to use for_each_netdev() loop
+ * defined above, rather than first_netdev()/next_netdev() macros.
+ * for_each_netdev() loop makes the intentions clearer, and gives more
+ * flexibility in device list implementation.
+ * While next_netdev() is unavoidable in seq_proc functions,
+ * first_netdev() should be needed quite rarely.
+ */
+#define first_netdev()		({ \
+					list_empty(&dev_base_head) ? NULL : \
+						list_entry(dev_base_head.next, \
+							struct net_device, \
+							dev_list); \
+				 })
+#define next_netdev(dev)	({ \
+					struct list_head *__next; \
+					__next = (dev)->dev_list.next; \
+					__next == &dev_base_head ? NULL : \
+						list_entry(__next, \
+							struct net_device, \
+							dev_list); \
+				 })
+
 extern int 			netdev_boot_setup_check(struct net_device *dev);
 extern unsigned long		netdev_boot_base(const char *prefix, int unit);
 extern struct net_device    *dev_getbyhwaddr(unsigned short type, char *hwaddr);
--- ./net/core/dev.c.vedevbase-core	Mon Jul  3 15:14:19 2006
+++ ./net/core/dev.c	Mon Jul  3 16:09:11 2006
@@ -181,6 +181,9 @@ DEFINE_RWLOCK(dev_base_lock);
 EXPORT_SYMBOL(dev_base);
 EXPORT_SYMBOL(dev_base_lock);
 
+LIST_HEAD(dev_base_head);
+EXPORT_SYMBOL(dev_base_head);
+
 #define NETDEV_HASHBITS	8
 static struct hlist_head dev_name_head[1<<NETDEV_HASHBITS];
 static struct hlist_head dev_index_head[1<<NETDEV_HASHBITS];
@@ -575,11 +578,11 @@ struct net_device *dev_getbyhwaddr(unsig
 
 	ASSERT_RTNL();
 
-	for (dev = dev_base; dev; dev = dev->next)
+	for_each_netdev(dev)
 		if (dev->type == type &&
 		    !memcmp(dev->dev_addr, ha, dev->addr_len))
-			break;
-	return dev;
+			return dev;
+	return NULL;
 }
 
 EXPORT_SYMBOL(dev_getbyhwaddr);
@@ -589,14 +592,15 @@ struct net_device *dev_getfirstbyhwtype(
 	struct net_device *dev;
 
 	rtnl_lock();
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		if (dev->type == type) {
 			dev_hold(dev);
-			break;
+			rtnl_unlock();
+			return dev;
 		}
 	}
 	rtnl_unlock();
-	return dev;
+	return NULL;
 }
 
 EXPORT_SYMBOL(dev_getfirstbyhwtype);
@@ -617,14 +621,15 @@ struct net_device * dev_get_by_flags(uns
 	struct net_device *dev;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if (((dev->flags ^ if_flags) & mask) == 0) {
 			dev_hold(dev);
-			break;
+			read_unlock(&dev_base_lock);
+			return dev;
 		}
 	}
 	read_unlock(&dev_base_lock);
-	return dev;
+	return NULL;
 }
 
 /**
@@ -680,7 +685,7 @@ int dev_alloc_name(struct net_device *de
 		if (!inuse)
 			return -ENOMEM;
 
-		for (d = dev_base; d; d = d->next) {
+		for_each_netdev(d) {
 			if (!sscanf(d->name, name, &i))
 				continue;
 			if (i < 0 || i >= max_netdevices)
@@ -966,7 +971,7 @@ int register_netdevice_notifier(struct n
 	rtnl_lock();
 	err = raw_notifier_chain_register(&netdev_chain, nb);
 	if (!err) {
-		for (dev = dev_base; dev; dev = dev->next) {
+		for_each_netdev(dev) {
 			nb->notifier_call(nb, NETDEV_REGISTER, dev);
 
 			if (dev->flags & IFF_UP) 
@@ -2035,7 +2040,7 @@ static int dev_ifconf(char __user *arg)
 	 */
 
 	total = 0;
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		for (i = 0; i < NPROTO; i++) {
 			if (gifconf_list[i]) {
 				int done;
@@ -2974,6 +2979,7 @@ int register_netdevice(struct net_device
 	write_lock_bh(&dev_base_lock);
 	*dev_tail = dev;
 	dev_tail = &dev->next;
+	list_add_tail(&dev->dev_list, &dev_base_head);
 	hlist_add_head(&dev->name_hlist, head);
 	hlist_add_head(&dev->index_hlist, dev_index_hash(dev->ifindex));
 	dev_hold(dev);
@@ -3271,22 +3277,22 @@ int unregister_netdevice(struct net_devi
 
 	/* And unlink it from device chain. */
 	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
-		if (d == dev) {
-			write_lock_bh(&dev_base_lock);
-			hlist_del(&dev->name_hlist);
-			hlist_del(&dev->index_hlist);
-			if (dev_tail == &dev->next)
-				dev_tail = dp;
-			*dp = d->next;
-			write_unlock_bh(&dev_base_lock);
+		if (d == dev)
 			break;
-		}
 	}
 	if (!d) {
 		printk(KERN_ERR "unregister net_device: '%s' not found\n",
 		       dev->name);
 		return -ENODEV;
 	}
+	write_lock_bh(&dev_base_lock);
+	list_del(&dev->dev_list);
+	hlist_del(&dev->name_hlist);
+	hlist_del(&dev->index_hlist);
+	if (dev_tail == &dev->next)
+		dev_tail = dp;
+	*dp = d->next;
+	write_unlock_bh(&dev_base_lock);
 
 	dev->reg_state = NETREG_UNREGISTERING;
 
--- ./net/ipv4/devinet.c.vedevbase-core	Mon Jul  3 15:14:20 2006
+++ ./net/ipv4/devinet.c	Mon Jul  3 16:09:11 2006
@@ -841,7 +841,7 @@ no_in_dev:
 	 */
 	read_lock(&dev_base_lock);
 	rcu_read_lock();
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
 			continue;
 
@@ -920,7 +920,7 @@ u32 inet_confirm_addr(const struct net_d
 
 	read_lock(&dev_base_lock);
 	rcu_read_lock();
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((in_dev = __in_dev_get_rcu(dev))) {
 			addr = confirm_addr_indev(in_dev, dst, local, scope);
 			if (addr)
@@ -1170,7 +1170,7 @@ void inet_forward_change(void)
 	ipv4_devconf_dflt.forwarding = on;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		struct in_device *in_dev;
 		rcu_read_lock();
 		in_dev = __in_dev_get_rcu(dev);
--- ./net/ipv6/addrconf.c.vedevbase-core	Mon Jul  3 15:14:22 2006
+++ ./net/ipv6/addrconf.c	Mon Jul  3 16:09:11 2006
@@ -469,7 +469,7 @@ static void addrconf_forward_change(void
 	struct inet6_dev *idev;
 
 	read_lock(&dev_base_lock);
-	for (dev=dev_base; dev; dev=dev->next) {
+	for_each_netdev(dev) {
 		read_lock(&addrconf_lock);
 		idev = __in6_dev_get(dev);
 		if (idev) {
@@ -894,7 +894,7 @@ int ipv6_dev_get_saddr(struct net_device
 	read_lock(&dev_base_lock);
 	read_lock(&addrconf_lock);
 
-	for (dev = dev_base; dev; dev=dev->next) {
+	for_each_netdev(dev) {
 		struct inet6_dev *idev;
 		struct inet6_ifaddr *ifa;
 
@@ -1979,7 +1979,7 @@ static void sit_add_v4_addrs(struct inet
 		return;
 	}
 
-        for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		struct in_device * in_dev = __in_dev_get_rtnl(dev);
 		if (in_dev && (dev->flags & IFF_UP)) {
 			struct in_ifaddr * ifa;
@@ -2128,7 +2128,7 @@ static void ip6_tnl_add_linklocal(struct
 			return;
 	}
 	/* then try to inherit it from any device */
-	for (link_dev = dev_base; link_dev; link_dev = link_dev->next) {
+	for_each_netdev(link_dev) {
 		if (!ipv6_inherit_linklocal(idev, link_dev))
 			return;
 	}
--- ./net/ipv6/anycast.c.vedevbase-core	Mon Jul  3 15:14:22 2006
+++ ./net/ipv6/anycast.c	Mon Jul  3 16:09:11 2006
@@ -427,11 +427,13 @@ int ipv6_chk_acast_addr(struct net_devic
 	if (dev)
 		return ipv6_chk_acast_dev(dev, addr);
 	read_lock(&dev_base_lock);
-	for (dev=dev_base; dev; dev=dev->next)
-		if (ipv6_chk_acast_dev(dev, addr))
-			break;
+	for_each_netdev(dev)
+		if (ipv6_chk_acast_dev(dev, addr)) {
+			read_unlock(&dev_base_lock);
+			return 1;
+		}
 	read_unlock(&dev_base_lock);
-	return dev != 0;
+	return 0;
 }
 
 

[patch 2/7] net_device list cleanup: proc seq_file output

[Andrey Savochkin]

Cleanup of net_device list use in seq_file output routines in core networking
files.  Implementation of /proc/net/dev was copied from dev_mcast, since the
latter did the same in a more compact and cleaner way.

The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
Note: functions covered by this patch are good candidates for further
restructuring by introduction of "library" routines for seq_file's showing some
information for each device.

 core/dev.c       |   23 +++++++++++------------
 core/dev_mcast.c |    4 ++--
 ipv4/igmp.c      |   25 +++++++++++++++----------
 ipv6/anycast.c   |   12 +++++++-----
 ipv6/mcast.c     |   25 +++++++++++++++----------
 5 files changed, 50 insertions(+), 39 deletions(-)

--- ./net/core/dev.c.vedevbase-proc	Mon Jul  3 16:09:54 2006
+++ ./net/core/dev.c	Mon Jul  3 16:09:54 2006
@@ -2072,26 +2072,25 @@ static int dev_ifconf(char __user *arg)
  *	This is invoked by the /proc filesystem handler to display a device
  *	in detail.
  */
-static __inline__ struct net_device *dev_get_idx(loff_t pos)
-{
-	struct net_device *dev;
-	loff_t i;
-
-	for (i = 0, dev = dev_base; dev && i < pos; ++i, dev = dev->next);
-
-	return i == pos ? dev : NULL;
-}
-
 void *dev_seq_start(struct seq_file *seq, loff_t *pos)
 {
+	struct net_device *dev;
+	loff_t off = 1;
 	read_lock(&dev_base_lock);
-	return *pos ? dev_get_idx(*pos - 1) : SEQ_START_TOKEN;
+	if (!*pos)
+		return SEQ_START_TOKEN;
+	for_each_netdev(dev) {
+		if (off++ == *pos)
+			return dev;
+	}
+	return NULL;
 }
 
 void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
+	struct net_device *dev = v;
 	++*pos;
-	return v == SEQ_START_TOKEN ? dev_base : ((struct net_device *)v)->next;
+	return v == SEQ_START_TOKEN ? first_netdev() : next_netdev(dev);
 }
 
 void dev_seq_stop(struct seq_file *seq, void *v)
--- ./net/core/dev_mcast.c.vedevbase-proc	Mon Jul  3 15:14:19 2006
+++ ./net/core/dev_mcast.c	Mon Jul  3 16:09:54 2006
@@ -225,7 +225,7 @@ static void *dev_mc_seq_start(struct seq
 	loff_t off = 0;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		if (off++ == *pos) 
 			return dev;
 	}
@@ -236,7 +236,7 @@ static void *dev_mc_seq_next(struct seq_
 {
 	struct net_device *dev = v;
 	++*pos;
-	return dev->next;
+	return next_netdev(dev);
 }
 
 static void dev_mc_seq_stop(struct seq_file *seq, void *v)
--- ./net/ipv4/igmp.c.vedevbase-proc	Mon Jul  3 15:14:20 2006
+++ ./net/ipv4/igmp.c	Mon Jul  3 16:09:54 2006
@@ -2254,19 +2254,21 @@ struct igmp_mc_iter_state {
 
 static inline struct ip_mc_list *igmp_mc_get_first(struct seq_file *seq)
 {
+	struct net_device *dev;
 	struct ip_mc_list *im = NULL;
 	struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
 
-	for (state->dev = dev_base, state->in_dev = NULL;
-	     state->dev; 
-	     state->dev = state->dev->next) {
+	state->dev = NULL;
+	state->in_dev = NULL;
+	for_each_netdev(dev) {
 		struct in_device *in_dev;
-		in_dev = in_dev_get(state->dev);
+		in_dev = in_dev_get(dev);
 		if (!in_dev)
 			continue;
 		read_lock(&in_dev->mc_list_lock);
 		im = in_dev->mc_list;
 		if (im) {
+			state->dev = dev;
 			state->in_dev = in_dev;
 			break;
 		}
@@ -2285,7 +2287,7 @@ static struct ip_mc_list *igmp_mc_get_ne
 			read_unlock(&state->in_dev->mc_list_lock);
 			in_dev_put(state->in_dev);
 		}
-		state->dev = state->dev->next;
+		state->dev = next_netdev(state->dev);
 		if (!state->dev) {
 			state->in_dev = NULL;
 			break;
@@ -2416,15 +2418,17 @@ struct igmp_mcf_iter_state {
 
 static inline struct ip_sf_list *igmp_mcf_get_first(struct seq_file *seq)
 {
+	struct net_device *dev;
 	struct ip_sf_list *psf = NULL;
 	struct ip_mc_list *im = NULL;
 	struct igmp_mcf_iter_state *state = igmp_mcf_seq_private(seq);
 
-	for (state->dev = dev_base, state->idev = NULL, state->im = NULL;
-	     state->dev; 
-	     state->dev = state->dev->next) {
+	state->dev = NULL;
+	state->im = NULL;
+	state->idev = NULL;
+	for_each_netdev(dev) {
 		struct in_device *idev;
-		idev = in_dev_get(state->dev);
+		idev = in_dev_get(dev);
 		if (unlikely(idev == NULL))
 			continue;
 		read_lock(&idev->mc_list_lock);
@@ -2433,6 +2437,7 @@ static inline struct ip_sf_list *igmp_mc
 			spin_lock_bh(&im->lock);
 			psf = im->sources;
 			if (likely(psf != NULL)) {
+				state->dev = dev;
 				state->im = im;
 				state->idev = idev;
 				break;
@@ -2458,7 +2463,7 @@ static struct ip_sf_list *igmp_mcf_get_n
 				read_unlock(&state->idev->mc_list_lock);
 				in_dev_put(state->idev);
 			}
-			state->dev = state->dev->next;
+			state->dev = next_netdev(state->dev);
 			if (!state->dev) {
 				state->idev = NULL;
 				goto out;
--- ./net/ipv6/anycast.c.vedevbase-proc	Mon Jul  3 16:09:54 2006
+++ ./net/ipv6/anycast.c	Mon Jul  3 16:09:54 2006
@@ -447,19 +447,21 @@ struct ac6_iter_state {
 
 static inline struct ifacaddr6 *ac6_get_first(struct seq_file *seq)
 {
+	struct net_device *dev;
 	struct ifacaddr6 *im = NULL;
 	struct ac6_iter_state *state = ac6_seq_private(seq);
 
-	for (state->dev = dev_base, state->idev = NULL;
-	     state->dev;
-	     state->dev = state->dev->next) {
+	state->dev = NULL;
+	state->idev = NULL;
+	for_each_netdev(dev) {
 		struct inet6_dev *idev;
-		idev = in6_dev_get(state->dev);
+		idev = in6_dev_get(dev);
 		if (!idev)
 			continue;
 		read_lock_bh(&idev->lock);
 		im = idev->ac_list;
 		if (im) {
+			state->dev = dev;
 			state->idev = idev;
 			break;
 		}
@@ -478,7 +480,7 @@ static struct ifacaddr6 *ac6_get_next(st
 			read_unlock_bh(&state->idev->lock);
 			in6_dev_put(state->idev);
 		}
-		state->dev = state->dev->next;
+		state->dev = next_netdev(state->dev);
 		if (!state->dev) {
 			state->idev = NULL;
 			break;
--- ./net/ipv6/mcast.c.vedevbase-proc	Mon Jul  3 15:14:22 2006
+++ ./net/ipv6/mcast.c	Mon Jul  3 16:09:54 2006
@@ -2322,19 +2322,21 @@ struct igmp6_mc_iter_state {
 
 static inline struct ifmcaddr6 *igmp6_mc_get_first(struct seq_file *seq)
 {
+	struct net_device *dev;
 	struct ifmcaddr6 *im = NULL;
 	struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
 
-	for (state->dev = dev_base, state->idev = NULL;
-	     state->dev; 
-	     state->dev = state->dev->next) {
+	state->dev = NULL;
+	state->idev = NULL;
+	for_each_netdev(dev) {
 		struct inet6_dev *idev;
-		idev = in6_dev_get(state->dev);
+		idev = in6_dev_get(dev);
 		if (!idev)
 			continue;
 		read_lock_bh(&idev->lock);
 		im = idev->mc_list;
 		if (im) {
+			state->dev = dev;
 			state->idev = idev;
 			break;
 		}
@@ -2354,7 +2356,7 @@ static struct ifmcaddr6 *igmp6_mc_get_ne
 			read_unlock_bh(&state->idev->lock);
 			in6_dev_put(state->idev);
 		}
-		state->dev = state->dev->next;
+		state->dev = next_netdev(state->dev);
 		if (!state->dev) {
 			state->idev = NULL;
 			break;
@@ -2465,15 +2467,17 @@ struct igmp6_mcf_iter_state {
 
 static inline struct ip6_sf_list *igmp6_mcf_get_first(struct seq_file *seq)
 {
+	struct net_device *dev;
 	struct ip6_sf_list *psf = NULL;
 	struct ifmcaddr6 *im = NULL;
 	struct igmp6_mcf_iter_state *state = igmp6_mcf_seq_private(seq);
 
-	for (state->dev = dev_base, state->idev = NULL, state->im = NULL;
-	     state->dev; 
-	     state->dev = state->dev->next) {
+	state->dev = NULL;
+	state->im = NULL;
+	state->idev = NULL;
+	for_each_netdev(dev) {
 		struct inet6_dev *idev;
-		idev = in6_dev_get(state->dev);
+		idev = in6_dev_get(dev);
 		if (unlikely(idev == NULL))
 			continue;
 		read_lock_bh(&idev->lock);
@@ -2482,6 +2486,7 @@ static inline struct ip6_sf_list *igmp6_
 			spin_lock_bh(&im->mca_lock);
 			psf = im->mca_sources;
 			if (likely(psf != NULL)) {
+				state->dev = dev;
 				state->im = im;
 				state->idev = idev;
 				break;
@@ -2507,7 +2512,7 @@ static struct ip6_sf_list *igmp6_mcf_get
 				read_unlock_bh(&state->idev->lock);
 				in6_dev_put(state->idev);
 			}
-			state->dev = state->dev->next;
+			state->dev = next_netdev(state->dev);
 			if (!state->dev) {
 				state->idev = NULL;
 				goto out;

[patch 3/7] net_device list cleanup: netlink_dump

[Andrey Savochkin]

Cleanup of net_device list use in netlink_dump routines in core networking
files.

The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
 core/rtnetlink.c |   18 ++++++++++--------
 ipv4/devinet.c   |   14 ++++++++------
 ipv6/addrconf.c  |   20 +++++++++++++-------
 sched/sch_api.c  |    8 ++++++--
 4 files changed, 37 insertions(+), 23 deletions(-)

--- ./net/core/rtnetlink.c.vedevbase-dump	Mon Jul  3 15:14:19 2006
+++ ./net/core/rtnetlink.c	Mon Jul  3 16:10:12 2006
@@ -319,14 +319,16 @@ static int rtnetlink_dump_ifinfo(struct 
 	struct net_device *dev;
 
 	read_lock(&dev_base_lock);
-	for (dev=dev_base, idx=0; dev; dev = dev->next, idx++) {
-		if (idx < s_idx)
-			continue;
-		if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK,
-					  NETLINK_CB(cb->skb).pid,
-					  cb->nlh->nlmsg_seq, 0,
-					  NLM_F_MULTI) <= 0)
-			break;
+	idx = 0;
+	for_each_netdev(dev) {
+		if (idx >= s_idx) {
+			if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK,
+						  NETLINK_CB(cb->skb).pid,
+						  cb->nlh->nlmsg_seq, 0,
+						  NLM_F_MULTI) <= 0)
+				break;
+		}
+		idx++;
 	}
 	read_unlock(&dev_base_lock);
 	cb->args[0] = idx;
--- ./net/ipv4/devinet.c.vedevbase-dump	Mon Jul  3 16:10:12 2006
+++ ./net/ipv4/devinet.c	Mon Jul  3 16:10:12 2006
@@ -1094,18 +1094,17 @@ static int inet_dump_ifaddr(struct sk_bu
 	struct in_ifaddr *ifa;
 	int s_ip_idx, s_idx = cb->args[0];
 
+	idx = 0;
 	s_ip_idx = ip_idx = cb->args[1];
 	read_lock(&dev_base_lock);
-	for (dev = dev_base, idx = 0; dev; dev = dev->next, idx++) {
+	for_each_netdev(dev) {
 		if (idx < s_idx)
-			continue;
+			goto cont;
 		if (idx > s_idx)
 			s_ip_idx = 0;
 		rcu_read_lock();
-		if ((in_dev = __in_dev_get_rcu(dev)) == NULL) {
-			rcu_read_unlock();
-			continue;
-		}
+		if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
+			goto cont_unlock;
 
 		for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
 		     ifa = ifa->ifa_next, ip_idx++) {
@@ -1118,7 +1117,10 @@ static int inet_dump_ifaddr(struct sk_bu
 				goto done;
 			}
 		}
+cont_unlock:
 		rcu_read_unlock();
+cont:
+		idx++;
 	}
 
 done:
--- ./net/ipv6/addrconf.c.vedevbase-dump	Mon Jul  3 16:10:12 2006
+++ ./net/ipv6/addrconf.c	Mon Jul  3 16:10:12 2006
@@ -3013,18 +3013,19 @@ static int inet6_dump_addr(struct sk_buf
 	struct ifmcaddr6 *ifmca;
 	struct ifacaddr6 *ifaca;
 
+	idx = 0;
 	s_idx = cb->args[0];
 	s_ip_idx = ip_idx = cb->args[1];
 	read_lock(&dev_base_lock);
 	
-	for (dev = dev_base, idx = 0; dev; dev = dev->next, idx++) {
+	for_each_netdev(dev) {
 		if (idx < s_idx)
-			continue;
+			goto cont;
 		if (idx > s_idx)
 			s_ip_idx = 0;
 		ip_idx = 0;
 		if ((idev = in6_dev_get(dev)) == NULL)
-			continue;
+			goto cont;
 		read_lock_bh(&idev->lock);
 		switch (type) {
 		case UNICAST_ADDR:
@@ -3071,6 +3072,8 @@ static int inet6_dump_addr(struct sk_buf
 		}
 		read_unlock_bh(&idev->lock);
 		in6_dev_put(idev);
+cont:
+		idx++;
 	}
 done:
 	if (err <= 0) {
@@ -3238,17 +3241,20 @@ static int inet6_dump_ifinfo(struct sk_b
 	struct net_device *dev;
 	struct inet6_dev *idev;
 
+	idx = 0;
 	read_lock(&dev_base_lock);
-	for (dev=dev_base, idx=0; dev; dev = dev->next, idx++) {
+	for_each_netdev(dev) {
 		if (idx < s_idx)
-			continue;
+			goto cont;
 		if ((idev = in6_dev_get(dev)) == NULL)
-			continue;
+			goto cont;
 		err = inet6_fill_ifinfo(skb, idev, NETLINK_CB(cb->skb).pid, 
 				cb->nlh->nlmsg_seq, RTM_NEWLINK, NLM_F_MULTI);
 		in6_dev_put(idev);
 		if (err <= 0)
 			break;
+cont:
+		idx++;
 	}
 	read_unlock(&dev_base_lock);
 	cb->args[0] = idx;
@@ -3872,7 +3878,7 @@ void __exit addrconf_cleanup(void)
 	 *	clean dev list.
 	 */
 
-	for (dev=dev_base; dev; dev=dev->next) {
+	for_each_netdev(dev) {
 		if ((idev = __in6_dev_get(dev)) == NULL)
 			continue;
 		addrconf_ifdown(dev, 1);
--- ./net/sched/sch_api.c.vedevbase-dump	Mon Jul  3 15:14:22 2006
+++ ./net/sched/sch_api.c	Mon Jul  3 16:10:12 2006
@@ -829,12 +829,15 @@ static int tc_dump_qdisc(struct sk_buff 
 	struct net_device *dev;
 	struct Qdisc *q;
 
+	idx = 0;
 	s_idx = cb->args[0];
 	s_q_idx = q_idx = cb->args[1];
 	read_lock(&dev_base_lock);
-	for (dev=dev_base, idx=0; dev; dev = dev->next, idx++) {
-		if (idx < s_idx)
+	for_each_netdev(dev) {
+		if (idx < s_idx) {
+			idx++;
 			continue;
+		}
 		if (idx > s_idx)
 			s_q_idx = 0;
 		read_lock_bh(&qdisc_tree_lock);
@@ -852,6 +855,7 @@ static int tc_dump_qdisc(struct sk_buff 
 			q_idx++;
 		}
 		read_unlock_bh(&qdisc_tree_lock);
+		idx++;
 	}
 
 done:

[patch 4/7] net_device list cleanup: drivers and non-IP protocols

[Andrey Savochkin]

Cleanup of net_device list use in network device drivers and protocols
other than IP.

The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
Requires "bridge: br_dump_ifinfo index fix"

 drivers/net/wireless/strip.c |    4 +---
 net/8021q/vlan.c             |    4 ++--
 net/8021q/vlanproc.c         |   10 +++++-----
 net/bridge/br_if.c           |    4 ++--
 net/bridge/br_ioctl.c        |    4 +++-
 net/bridge/br_netlink.c      |    3 ++-
 net/decnet/af_decnet.c       |   11 +++++++----
 net/decnet/dn_dev.c          |   17 ++++++++++-------
 net/decnet/dn_fib.c          |    2 +-
 net/decnet/dn_route.c        |   13 +++++++------
 net/llc/llc_core.c           |    7 +++++--
 net/netrom/nr_route.c        |    5 +++--
 net/rose/rose_route.c        |    8 +++++---
 net/sctp/protocol.c          |    2 +-
 net/tipc/eth_media.c         |   11 +++++++----
 15 files changed, 61 insertions(+), 44 deletions(-)

--- ./drivers/net/wireless/strip.c.vedevbase-onet	Mon Jul  3 15:13:46 2006
+++ ./drivers/net/wireless/strip.c	Mon Jul  3 16:12:11 2006
@@ -1969,8 +1969,7 @@ static struct net_device *get_strip_dev(
 		      sizeof(zero_address))) {
 		struct net_device *dev;
 		read_lock_bh(&dev_base_lock);
-		dev = dev_base;
-		while (dev) {
+		for_each_netdev(dev) {
 			if (dev->type == strip_info->dev->type &&
 			    !memcmp(dev->dev_addr,
 				    &strip_info->true_dev_addr,
@@ -1981,7 +1980,6 @@ static struct net_device *get_strip_dev(
 				read_unlock_bh(&dev_base_lock);
 				return (dev);
 			}
-			dev = dev->next;
 		}
 		read_unlock_bh(&dev_base_lock);
 	}
--- ./net/8021q/vlan.c.vedevbase-onet	Mon Jul  3 15:14:17 2006
+++ ./net/8021q/vlan.c	Mon Jul  3 16:12:11 2006
@@ -121,8 +121,8 @@ static void __exit vlan_cleanup_devices(
 	struct net_device *dev, *nxt;
 
 	rtnl_lock();
-	for (dev = dev_base; dev; dev = nxt) {
-		nxt = dev->next;
+	for (dev = first_netdev(); dev; dev = nxt) {
+		nxt = next_netdev(dev);
 		if (dev->priv_flags & IFF_802_1Q_VLAN) {
 			unregister_vlan_dev(VLAN_DEV_INFO(dev)->real_dev,
 					    VLAN_DEV_INFO(dev)->vlan_id);
--- ./net/8021q/vlanproc.c.vedevbase-onet	Mon Jul  3 15:14:17 2006
+++ ./net/8021q/vlanproc.c	Mon Jul  3 16:12:11 2006
@@ -241,7 +241,7 @@ int vlan_proc_rem_dev(struct net_device 
 static struct net_device *vlan_skip(struct net_device *dev) 
 {
 	while (dev && !(dev->priv_flags & IFF_802_1Q_VLAN)) 
-		dev = dev->next;
+		dev = next_netdev(dev);
 
 	return dev;
 }
@@ -257,8 +257,8 @@ static void *vlan_seq_start(struct seq_f
 	if (*pos == 0)
 		return SEQ_START_TOKEN;
 	
-	for (dev = vlan_skip(dev_base); dev && i < *pos; 
-	     dev = vlan_skip(dev->next), ++i);
+	for (dev = vlan_skip(first_netdev()); dev && i < *pos; 
+	     dev = vlan_skip(next_netdev(dev)), ++i);
 		
 	return  (i == *pos) ? dev : NULL;
 } 
@@ -268,8 +268,8 @@ static void *vlan_seq_next(struct seq_fi
 	++*pos;
 
 	return vlan_skip((v == SEQ_START_TOKEN)  
-			    ? dev_base 
-			    : ((struct net_device *)v)->next);
+			    ? first_netdev()
+			    : next_netdev((struct net_device *)v));
 }
 
 static void vlan_seq_stop(struct seq_file *seq, void *v)
--- ./net/bridge/br_if.c.vedevbase-onet	Mon Jul  3 15:14:19 2006
+++ ./net/bridge/br_if.c	Mon Jul  3 16:12:11 2006
@@ -474,8 +474,8 @@ void __exit br_cleanup_bridges(void)
 	struct net_device *dev, *nxt;
 
 	rtnl_lock();
-	for (dev = dev_base; dev; dev = nxt) {
-		nxt = dev->next;
+	for (dev = first_netdev(); dev; dev = nxt) {
+		nxt = next_netdev(dev);
 		if (dev->priv_flags & IFF_EBRIDGE)
 			del_br(dev->priv);
 	}
--- ./net/bridge/br_ioctl.c.vedevbase-onet	Mon Mar 20 08:53:29 2006
+++ ./net/bridge/br_ioctl.c	Mon Jul  3 16:12:11 2006
@@ -27,7 +27,9 @@ static int get_bridge_ifindices(int *ind
 	struct net_device *dev;
 	int i = 0;
 
-	for (dev = dev_base; dev && i < num; dev = dev->next) {
+	for_each_netdev(dev) {
+		if (i >= num)
+			break;
 		if (dev->priv_flags & IFF_EBRIDGE) 
 			indices[i++] = dev->ifindex;
 	}
--- ./net/bridge/br_netlink.c.vedevbase-onet	Mon Jul  3 16:12:11 2006
+++ ./net/bridge/br_netlink.c	Mon Jul  3 16:12:11 2006
@@ -109,7 +109,8 @@ static int br_dump_ifinfo(struct sk_buff
 	int err = 0;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base, idx = 0; dev; dev = dev->next) {
+	idx = 0;
+	for_each_netdev(dev) {
 		struct net_bridge_port *p = dev->br_port;
 
 		/* not a bridge port */
--- ./net/decnet/af_decnet.c.vedevbase-onet	Mon Jul  3 15:14:19 2006
+++ ./net/decnet/af_decnet.c	Mon Jul  3 16:12:11 2006
@@ -720,7 +720,7 @@ static int dn_bind(struct socket *sock, 
 	struct sock *sk = sock->sk;
 	struct dn_scp *scp = DN_SK(sk);
 	struct sockaddr_dn *saddr = (struct sockaddr_dn *)uaddr;
-	struct net_device *dev;
+	struct net_device *pdev, *dev;
 	int rv;
 
 	if (addr_len != sizeof(struct sockaddr_dn))
@@ -744,12 +744,15 @@ static int dn_bind(struct socket *sock, 
 
 	if (!(saddr->sdn_flags & SDF_WILD)) {
 		if (dn_ntohs(saddr->sdn_nodeaddrl)) {
+			dev = NULL;
 			read_lock(&dev_base_lock);
-			for(dev = dev_base; dev; dev = dev->next) {
-				if (!dev->dn_ptr)
+			for_each_netdev(pdev) {
+				if (!pdev->dn_ptr)
 					continue;
-				if (dn_dev_islocal(dev, dn_saddr2dn(saddr)))
+				if (dn_dev_islocal(pdev, dn_saddr2dn(saddr))) {
+					dev = pdev;
 					break;
+				}
 			}
 			read_unlock(&dev_base_lock);
 			if (dev == NULL)
--- ./net/decnet/dn_dev.c.vedevbase-onet	Mon Jul  3 15:14:19 2006
+++ ./net/decnet/dn_dev.c	Mon Jul  3 16:12:11 2006
@@ -775,13 +775,14 @@ static int dn_dev_dump_ifaddr(struct sk_
 	s_idx = cb->args[0];
 	s_dn_idx = dn_idx = cb->args[1];
 	read_lock(&dev_base_lock);
-	for(dev = dev_base, idx = 0; dev; dev = dev->next, idx++) {
+	idx = 0;
+	for_each_netdev(dev) {
 		if (idx < s_idx)
-			continue;
+			goto cont;
 		if (idx > s_idx)
 			s_dn_idx = 0;
 		if ((dn_db = dev->dn_ptr) == NULL)
-			continue;
+			goto cont;
 
 		for(ifa = dn_db->ifa_list, dn_idx = 0; ifa; ifa = ifa->ifa_next, dn_idx++) {
 			if (dn_idx < s_dn_idx)
@@ -794,6 +795,8 @@ static int dn_dev_dump_ifaddr(struct sk_
 					       NLM_F_MULTI) <= 0)
 				goto done;
 		}
+cont:
+		idx++;
 	}
 done:
 	read_unlock(&dev_base_lock);
@@ -1264,7 +1267,7 @@ void dn_dev_devices_off(void)
 	struct net_device *dev;
 
 	rtnl_lock();
-	for(dev = dev_base; dev; dev = dev->next)
+	for_each_netdev(dev)
 		dn_dev_down(dev);
 	rtnl_unlock();
 
@@ -1275,7 +1278,7 @@ void dn_dev_devices_on(void)
 	struct net_device *dev;
 
 	rtnl_lock();
-	for(dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		if (dev->flags & IFF_UP)
 			dn_dev_up(dev);
 	}
@@ -1296,7 +1299,7 @@ int unregister_dnaddr_notifier(struct no
 static inline struct net_device *dn_dev_get_next(struct seq_file *seq, struct net_device *dev)
 {
 	do {
-		dev = dev->next;
+		dev = next_netdev(dev);
 	} while(dev && !dev->dn_ptr);
 
 	return dev;
@@ -1306,7 +1309,7 @@ static struct net_device *dn_dev_get_idx
 {
 	struct net_device *dev;
 
-	dev = dev_base;
+	dev = first_netdev();
 	if (dev && !dev->dn_ptr)
 		dev = dn_dev_get_next(seq, dev);
 	if (pos) {
--- ./net/decnet/dn_fib.c.vedevbase-onet	Mon Jul  3 15:14:19 2006
+++ ./net/decnet/dn_fib.c	Mon Jul  3 16:12:11 2006
@@ -630,7 +630,7 @@ static void dn_fib_del_ifaddr(struct dn_
 
 	/* Scan device list */
 	read_lock(&dev_base_lock);
-	for(dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		dn_db = dev->dn_ptr;
 		if (dn_db == NULL)
 			continue;
--- ./net/decnet/dn_route.c.vedevbase-onet	Mon Jul  3 15:14:19 2006
+++ ./net/decnet/dn_route.c	Mon Jul  3 16:12:47 2006
@@ -922,16 +922,17 @@ static int dn_route_output_slow(struct d
 			goto out;
 		}
 		read_lock(&dev_base_lock);
-		for(dev_out = dev_base; dev_out; dev_out = dev_out->next) {
+		for_each_netdev(dev_out) {
 			if (!dev_out->dn_ptr)
 				continue;
-			if (dn_dev_islocal(dev_out, oldflp->fld_src))
-				break;
+			if (dn_dev_islocal(dev_out, oldflp->fld_src)) {
+				dev_hold(dev_out);
+				read_unlock(&dev_base_lock);
+				goto source_ok;
+			}
 		}
 		read_unlock(&dev_base_lock);
-		if (dev_out == NULL)
-			goto out;
-		dev_hold(dev_out);
+		goto out;
 source_ok:
 		;
 	}
--- ./net/llc/llc_core.c.vedevbase-onet	Mon Jul  3 15:10:43 2006
+++ ./net/llc/llc_core.c	Mon Jul  3 16:12:11 2006
@@ -161,8 +161,11 @@ static struct packet_type llc_tr_packet_
 
 static int __init llc_init(void)
 {
-	if (dev_base->next)
-		memcpy(llc_station_mac_sa, dev_base->next->dev_addr, ETH_ALEN);
+	struct net_device *dev;
+
+	dev = next_netdev(first_netdev());
+	if (dev)
+		memcpy(llc_station_mac_sa, dev->dev_addr, ETH_ALEN);
 	else
 		memset(llc_station_mac_sa, 0, ETH_ALEN);
 	dev_add_pack(&llc_packet_type);
--- ./net/netrom/nr_route.c.vedevbase-onet	Mon Jul  3 15:14:22 2006
+++ ./net/netrom/nr_route.c	Mon Jul  3 16:12:11 2006
@@ -595,7 +595,7 @@ struct net_device *nr_dev_first(void)
 	struct net_device *dev, *first = NULL;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM)
 			if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
 				first = dev;
@@ -615,12 +615,13 @@ struct net_device *nr_dev_get(ax25_addre
 	struct net_device *dev;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM && ax25cmp(addr, (ax25_address *)dev->dev_addr) == 0) {
 			dev_hold(dev);
 			goto out;
 		}
 	}
+	dev = NULL;
 out:
 	read_unlock(&dev_base_lock);
 	return dev;
--- ./net/rose/rose_route.c.vedevbase-onet	Mon Jul  3 15:10:43 2006
+++ ./net/rose/rose_route.c	Mon Jul  3 16:12:11 2006
@@ -600,7 +600,7 @@ struct net_device *rose_dev_first(void)
 	struct net_device *dev, *first = NULL;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE)
 			if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
 				first = dev;
@@ -618,12 +618,13 @@ struct net_device *rose_dev_get(rose_add
 	struct net_device *dev;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0) {
 			dev_hold(dev);
 			goto out;
 		}
 	}
+	dev = NULL;
 out:
 	read_unlock(&dev_base_lock);
 	return dev;
@@ -634,10 +635,11 @@ static int rose_dev_exists(rose_address 
 	struct net_device *dev;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0)
 			goto out;
 	}
+	dev = NULL;
 out:
 	read_unlock(&dev_base_lock);
 	return dev != NULL;
--- ./net/sctp/protocol.c.vedevbase-onet	Mon Jul  3 15:14:22 2006
+++ ./net/sctp/protocol.c	Mon Jul  3 16:12:11 2006
@@ -177,7 +177,7 @@ static void __sctp_get_local_addr_list(v
 	struct sctp_af *af;
 
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 		__list_for_each(pos, &sctp_address_families) {
 			af = list_entry(pos, struct sctp_af, list);
 			af->copy_addrlist(&sctp_local_addr_list, dev);
--- ./net/tipc/eth_media.c.vedevbase-onet	Mon Jul  3 15:14:23 2006
+++ ./net/tipc/eth_media.c	Mon Jul  3 16:14:31 2006
@@ -120,16 +120,19 @@ static int recv_msg(struct sk_buff *buf,
 
 static int enable_bearer(struct tipc_bearer *tb_ptr)
 {
-	struct net_device *dev = dev_base;
+	struct net_device *pdev, *dev;
 	struct eth_bearer *eb_ptr = &eth_bearers[0];
 	struct eth_bearer *stop = &eth_bearers[MAX_ETH_BEARERS];
 	char *driver_name = strchr((const char *)tb_ptr->name, ':') + 1;
 
 	/* Find device with specified name */
 
-	while (dev && dev->name && strncmp(dev->name, driver_name, IFNAMSIZ)) {	
-		dev = dev->next;
-	}
+	dev = NULL;
+	for_each_netdev(pdev)
+		if (pdev->name && !strncmp(pdev->name, driver_name, IFNAMSIZ)) {
+			dev = pdev;
+			break;
+		}
 	if (!dev)
 		return -ENODEV;
 

[patch 5/7] net_device list cleanup: arch-dependent code and block devices

[Andrey Savochkin]

Cleanup of net_device list use in arch-dependent code and block devices.

The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
 arch/s390/appldata/appldata_net_sum.c |    2 +-
 arch/sparc64/solaris/ioctl.c          |    2 +-
 drivers/block/aoe/aoecmd.c            |    8 ++++++--
 drivers/parisc/led.c                  |    2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

--- ./arch/s390/appldata/appldata_net_sum.c.vedevbase-misc	Mon Jul  3 15:13:15 2006
+++ ./arch/s390/appldata/appldata_net_sum.c	Mon Jul  3 16:16:05 2006
@@ -107,7 +107,7 @@ static void appldata_get_net_sum_data(vo
 	tx_dropped = 0;
 	collisions = 0;
 	read_lock(&dev_base_lock);
-	for (dev = dev_base; dev != NULL; dev = dev->next) {
+	for_each_netdev(dev) {
 		if (dev->get_stats == NULL) {
 			continue;
 		}
--- ./arch/sparc64/solaris/ioctl.c.vedevbase-misc	Mon Mar 20 08:53:29 2006
+++ ./arch/sparc64/solaris/ioctl.c	Mon Jul  3 16:16:05 2006
@@ -686,7 +686,7 @@ static inline int solaris_i(unsigned int
 			int i = 0;
 			
 			read_lock_bh(&dev_base_lock);
-			for (d = dev_base; d; d = d->next) i++;
+			for_each_netdev(d) i++;
 			read_unlock_bh(&dev_base_lock);
 
 			if (put_user (i, (int __user *)A(arg)))
--- ./drivers/block/aoe/aoecmd.c.vedevbase-misc	Mon Jul  3 15:09:57 2006
+++ ./drivers/block/aoe/aoecmd.c	Mon Jul  3 16:16:05 2006
@@ -204,14 +204,17 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
 	sl = sl_tail = NULL;
 
 	read_lock(&dev_base_lock);
-	for (ifp = dev_base; ifp; dev_put(ifp), ifp = ifp->next) {
+	for_each_netdev(ifp) {
 		dev_hold(ifp);
-		if (!is_aoe_netif(ifp))
+		if (!is_aoe_netif(ifp)) {
+			dev_put(ifp);
 			continue;
+		}
 
 		skb = new_skb(ifp, sizeof *h + sizeof *ch);
 		if (skb == NULL) {
 			printk(KERN_INFO "aoe: aoecmd_cfg: skb alloc failure\n");
+			dev_put(ifp);
 			continue;
 		}
 		if (sl_tail == NULL)
@@ -229,6 +232,7 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
 
 		skb->next = sl;
 		sl = skb;
+		dev_put(ifp);
 	}
 	read_unlock(&dev_base_lock);
 
--- ./drivers/parisc/led.c.vedevbase-misc	Mon Jul  3 15:13:46 2006
+++ ./drivers/parisc/led.c	Mon Jul  3 16:16:05 2006
@@ -367,7 +367,7 @@ static __inline__ int led_get_net_activi
 	 * for reading should be OK */
 	read_lock(&dev_base_lock);
 	rcu_read_lock();
-	for (dev = dev_base; dev; dev = dev->next) {
+	for_each_netdev(dev) {
 	    struct net_device_stats *stats;
 	    struct in_device *in_dev = __in_dev_get_rcu(dev);
 	    if (!in_dev || !in_dev->ifa_list)

[patch 6/7] net_device list cleanup: dev_base removal

[Andrey Savochkin]

Removal of dev_base and dev_tail variables as a part of net_device list
cleanup.
The cleanup consists of
 - converting the to list_head, to make the list double-linked (thus making
   remove operation O(1)), and list walks more readable;
 - introducing of for_each_netdev wrapper over list_for_each.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
 include/linux/netdevice.h |    2 --
 net/core/dev.c            |   21 ---------------------
 2 files changed, 23 deletions(-)

--- ./include/linux/netdevice.h.vedevbase-fin	Mon Jul  3 16:16:28 2006
+++ ./include/linux/netdevice.h	Mon Jul  3 16:16:28 2006
@@ -289,7 +289,6 @@ struct net_device
 
 	unsigned long		state;
 
-	struct net_device	*next;
 	struct list_head	dev_list;
 
 	/* The device initialization function. Called only once. */
@@ -558,7 +557,6 @@ struct packet_type {
 #include <linux/notifier.h>
 
 extern struct net_device		loopback_dev;		/* The loopback */
-extern struct net_device		*dev_base;		/* All devices */
 extern struct list_head			dev_base_head;		/* All devices */
 extern rwlock_t				dev_base_lock;		/* Device list lock */
 
--- ./net/core/dev.c.vedevbase-fin	Mon Jul  3 16:16:28 2006
+++ ./net/core/dev.c	Mon Jul  3 16:16:28 2006
@@ -174,11 +174,7 @@ static spinlock_t net_dma_event_lock;
  * unregister_netdevice(), which must be called with the rtnl
  * semaphore held.
  */
-struct net_device *dev_base;
-static struct net_device **dev_tail = &dev_base;
 DEFINE_RWLOCK(dev_base_lock);
-
-EXPORT_SYMBOL(dev_base);
 EXPORT_SYMBOL(dev_base_lock);
 
 LIST_HEAD(dev_base_head);
@@ -2973,11 +2969,8 @@ int register_netdevice(struct net_device
 
 	set_bit(__LINK_STATE_PRESENT, &dev->state);
 
-	dev->next = NULL;
 	dev_init_scheduler(dev);
 	write_lock_bh(&dev_base_lock);
-	*dev_tail = dev;
-	dev_tail = &dev->next;
 	list_add_tail(&dev->dev_list, &dev_base_head);
 	hlist_add_head(&dev->name_hlist, head);
 	hlist_add_head(&dev->index_hlist, dev_index_hash(dev->ifindex));
@@ -3256,8 +3249,6 @@ void synchronize_net(void) 
 
 int unregister_netdevice(struct net_device *dev)
 {
-	struct net_device *d, **dp;
-
 	BUG_ON(dev_boot_phase);
 	ASSERT_RTNL();
 
@@ -3275,22 +3266,10 @@ int unregister_netdevice(struct net_devi
 		dev_close(dev);
 
 	/* And unlink it from device chain. */
-	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
-		if (d == dev)
-			break;
-	}
-	if (!d) {
-		printk(KERN_ERR "unregister net_device: '%s' not found\n",
-		       dev->name);
-		return -ENODEV;
-	}
 	write_lock_bh(&dev_base_lock);
 	list_del(&dev->dev_list);
 	hlist_del(&dev->name_hlist);
 	hlist_del(&dev->index_hlist);
-	if (dev_tail == &dev->next)
-		dev_tail = dp;
-	*dp = d->next;
 	write_unlock_bh(&dev_base_lock);
 
 	dev->reg_state = NETREG_UNREGISTERING;

[patch 7/7] net_device list cleanup: debugging

[Andrey Savochkin]

Optional code to catch cases when loop cursor is used after for_each_netdev
loop: often it's a sign of a bug, since it isn't guaranteed to point to a
device.

Signed-off-by: Andrey Savochkin <saw@swsoft.com>
---
If anyone wants to keep this under some debug config option,
let me know which one.

 netdevice.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletion(-)

--- ./include/linux/netdevice.h.vedevbase-dbg	Mon Jul  3 16:16:51 2006
+++ ./include/linux/netdevice.h	Mon Jul  3 16:16:51 2006
@@ -560,7 +560,13 @@ extern struct net_device		loopback_dev;	
 extern struct list_head			dev_base_head;		/* All devices */
 extern rwlock_t				dev_base_lock;		/* Device list lock */
 
-#define for_each_netdev(p)	list_for_each_entry(p, &dev_base_head, dev_list)
+#define for_each_netdev(pos)	\
+for (pos = list_entry(dev_base_head.next, typeof(*pos), dev_list); \
+     prefetch(pos->dev_list.next), \
+	&pos->dev_list != &dev_base_head ? : \
+		({ void *__check_dev_use_after_for_each_netdev; \
+		   pos = __check_dev_use_after_for_each_netdev; 0; }); \
+     pos = list_entry(pos->dev_list.next, typeof(*pos), dev_list))
 
 /*
  * When possible, it is preferrable to use for_each_netdev() loop

Re: [patch 1/7] net_device list cleanup: core

[Christoph Hellwig]

On Mon, Jul 03, 2006 at 12:18:51PM +0400, Andrey Savochkin wrote:
> Cleanup of net_device list use in net_dev core and IP.
> The cleanup consists of
>  - converting the to list_head, to make the list double-linked (thus making
>    remove operation O(1)), and list walks more readable;
>  - introducing of for_each_netdev wrapper over list_for_each.

When you change all this please make sure dev_base_head is never directly
accessed anymore, not even through macros and dev_base_head is not exported
anymore.  That's the only way to keep drivers messing with it.

Yes, it's a little more work as you need to audit all drivers to see what
they are doing and find suitable abstractions but it's a must have that
should have been done a lot earlier.

Re: [patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

Christoph,

On Mon, Jul 03, 2006 at 06:46:50PM +0100, Christoph Hellwig wrote:
> On Mon, Jul 03, 2006 at 12:18:51PM +0400, Andrey Savochkin wrote:
> > Cleanup of net_device list use in net_dev core and IP.
> > The cleanup consists of
> >  - converting the to list_head, to make the list double-linked (thus making
> >    remove operation O(1)), and list walks more readable;
> >  - introducing of for_each_netdev wrapper over list_for_each.
> 
> When you change all this please make sure dev_base_head is never directly
> accessed anymore, not even through macros and dev_base_head is not exported
> anymore.  That's the only way to keep drivers messing with it.
> 
> Yes, it's a little more work as you need to audit all drivers to see what
> they are doing and find suitable abstractions but it's a must have that
> should have been done a lot earlier.

Hiding dev_base_head can be done by converting first_netdev/next_netdev into
functions and implementing for_each_netdev loop through them.

Or are you talking about abstractions like functions
for_each_netdev/find_netdev with callbacks?
Do you think that hiding the list internals is worth the additional
complexity and substantial increase of the patch size?

	Andrey

Re: [patch 1/7] net_device list cleanup: core

[Christoph Hellwig]

On Tue, Jul 04, 2006 at 11:24:05AM +0400, Andrey Savochkin wrote:
> > Yes, it's a little more work as you need to audit all drivers to see what
> > they are doing and find suitable abstractions but it's a must have that
> > should have been done a lot earlier.
> 
> Hiding dev_base_head can be done by converting first_netdev/next_netdev into
> functions and implementing for_each_netdev loop through them.
> 
> Or are you talking about abstractions like functions
> for_each_netdev/find_netdev with callbacks?

an for_each_netdev with a callback makes sense and gives a cleaner
abstraction, yes.  I don't think you should need a callback for the lookup
structure.

> Do you think that hiding the list internals is worth the additional
> complexity and substantial increase of the patch size?

Yes, absolutely.  We've converted scsi hosts and devices from a model
where drivers could directly access the list to strict iterators in the
2.5 series.  It's quite a lot of work as you have to understand what
the drivers actually do (and to at least 50% they were doing something
really stupid) and convert them to the right abstractions.

Re: [patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

On Tue, Jul 04, 2006 at 10:10:03AM +0100, Christoph Hellwig wrote:
> On Tue, Jul 04, 2006 at 11:24:05AM +0400, Andrey Savochkin wrote:
> > > Yes, it's a little more work as you need to audit all drivers to see what
> > > they are doing and find suitable abstractions but it's a must have that
> > > should have been done a lot earlier.
> > 
> > Hiding dev_base_head can be done by converting first_netdev/next_netdev into
> > functions and implementing for_each_netdev loop through them.
> > 
> > Or are you talking about abstractions like functions
> > for_each_netdev/find_netdev with callbacks?
> 
> an for_each_netdev with a callback makes sense and gives a cleaner
> abstraction, yes.  I don't think you should need a callback for the lookup
> structure.

Different modules want different kinds of lookup.
So, I'm thinking about something like ilookup5.

> 
> > Do you think that hiding the list internals is worth the additional
> > complexity and substantial increase of the patch size?
> 
> Yes, absolutely.  We've converted scsi hosts and devices from a model
> where drivers could directly access the list to strict iterators in the
> 2.5 series.  It's quite a lot of work as you have to understand what
> the drivers actually do (and to at least 50% they were doing something
> really stupid) and convert them to the right abstractions.

The next question: would people agree to review a patch doing this for
net_devices? :)

	Andrey

Re: [patch 1/7] net_device list cleanup: core

[Alexey Kuznetsov]

Hello!

> Different modules want different kinds of lookup.
> So, I'm thinking about something like ilookup5.
....
> The next question: would people agree to review a patch doing this for
> net_devices? :)

One not original suggestion, which did not sound nevertheless:
to implement netdev_iterate_list() or whatever, update only core
and a few of devices and deprecate dev_base_head
with __deprecated_for_modules adding it to
Documentation/feature-removal-schedule.txt

Alexey

Re: [patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

On Tue, Jul 04, 2006 at 08:35:37PM +0400, A.N.Kuznetsov wrote:
> 
> > Different modules want different kinds of lookup.
> > So, I'm thinking about something like ilookup5.
> ....
> > The next question: would people agree to review a patch doing this for
> > net_devices? :)
> 
> One not original suggestion, which did not sound nevertheless:
> to implement netdev_iterate_list() or whatever, update only core
> and a few of devices and deprecate dev_base_head
> with __deprecated_for_modules adding it to
> Documentation/feature-removal-schedule.txt

I like this idea

	Andrey

Re: [patch 1/7] net_device list cleanup: core

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20060630150823.A22285@castle.nmd.msu.ru> (at Mon, 3 Jul 2006 12:18:51 +0400), Andrey Savochkin <saw@swsoft.com> says:

> @@ -3271,22 +3277,22 @@ int unregister_netdevice(struct net_devi
>  
>  	/* And unlink it from device chain. */
>  	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {

Why not for_each_netdev?

--yoshfuji

Re: [patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

On Fri, Jul 07, 2006 at 01:34:34PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B wrote:
> In article <20060630150823.A22285@castle.nmd.msu.ru> (at Mon, 3 Jul 2006 12:18:51 +0400), Andrey Savochkin <saw@swsoft.com> says:
> 
> > @@ -3271,22 +3277,22 @@ int unregister_netdevice(struct net_devi
> >  
> >  	/* And unlink it from device chain. */
> >  	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
> 
> Why not for_each_netdev?

it's a different list

Re: [patch 1/7] net_device list cleanup: core

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20060707115425.A28991@castle.nmd.msu.ru> (at Fri, 7 Jul 2006 11:54:25 +0400), Andrey Savochkin <saw@swsoft.com> says:

> On Fri, Jul 07, 2006 at 01:34:34PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B wrote:
> > In article <20060630150823.A22285@castle.nmd.msu.ru> (at Mon, 3 Jul 2006 12:18:51 +0400), Andrey Savochkin <saw@swsoft.com> says:
> > 
> > > @@ -3271,22 +3277,22 @@ int unregister_netdevice(struct net_devi
> > >  
> > >  	/* And unlink it from device chain. */
> > >  	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
> > 
> > Why not for_each_netdev?
> 
> it's a different list

Sorry, I still do not understand.
In other words, why will we still have dev->next?
After introducing net_device->dev_list, we do not need
dev->next anymore, do we?

--yoshfuji

Re: [patch 4/7] net_device list cleanup: drivers and non-IP protocols

[Stephen Hemminger]

On Mon, 3 Jul 2006 12:18:54 +0400
Andrey Savochkin <saw@swsoft.com> wrote:

> Cleanup of net_device list use in network device drivers and protocols
> other than IP.
> 
> The cleanup consists of
>  - converting the to list_head, to make the list double-linked (thus making
>    remove operation O(1)), and list walks more readable;

Maybe this should be RCU'd to get rid of the read_lock.

>  - introducing of for_each_netdev wrapper over list_for_each.
> 
> Signed-off-by: Andrey Savochkin <saw@swsoft.com>

Looks good thanks.

Re: [patch 1/7] net_device list cleanup: core

[Andrey Savochkin]

On Sat, Jul 08, 2006 at 01:48:13AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B wrote:
> In article <20060707115425.A28991@castle.nmd.msu.ru> (at Fri, 7 Jul 2006 11:54:25 +0400), Andrey Savochkin <saw@swsoft.com> says:
> 
> > On Fri, Jul 07, 2006 at 01:34:34PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B wrote:
> > > In article <20060630150823.A22285@castle.nmd.msu.ru> (at Mon, 3 Jul 2006 12:18:51 +0400), Andrey Savochkin <saw@swsoft.com> says:
> > > 
> > > > @@ -3271,22 +3277,22 @@ int unregister_netdevice(struct net_devi
> > > >  
> > > >  	/* And unlink it from device chain. */
> > > >  	for (dp = &dev_base; (d = *dp) != NULL; dp = &d->next) {
> > > 
> > > Why not for_each_netdev?
> > 
> > it's a different list
> 
> Sorry, I still do not understand.
> In other words, why will we still have dev->next?
> After introducing net_device->dev_list, we do not need
> dev->next anymore, do we?

dev->next is removed in the last patch, to make possible the bisection
of patch list.