[2.6 patch] net/decnet/: misc possible cleanups

[2.6 patch] net/decnet/: misc possible cleanups

[Adrian Bunk]

The patch below contains the following possible cleanups:
- make needlessly global code static
- dn_fib.c: remove the write-only global variable dn_fib_info_cnt
- dn_fib.c: remove the unused global function dn_fib_rt_message
- dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
- dn_timer.c: remove the fast timer code that isn't used

Please review and comment on this patch.


diffstat output:
 include/net/dn.h       |    2 -
 include/net/dn_fib.h   |    1 
 net/decnet/af_decnet.c |    6 +----
 net/decnet/dn_fib.c    |   15 -------------
 net/decnet/dn_neigh.c  |    8 ------
 net/decnet/dn_route.c  |    6 ++---
 net/decnet/dn_timer.c  |   47 -----------------------------------------
 7 files changed, 5 insertions(+), 80 deletions(-)


Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.10-rc3-mm1-full/net/decnet/af_decnet.c.old	2004-12-14 03:30:13.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/net/decnet/af_decnet.c	2004-12-14 03:47:00.000000000 +0100
@@ -246,7 +246,7 @@
 	write_unlock_bh(&dn_hash_lock);
 }
 
-struct hlist_head *listen_hash(struct sockaddr_dn *addr)
+static struct hlist_head *listen_hash(struct sockaddr_dn *addr)
 {
 	int i;
 	unsigned hash = addr->sdn_objnum;
@@ -447,7 +447,7 @@
 	dst_release(xchg(&sk->sk_dst_cache, NULL));
 }
 
-struct sock *dn_alloc_sock(struct socket *sock, int gfp)
+static struct sock *dn_alloc_sock(struct socket *sock, int gfp)
 {
 	struct dn_scp *scp;
 	struct sock *sk = sk_alloc(PF_DECnet, gfp, sizeof(struct dn_sock),
@@ -578,7 +578,6 @@
 	if (sk->sk_socket)
 		return 0;
 
-	dn_stop_fast_timer(sk); /* unlikely, but possible that this is runninng */
 	if ((jiffies - scp->stamp) >= (HZ * decnet_time_wait)) {
 		dn_unhash_sock(sk);
 		sock_put(sk);
@@ -631,7 +630,6 @@
 		default:
 			printk(KERN_DEBUG "DECnet: dn_destroy_sock passed socket in invalid state\n");
 		case DN_O:
-			dn_stop_fast_timer(sk);
 			dn_stop_slow_timer(sk);
 
 			dn_unhash_sock_bh(sk);
--- linux-2.6.10-rc3-mm1-full/include/net/dn_fib.h.old	2004-12-14 03:32:14.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/include/net/dn_fib.h	2004-12-14 03:32:19.000000000 +0100
@@ -117,7 +117,6 @@
 extern void dn_fib_init(void);
 extern void dn_fib_cleanup(void);
 
-extern int dn_fib_rt_message(struct sk_buff *skb);
 extern int dn_fib_ioctl(struct socket *sock, unsigned int cmd, 
 			unsigned long arg);
 extern struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r, 
--- linux-2.6.10-rc3-mm1-full/net/decnet/dn_fib.c.old	2004-12-14 03:31:33.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/net/decnet/dn_fib.c	2004-12-14 03:32:31.000000000 +0100
@@ -60,7 +60,6 @@
 static spinlock_t dn_fib_multipath_lock = SPIN_LOCK_UNLOCKED;
 static struct dn_fib_info *dn_fib_info_list;
 static rwlock_t dn_fib_info_lock = RW_LOCK_UNLOCKED;
-int dn_fib_info_cnt;
 
 static struct
 {
@@ -93,7 +92,6 @@
 			dev_put(nh->nh_dev);
 		nh->nh_dev = NULL;
 	} endfor_nexthops(fi);
-	dn_fib_info_cnt--;
 	kfree(fi);
 }
 
@@ -388,7 +386,6 @@
 	if (dn_fib_info_list)
 		dn_fib_info_list->fib_prev = fi;
 	dn_fib_info_list = fi;
-	dn_fib_info_cnt++;
 	write_unlock(&dn_fib_info_lock);
 	return fi;
 
@@ -486,18 +483,6 @@
 }
 
 
-/*
- * Punt to user via netlink for example, but for now
- * we just drop it.
- */
-int dn_fib_rt_message(struct sk_buff *skb)
-{
-	kfree_skb(skb);
-
-	return 0;
-}
-
-
 static int dn_fib_check_attr(struct rtmsg *r, struct rtattr **rta)
 {
 	int i;
--- linux-2.6.10-rc3-mm1-full/net/decnet/dn_neigh.c.old	2004-12-14 03:32:55.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/net/decnet/dn_neigh.c	2004-12-14 03:33:29.000000000 +0100
@@ -355,14 +355,6 @@
  * basically does a neigh_lookup(), but without comparing the device
  * field. This is required for the On-Ethernet cache
  */
-/*
- * Any traffic on a pointopoint link causes the timer to be reset
- * for the entry in the neighbour table.
- */
-void dn_neigh_pointopoint_notify(struct sk_buff *skb)
-{
-	return;
-}
 
 /*
  * Pointopoint link receives a hello message
--- linux-2.6.10-rc3-mm1-full/net/decnet/dn_route.c.old	2004-12-14 03:33:53.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/net/decnet/dn_route.c	2004-12-14 03:34:54.000000000 +0100
@@ -99,9 +99,9 @@
 
 static unsigned char dn_hiord_addr[6] = {0xAA,0x00,0x04,0x00,0x00,0x00};
 
-int dn_rt_min_delay = 2 * HZ;
-int dn_rt_max_delay = 10 * HZ;
-int dn_rt_mtu_expires = 10 * 60 * HZ;
+static const int dn_rt_min_delay = 2 * HZ;
+static const int dn_rt_max_delay = 10 * HZ;
+static const int dn_rt_mtu_expires = 10 * 60 * HZ;
 
 static unsigned long dn_rt_deadline;
 
--- linux-2.6.10-rc3-mm1-full/include/net/dn.h.old	2004-12-14 03:35:13.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/include/net/dn.h	2004-12-14 03:46:38.000000000 +0100
@@ -220,8 +220,6 @@
 
 extern void dn_start_slow_timer(struct sock *sk);
 extern void dn_stop_slow_timer(struct sock *sk);
-extern void dn_start_fast_timer(struct sock *sk);
-extern void dn_stop_fast_timer(struct sock *sk);
 
 extern dn_address decnet_address;
 extern int decnet_debug_level;
--- linux-2.6.10-rc3-mm1-full/net/decnet/dn_timer.c.old	2004-12-14 03:35:29.000000000 +0100
+++ linux-2.6.10-rc3-mm1-full/net/decnet/dn_timer.c	2004-12-14 03:48:49.000000000 +0100
@@ -27,11 +27,9 @@
 #include <net/dn.h>
 
 /*
- * Fast timer is for delayed acks (200mS max)
  * Slow timer is for everything else (n * 500mS)
  */
 
-#define FAST_INTERVAL (HZ/5)
 #define SLOW_INTERVAL (HZ/2)
 
 static void dn_slow_timer(unsigned long arg);
@@ -109,48 +107,3 @@
 	bh_unlock_sock(sk);
 	sock_put(sk);
 }
-
-static void dn_fast_timer(unsigned long arg)
-{
-	struct sock *sk = (struct sock *)arg;
-	struct dn_scp *scp = DN_SK(sk);
-
-	bh_lock_sock(sk);
-	if (sock_owned_by_user(sk)) {
-		scp->delack_timer.expires = jiffies + HZ / 20;
-		add_timer(&scp->delack_timer);
-		goto out;
-	}
-
-	scp->delack_pending = 0;
-
-	if (scp->delack_fxn)
-		scp->delack_fxn(sk);
-out:
-	bh_unlock_sock(sk);
-}
-
-void dn_start_fast_timer(struct sock *sk)
-{
-	struct dn_scp *scp = DN_SK(sk);
-
-	if (!scp->delack_pending) {
-		scp->delack_pending = 1;
-		init_timer(&scp->delack_timer);
-		scp->delack_timer.expires = jiffies + FAST_INTERVAL;
-		scp->delack_timer.data = (unsigned long)sk;
-		scp->delack_timer.function = dn_fast_timer;
-		add_timer(&scp->delack_timer);
-	}
-}
-
-void dn_stop_fast_timer(struct sock *sk)
-{
-	struct dn_scp *scp = DN_SK(sk);
-
-	if (scp->delack_pending) {
-		scp->delack_pending = 0;
-		del_timer(&scp->delack_timer);
-	}
-}
-

Re: [2.6 patch] net/decnet/: misc possible cleanups

[Steven Whitehouse]

Hi,

In general this looks fine to me. Patrick has the last word though. I
would suggest hanging on to the fast timer code though for now... its
there so that delayed acks can be used and the rest of the code is
very close to actually allowing that to work. The DECnet code has a
habit of sending lots of (rather small) ack packets and it would be nice if
that could be fixed at some stage.

The issues at stake are that it would interact with the code thats working
out what the correct send window is to a certain extent. I know there is
also the argument that it can always be added back should someone have time
to add delayed acks, so I've no strong opinion either way.

Also, when I was writing the routing code - a lot of the design was "borrowed"
from the ipv4 routing code. It might be worth doing a comparison to see where
the two have diverged (something I used to do now and again) to pick up any
bugs I'd inadvertently copied over, if you are working on clean ups in this
area,

Steve.

On Tue, Dec 14, 2004 at 01:58:38PM +0100, Adrian Bunk wrote:
> The patch below contains the following possible cleanups:
> - make needlessly global code static
> - dn_fib.c: remove the write-only global variable dn_fib_info_cnt
> - dn_fib.c: remove the unused global function dn_fib_rt_message
> - dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
> - dn_timer.c: remove the fast timer code that isn't used
> 
> Please review and comment on this patch.

Re: [2.6 patch] net/decnet/: misc possible cleanups

[Patrick Caulfield]

On Tue, Dec 14, 2004 at 01:58:38PM +0100, Adrian Bunk wrote:
> The patch below contains the following possible cleanups:
> - make needlessly global code static
> - dn_fib.c: remove the write-only global variable dn_fib_info_cnt
> - dn_fib.c: remove the unused global function dn_fib_rt_message
> - dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
> - dn_timer.c: remove the fast timer code that isn't used
> 
> Please review and comment on this patch.
> 

Looks fine to me. I'm quite happy to lose the fast ack code - unused code is
only a confusion to those reading it IMHO. If we do the delayed-ack code in
future then it's easy enough to reinstate.

Thanks.

-- 

patrick

Re: [2.6 patch] net/decnet/: misc possible cleanups

[Adrian Bunk]

On Tue, Dec 14, 2004 at 01:32:35PM +0000, Steven Whitehouse wrote:

> Hi,

Hi Steven,

>...
> Also, when I was writing the routing code - a lot of the design was "borrowed"
> from the ipv4 routing code. It might be worth doing a comparison to see where
> the two have diverged (something I used to do now and again) to pick up any
> bugs I'd inadvertently copied over, if you are working on clean ups in this
> area,

unfortunately, I'm not working especially in this area.

I'm currently going through the complete kernel sources searching for 
global code that can be made static or even removed.

> Steve.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed