[PATCH net-next 1/5] tipc: Enhance enabling and disabling of Ethernet bearers

[PATCH net-next 1/5] tipc: Enhance enabling and disabling of Ethernet bearers

[Paul Gortmaker]

From: Allan Stephens <allan.stephens@windriver.com>

Use work queue to eliminate release of TIPC's configuration lock when
registering for device notifications while activating Ethernet media
support. Optimize code that locates an unused bearer entry when enabling
an Ethernet bearer. Use work queue to break the association between a
newly disabled Ethernet bearer and its device driver, thereby allowing
quicker reuse of the bearer entry.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/config.c    |   13 +------
 net/tipc/eth_media.c |   93 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/net/tipc/config.c b/net/tipc/config.c
index 961d1b0..a43450c 100644
--- a/net/tipc/config.c
+++ b/net/tipc/config.c
@@ -332,19 +332,8 @@ static struct sk_buff *cfg_set_own_addr(void)
 		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
 						   " (cannot change node address once assigned)");
 
-	/*
-	 * Must release all spinlocks before calling start_net() because
-	 * Linux version of TIPC calls eth_media_start() which calls
-	 * register_netdevice_notifier() which may block!
-	 *
-	 * Temporarily releasing the lock should be harmless for non-Linux TIPC,
-	 * but Linux version of eth_media_start() should really be reworked
-	 * so that it can be called with spinlocks held.
-	 */
-
-	spin_unlock_bh(&config_lock);
 	tipc_core_start_net(addr);
-	spin_lock_bh(&config_lock);
+
 	return tipc_cfg_reply_none();
 }
 
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index 6e988ba..479dbc0 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -51,17 +51,20 @@
  * @bearer: ptr to associated "generic" bearer structure
  * @dev: ptr to associated Ethernet network device
  * @tipc_packet_type: used in binding TIPC to Ethernet driver
+ * @cleanup: work item used when disabling bearer
  */
 
 struct eth_bearer {
 	struct tipc_bearer *bearer;
 	struct net_device *dev;
 	struct packet_type tipc_packet_type;
+	struct work_struct cleanup;
 };
 
 static struct eth_bearer eth_bearers[MAX_ETH_BEARERS];
 static int eth_started = 0;
 static struct notifier_block notifier;
+static struct work_struct reg_notifier;
 
 /**
  * send_msg - send a TIPC message out over an Ethernet interface
@@ -157,22 +160,22 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 	if (!dev)
 		return -ENODEV;
 
-	/* Find Ethernet bearer for device (or create one) */
-
-	for (;(eb_ptr != stop) && eb_ptr->dev && (eb_ptr->dev != dev); eb_ptr++);
-	if (eb_ptr == stop)
-		return -EDQUOT;
-	if (!eb_ptr->dev) {
-		eb_ptr->dev = dev;
-		eb_ptr->tipc_packet_type.type = htons(ETH_P_TIPC);
-		eb_ptr->tipc_packet_type.dev = dev;
-		eb_ptr->tipc_packet_type.func = recv_msg;
-		eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
-		INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list));
-		dev_hold(dev);
-		dev_add_pack(&eb_ptr->tipc_packet_type);
+	/* Create Ethernet bearer for device */
+
+	while (eb_ptr->dev != NULL) {
+		if (++eb_ptr == stop)
+			return -EDQUOT;
 	}
 
+	eb_ptr->dev = dev;
+	eb_ptr->tipc_packet_type.type = __constant_htons(ETH_P_TIPC);
+	eb_ptr->tipc_packet_type.dev = dev;
+	eb_ptr->tipc_packet_type.func = recv_msg;
+	eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
+	INIT_LIST_HEAD(&eb_ptr->tipc_packet_type.list);
+	dev_hold(dev);
+	dev_add_pack(&eb_ptr->tipc_packet_type);
+
 	/* Associate TIPC bearer with Ethernet bearer */
 
 	eb_ptr->bearer = tb_ptr;
@@ -185,16 +188,36 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 }
 
 /**
+ * cleanup_bearer - break association between Ethernet bearer and interface
+ *
+ * This routine must be invoked from a work queue because it can sleep.
+ */
+
+static void cleanup_bearer(struct work_struct *work)
+{
+	struct eth_bearer *eb_ptr =
+		container_of(work, struct eth_bearer, cleanup);
+
+	dev_remove_pack(&eb_ptr->tipc_packet_type);
+	dev_put(eb_ptr->dev);
+	eb_ptr->dev = NULL;
+}
+
+/**
  * disable_bearer - detach TIPC bearer from an Ethernet interface
  *
- * We really should do dev_remove_pack() here, but this function can not be
- * called at tasklet level. => Use eth_bearer->bearer as a flag to throw away
- * incoming buffers, & postpone dev_remove_pack() to eth_media_stop() on exit.
+ * Mark Ethernet bearer as inactive so that incoming buffers are thrown away,
+ * then get worker thread to complete bearer cleanup.  (Can't do cleanup
+ * here because cleanup code needs to sleep and caller holds spinlocks.)
  */
 
 static void disable_bearer(struct tipc_bearer *tb_ptr)
 {
-	((struct eth_bearer *)tb_ptr->usr_handle)->bearer = NULL;
+	struct eth_bearer *eb_ptr = (struct eth_bearer *)tb_ptr->usr_handle;
+
+	eb_ptr->bearer = NULL;
+	INIT_WORK(&eb_ptr->cleanup, cleanup_bearer);
+	schedule_work(&eb_ptr->cleanup);
 }
 
 /**
@@ -265,6 +288,19 @@ static char *eth_addr2str(struct tipc_media_addr *a, char *str_buf, int str_size
 }
 
 /**
+ * do_registration - register TIPC to receive device notifications
+ *
+ * This routine must be invoked from a work queue because it can sleep.
+ */
+
+static void do_registration(struct work_struct *dummy)
+{
+	notifier.notifier_call = &recv_notification;
+	notifier.priority = 0;
+	register_netdevice_notifier(&notifier);
+}
+
+/**
  * tipc_eth_media_start - activate Ethernet bearer support
  *
  * Register Ethernet media type with TIPC bearer code.  Also register
@@ -291,11 +327,9 @@ int tipc_eth_media_start(void)
 	if (res)
 		return res;
 
-	notifier.notifier_call = &recv_notification;
-	notifier.priority = 0;
-	res = register_netdevice_notifier(&notifier);
-	if (!res)
-		eth_started = 1;
+	INIT_WORK(&reg_notifier, do_registration);
+	schedule_work(&reg_notifier);
+	eth_started = 1;
 	return res;
 }
 
@@ -305,22 +339,9 @@ int tipc_eth_media_start(void)
 
 void tipc_eth_media_stop(void)
 {
-	int i;
-
 	if (!eth_started)
 		return;
 
 	unregister_netdevice_notifier(&notifier);
-	for (i = 0; i < MAX_ETH_BEARERS ; i++) {
-		if (eth_bearers[i].bearer) {
-			eth_bearers[i].bearer->blocked = 1;
-			eth_bearers[i].bearer = NULL;
-		}
-		if (eth_bearers[i].dev) {
-			dev_remove_pack(&eth_bearers[i].tipc_packet_type);
-			dev_put(eth_bearers[i].dev);
-		}
-	}
-	memset(&eth_bearers, 0, sizeof(eth_bearers));
 	eth_started = 0;
 }
-- 
1.7.0.4

[PATCH net-next 2/5] tipc: Simplify bearer shutdown logic

[Paul Gortmaker]

From: Allan Stephens <allan.stephens@windriver.com>

Disable all active bearers when TIPC is shut down without having to do
a name-based search to locate each bearer object.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bearer.c |   61 ++++++++++++++++++++--------------------------------
 1 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 9c10c6b..9969ec6 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -280,39 +280,39 @@ static int bearer_name_validate(const char *name,
 }
 
 /**
- * bearer_find - locates bearer object with matching bearer name
+ * tipc_bearer_find_interface - locates bearer object with matching interface name
  */
 
-static struct bearer *bearer_find(const char *name)
+struct bearer *tipc_bearer_find_interface(const char *if_name)
 {
 	struct bearer *b_ptr;
+	char *b_if_name;
 	u32 i;
 
-	if (tipc_mode != TIPC_NET_MODE)
-		return NULL;
-
 	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
-		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
+		if (!b_ptr->active)
+			continue;
+		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
+		if (!strcmp(b_if_name, if_name))
 			return b_ptr;
 	}
 	return NULL;
 }
 
 /**
- * tipc_bearer_find_interface - locates bearer object with matching interface name
+ * bearer_find - locates bearer object with matching bearer name
  */
 
-struct bearer *tipc_bearer_find_interface(const char *if_name)
+static struct bearer *bearer_find(const char *name)
 {
 	struct bearer *b_ptr;
-	char *b_if_name;
 	u32 i;
 
+	if (tipc_mode != TIPC_NET_MODE)
+		return NULL;
+
 	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
-		if (!b_ptr->active)
-			continue;
-		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
-		if (!strcmp(b_if_name, if_name))
+		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
 			return b_ptr;
 	}
 	return NULL;
@@ -630,30 +630,17 @@ int tipc_block_bearer(const char *name)
  * Note: This routine assumes caller holds tipc_net_lock.
  */
 
-static int bearer_disable(const char *name)
+static int bearer_disable(struct bearer *b_ptr)
 {
-	struct bearer *b_ptr;
 	struct link *l_ptr;
 	struct link *temp_l_ptr;
 
-	b_ptr = bearer_find(name);
-	if (!b_ptr) {
-		warn("Attempt to disable unknown bearer <%s>\n", name);
-		return -EINVAL;
-	}
-
-	info("Disabling bearer <%s>\n", name);
+	info("Disabling bearer <%s>\n", b_ptr->publ.name);
 	tipc_disc_stop_link_req(b_ptr->link_req);
 	spin_lock_bh(&b_ptr->publ.lock);
 	b_ptr->link_req = NULL;
 	b_ptr->publ.blocked = 1;
-	if (b_ptr->media->disable_bearer) {
-		spin_unlock_bh(&b_ptr->publ.lock);
-		write_unlock_bh(&tipc_net_lock);
-		b_ptr->media->disable_bearer(&b_ptr->publ);
-		write_lock_bh(&tipc_net_lock);
-		spin_lock_bh(&b_ptr->publ.lock);
-	}
+	b_ptr->media->disable_bearer(&b_ptr->publ);
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
 		tipc_link_delete(l_ptr);
 	}
@@ -664,10 +651,16 @@ static int bearer_disable(const char *name)
 
 int tipc_disable_bearer(const char *name)
 {
+	struct bearer *b_ptr;
 	int res;
 
 	write_lock_bh(&tipc_net_lock);
-	res = bearer_disable(name);
+	b_ptr = bearer_find(name);
+	if (b_ptr == NULL) {
+		warn("Attempt to disable unknown bearer <%s>\n", name);
+		res = -EINVAL;
+	} else
+		res = bearer_disable(b_ptr);
 	write_unlock_bh(&tipc_net_lock);
 	return res;
 }
@@ -680,13 +673,7 @@ void tipc_bearer_stop(void)
 
 	for (i = 0; i < MAX_BEARERS; i++) {
 		if (tipc_bearers[i].active)
-			tipc_bearers[i].publ.blocked = 1;
-	}
-	for (i = 0; i < MAX_BEARERS; i++) {
-		if (tipc_bearers[i].active)
-			bearer_disable(tipc_bearers[i].publ.name);
+			bearer_disable(&tipc_bearers[i]);
 	}
 	media_count = 0;
 }
-
-
-- 
1.7.0.4

Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic

[Neil Horman]

On Tue, Oct 12, 2010 at 08:25:55PM -0400, Paul Gortmaker wrote:
> From: Allan Stephens <allan.stephens@windriver.com>
> 
> Disable all active bearers when TIPC is shut down without having to do
> a name-based search to locate each bearer object.
> 
It seems like you're doing a good deal more in this patch than just disabling
all active bearers without doing a name search.  The description is implemented
in the for loop of tipc_bearer_stop.  Whats the rest of it for?

> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |   61 ++++++++++++++++++++--------------------------------
>  1 files changed, 24 insertions(+), 37 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 9c10c6b..9969ec6 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -280,39 +280,39 @@ static int bearer_name_validate(const char *name,
>  }
>  
>  /**
> - * bearer_find - locates bearer object with matching bearer name
> + * tipc_bearer_find_interface - locates bearer object with matching interface name
>   */
>  
> -static struct bearer *bearer_find(const char *name)
> +struct bearer *tipc_bearer_find_interface(const char *if_name)
>  {
>  	struct bearer *b_ptr;
> +	char *b_if_name;
>  	u32 i;
>  
> -	if (tipc_mode != TIPC_NET_MODE)
> -		return NULL;
> -
>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
> -		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
> +		if (!b_ptr->active)
> +			continue;
> +		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
> +		if (!strcmp(b_if_name, if_name))
>  			return b_ptr;
>  	}
>  	return NULL;
>  }
>  
>  /**
> - * tipc_bearer_find_interface - locates bearer object with matching interface name
> + * bearer_find - locates bearer object with matching bearer name
>   */
>  
> -struct bearer *tipc_bearer_find_interface(const char *if_name)
> +static struct bearer *bearer_find(const char *name)
>  {
>  	struct bearer *b_ptr;
> -	char *b_if_name;
>  	u32 i;
>  
> +	if (tipc_mode != TIPC_NET_MODE)
> +		return NULL;
> +
I get why you're removing the tipc_mode check above, but why are you adding it
here?  commit d0021b252eaf65ca07ed14f0d66425dd9ccab9a6 modified tipc_bearers so
that calling this function should be tipc_mode safe (i.e. looking for a bearer
prior to entering TIPC_NET_MODE will return a NULL result).  Did you find a
subsequent problem?

>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
> -		if (!b_ptr->active)
> -			continue;
> -		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
> -		if (!strcmp(b_if_name, if_name))
> +		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
>  			return b_ptr;
>  	}
>  	return NULL;
> @@ -630,30 +630,17 @@ int tipc_block_bearer(const char *name)
>   * Note: This routine assumes caller holds tipc_net_lock.
>   */
>  
> -static int bearer_disable(const char *name)
> +static int bearer_disable(struct bearer *b_ptr)
>  {
> -	struct bearer *b_ptr;
>  	struct link *l_ptr;
>  	struct link *temp_l_ptr;
>  
> -	b_ptr = bearer_find(name);
> -	if (!b_ptr) {
> -		warn("Attempt to disable unknown bearer <%s>\n", name);
> -		return -EINVAL;
> -	}
> -
> -	info("Disabling bearer <%s>\n", name);
> +	info("Disabling bearer <%s>\n", b_ptr->publ.name);
>  	tipc_disc_stop_link_req(b_ptr->link_req);
>  	spin_lock_bh(&b_ptr->publ.lock);
>  	b_ptr->link_req = NULL;
>  	b_ptr->publ.blocked = 1;
> -	if (b_ptr->media->disable_bearer) {
> -		spin_unlock_bh(&b_ptr->publ.lock);
> -		write_unlock_bh(&tipc_net_lock);
> -		b_ptr->media->disable_bearer(&b_ptr->publ);
> -		write_lock_bh(&tipc_net_lock);
> -		spin_lock_bh(&b_ptr->publ.lock);
> -	}
> +	b_ptr->media->disable_bearer(&b_ptr->publ);
>  	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
>  		tipc_link_delete(l_ptr);
>  	}
> @@ -664,10 +651,16 @@ static int bearer_disable(const char *name)
>  
>  int tipc_disable_bearer(const char *name)
>  {
> +	struct bearer *b_ptr;
>  	int res;
>  
>  	write_lock_bh(&tipc_net_lock);
> -	res = bearer_disable(name);
> +	b_ptr = bearer_find(name);
> +	if (b_ptr == NULL) {
> +		warn("Attempt to disable unknown bearer <%s>\n", name);
> +		res = -EINVAL;
> +	} else
> +		res = bearer_disable(b_ptr);
>  	write_unlock_bh(&tipc_net_lock);
>  	return res;
>  }
> @@ -680,13 +673,7 @@ void tipc_bearer_stop(void)
>  
>  	for (i = 0; i < MAX_BEARERS; i++) {
>  		if (tipc_bearers[i].active)
> -			tipc_bearers[i].publ.blocked = 1;
> -	}
> -	for (i = 0; i < MAX_BEARERS; i++) {
> -		if (tipc_bearers[i].active)
> -			bearer_disable(tipc_bearers[i].publ.name);
> +			bearer_disable(&tipc_bearers[i]);
>  	}
>  	media_count = 0;
>  }
> -
> -
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic

[Paul Gortmaker]

[Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic] On 13/10/2010 (Wed 10:39) Neil Horman wrote:

> On Tue, Oct 12, 2010 at 08:25:55PM -0400, Paul Gortmaker wrote:
> > From: Allan Stephens <allan.stephens@windriver.com>
> > 
> > Disable all active bearers when TIPC is shut down without having to do
> > a name-based search to locate each bearer object.
> > 
> It seems like you're doing a good deal more in this patch than just disabling
> all active bearers without doing a name search.  The description is implemented
> in the for loop of tipc_bearer_stop.  Whats the rest of it for?

It seems the original needlessly bloated out the patch size by
swapping the order of tipc_bearer_find_interface & bearer_find
in the file (now fixed) - and you are right, the locking change
wasn't properly covered in the commit log.  The extra test you'd
suggested tossing out is also now gone.

This change doesn't explicitly depend on any other changes,
so if it is now OK, the option is there for it to be applied
independently of the others that haven't been reworked yet.

Thanks,
Paul.


>From 1771ad642cb076dbeb71e3533a25cb2f07df9cd8 Mon Sep 17 00:00:00 2001
From: Allan Stephens <allan.stephens@windriver.com>
Date: Sat, 4 Sep 2010 09:29:04 -0400
Subject: [PATCH] tipc: Simplify bearer shutdown logic

Optimize processing in TIPC's bearer shutdown code, including:

1. Remove an unnecessary check to see if TIPC bearer's can exist.
2. Don't release spinlocks before calling a media-specific disabling
routine, since the routine can't sleep.
3. Make bearer_disable() operate directly on a struct bearer, instead
of needlessly taking a name and then mapping that to the struct.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bearer.c |   38 +++++++++++---------------------------
 1 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 9c10c6b..fd9c06c 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -288,9 +288,6 @@ static struct bearer *bearer_find(const char *name)
 	struct bearer *b_ptr;
 	u32 i;
 
-	if (tipc_mode != TIPC_NET_MODE)
-		return NULL;
-
 	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
 		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
 			return b_ptr;
@@ -630,30 +627,17 @@ int tipc_block_bearer(const char *name)
  * Note: This routine assumes caller holds tipc_net_lock.
  */
 
-static int bearer_disable(const char *name)
+static int bearer_disable(struct bearer *b_ptr)
 {
-	struct bearer *b_ptr;
 	struct link *l_ptr;
 	struct link *temp_l_ptr;
 
-	b_ptr = bearer_find(name);
-	if (!b_ptr) {
-		warn("Attempt to disable unknown bearer <%s>\n", name);
-		return -EINVAL;
-	}
-
-	info("Disabling bearer <%s>\n", name);
+	info("Disabling bearer <%s>\n", b_ptr->publ.name);
 	tipc_disc_stop_link_req(b_ptr->link_req);
 	spin_lock_bh(&b_ptr->publ.lock);
 	b_ptr->link_req = NULL;
 	b_ptr->publ.blocked = 1;
-	if (b_ptr->media->disable_bearer) {
-		spin_unlock_bh(&b_ptr->publ.lock);
-		write_unlock_bh(&tipc_net_lock);
-		b_ptr->media->disable_bearer(&b_ptr->publ);
-		write_lock_bh(&tipc_net_lock);
-		spin_lock_bh(&b_ptr->publ.lock);
-	}
+	b_ptr->media->disable_bearer(&b_ptr->publ);
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
 		tipc_link_delete(l_ptr);
 	}
@@ -664,10 +648,16 @@ static int bearer_disable(const char *name)
 
 int tipc_disable_bearer(const char *name)
 {
+	struct bearer *b_ptr;
 	int res;
 
 	write_lock_bh(&tipc_net_lock);
-	res = bearer_disable(name);
+	b_ptr = bearer_find(name);
+	if (b_ptr == NULL) {
+		warn("Attempt to disable unknown bearer <%s>\n", name);
+		res = -EINVAL;
+	} else
+		res = bearer_disable(b_ptr);
 	write_unlock_bh(&tipc_net_lock);
 	return res;
 }
@@ -680,13 +670,7 @@ void tipc_bearer_stop(void)
 
 	for (i = 0; i < MAX_BEARERS; i++) {
 		if (tipc_bearers[i].active)
-			tipc_bearers[i].publ.blocked = 1;
-	}
-	for (i = 0; i < MAX_BEARERS; i++) {
-		if (tipc_bearers[i].active)
-			bearer_disable(tipc_bearers[i].publ.name);
+			bearer_disable(&tipc_bearers[i]);
 	}
 	media_count = 0;
 }
-
-
-- 
1.7.2.1

Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic

[Neil Horman]

On Thu, Oct 14, 2010 at 07:58:26PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 2/5] tipc: Simplify bearer shutdown logic] On 13/10/2010 (Wed 10:39) Neil Horman wrote:
> 
> > On Tue, Oct 12, 2010 at 08:25:55PM -0400, Paul Gortmaker wrote:
> > > From: Allan Stephens <allan.stephens@windriver.com>
> > > 
> > > Disable all active bearers when TIPC is shut down without having to do
> > > a name-based search to locate each bearer object.
> > > 
> > It seems like you're doing a good deal more in this patch than just disabling
> > all active bearers without doing a name search.  The description is implemented
> > in the for loop of tipc_bearer_stop.  Whats the rest of it for?
> 
> It seems the original needlessly bloated out the patch size by
> swapping the order of tipc_bearer_find_interface & bearer_find
> in the file (now fixed) - and you are right, the locking change
> wasn't properly covered in the commit log.  The extra test you'd
> suggested tossing out is also now gone.
> 
> This change doesn't explicitly depend on any other changes,
> so if it is now OK, the option is there for it to be applied
> independently of the others that haven't been reworked yet.
> 
> Thanks,
> Paul.
> 
> 
> From 1771ad642cb076dbeb71e3533a25cb2f07df9cd8 Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Sat, 4 Sep 2010 09:29:04 -0400
> Subject: [PATCH] tipc: Simplify bearer shutdown logic
> 
> Optimize processing in TIPC's bearer shutdown code, including:
> 
> 1. Remove an unnecessary check to see if TIPC bearer's can exist.
> 2. Don't release spinlocks before calling a media-specific disabling
> routine, since the routine can't sleep.
> 3. Make bearer_disable() operate directly on a struct bearer, instead
> of needlessly taking a name and then mapping that to the struct.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |   38 +++++++++++---------------------------
>  1 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 9c10c6b..fd9c06c 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -288,9 +288,6 @@ static struct bearer *bearer_find(const char *name)
>  	struct bearer *b_ptr;
>  	u32 i;
>  
> -	if (tipc_mode != TIPC_NET_MODE)
> -		return NULL;
> -
>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
>  		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
>  			return b_ptr;
> @@ -630,30 +627,17 @@ int tipc_block_bearer(const char *name)
>   * Note: This routine assumes caller holds tipc_net_lock.
>   */
>  
> -static int bearer_disable(const char *name)
> +static int bearer_disable(struct bearer *b_ptr)
>  {
> -	struct bearer *b_ptr;
>  	struct link *l_ptr;
>  	struct link *temp_l_ptr;
>  
> -	b_ptr = bearer_find(name);
> -	if (!b_ptr) {
> -		warn("Attempt to disable unknown bearer <%s>\n", name);
> -		return -EINVAL;
> -	}
> -
> -	info("Disabling bearer <%s>\n", name);
> +	info("Disabling bearer <%s>\n", b_ptr->publ.name);
>  	tipc_disc_stop_link_req(b_ptr->link_req);
>  	spin_lock_bh(&b_ptr->publ.lock);
>  	b_ptr->link_req = NULL;
>  	b_ptr->publ.blocked = 1;
> -	if (b_ptr->media->disable_bearer) {
> -		spin_unlock_bh(&b_ptr->publ.lock);
> -		write_unlock_bh(&tipc_net_lock);
> -		b_ptr->media->disable_bearer(&b_ptr->publ);
> -		write_lock_bh(&tipc_net_lock);
> -		spin_lock_bh(&b_ptr->publ.lock);
> -	}
> +	b_ptr->media->disable_bearer(&b_ptr->publ);
>  	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
>  		tipc_link_delete(l_ptr);
>  	}
> @@ -664,10 +648,16 @@ static int bearer_disable(const char *name)
>  
>  int tipc_disable_bearer(const char *name)
>  {
> +	struct bearer *b_ptr;
>  	int res;
>  
>  	write_lock_bh(&tipc_net_lock);
> -	res = bearer_disable(name);
> +	b_ptr = bearer_find(name);
> +	if (b_ptr == NULL) {
> +		warn("Attempt to disable unknown bearer <%s>\n", name);
> +		res = -EINVAL;
> +	} else
> +		res = bearer_disable(b_ptr);
>  	write_unlock_bh(&tipc_net_lock);
>  	return res;
>  }
> @@ -680,13 +670,7 @@ void tipc_bearer_stop(void)
>  
>  	for (i = 0; i < MAX_BEARERS; i++) {
>  		if (tipc_bearers[i].active)
> -			tipc_bearers[i].publ.blocked = 1;
> -	}
> -	for (i = 0; i < MAX_BEARERS; i++) {
> -		if (tipc_bearers[i].active)
> -			bearer_disable(tipc_bearers[i].publ.name);
> +			bearer_disable(&tipc_bearers[i]);
>  	}
>  	media_count = 0;
>  }
> -
> -
> -- 
> 1.7.2.1
> 
> 

Yes, this looks much better, thank you.
Reviewed-by: Neil Horman <nhorman@tuxdriver.com>

[PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Paul Gortmaker]

From: Allan Stephens <allan.stephens@windriver.com>

Introduces "enabling" state during activation of a new TIPC bearer,
which supplements the existing "disabled" and "enabled" states.
This change allows the new bearer to be added without having to
temporarily block the processing of incoming packets on existing
bearers during the binding of the new bearer to its associated
interface. It also makes it unnecessary to zero out the entire
bearer structure at the start of activation.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c  |    2 +-
 net/tipc/bearer.c |   27 +++++++++++++++++----------
 net/tipc/bearer.h |    8 ++++++--
 net/tipc/link.c   |    2 +-
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index ecfaac1..ba6dcb2 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -645,7 +645,7 @@ void tipc_bcbearer_sort(void)
 	for (b_index = 0; b_index < MAX_BEARERS; b_index++) {
 		struct bearer *b = &tipc_bearers[b_index];
 
-		if (!b->active || !b->nodes.count)
+		if ((b->state != BEARER_ENABLED) || !b->nodes.count)
 			continue;
 
 		if (!bp_temp[b->priority].primary)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 9969ec6..379338f 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -290,7 +290,7 @@ struct bearer *tipc_bearer_find_interface(const char *if_name)
 	u32 i;
 
 	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
-		if (!b_ptr->active)
+		if (b_ptr->state != BEARER_ENABLED)
 			continue;
 		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
 		if (!strcmp(b_if_name, if_name))
@@ -312,7 +312,8 @@ static struct bearer *bearer_find(const char *name)
 		return NULL;
 
 	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
-		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
+		if ((b_ptr->state == BEARER_ENABLED) &&
+		    (!strcmp(b_ptr->publ.name, name)))
 			return b_ptr;
 	}
 	return NULL;
@@ -337,7 +338,8 @@ struct sk_buff *tipc_bearer_get_names(void)
 	for (i = 0, m_ptr = media_list; i < media_count; i++, m_ptr++) {
 		for (j = 0; j < MAX_BEARERS; j++) {
 			b_ptr = &tipc_bearers[j];
-			if (b_ptr->active && (b_ptr->media == m_ptr)) {
+			if ((b_ptr->state == BEARER_ENABLED) &&
+			    (b_ptr->media == m_ptr)) {
 				tipc_cfg_append_tlv(buf, TIPC_TLV_BEARER_NAME,
 						    b_ptr->publ.name,
 						    strlen(b_ptr->publ.name) + 1);
@@ -532,7 +534,7 @@ restart:
 	bearer_id = MAX_BEARERS;
 	with_this_prio = 1;
 	for (i = MAX_BEARERS; i-- != 0; ) {
-		if (!tipc_bearers[i].active) {
+		if (tipc_bearers[i].state != BEARER_ENABLED) {
 			bearer_id = i;
 			continue;
 		}
@@ -559,21 +561,23 @@ restart:
 	}
 
 	b_ptr = &tipc_bearers[bearer_id];
-	memset(b_ptr, 0, sizeof(struct bearer));
-
+	b_ptr->state = BEARER_ENABLING;
 	strcpy(b_ptr->publ.name, name);
+	b_ptr->priority = priority;
+
+	write_unlock_bh(&tipc_net_lock);
 	res = m_ptr->enable_bearer(&b_ptr->publ);
 	if (res) {
+		b_ptr->state = BEARER_DISABLED;
 		warn("Bearer <%s> rejected, enable failure (%d)\n", name, -res);
-		goto failed;
+		return res;
 	}
+	write_lock_bh(&tipc_net_lock);
 
 	b_ptr->identity = bearer_id;
 	b_ptr->media = m_ptr;
 	b_ptr->net_plane = bearer_id + 'A';
-	b_ptr->active = 1;
 	b_ptr->detect_scope = bcast_scope;
-	b_ptr->priority = priority;
 	INIT_LIST_HEAD(&b_ptr->cong_links);
 	INIT_LIST_HEAD(&b_ptr->links);
 	if (m_ptr->bcast) {
@@ -581,7 +585,10 @@ restart:
 							  bcast_scope, 2);
 	}
 	spin_lock_init(&b_ptr->publ.lock);
+	b_ptr->state = BEARER_ENABLED;
+
 	write_unlock_bh(&tipc_net_lock);
+
 	info("Enabled bearer <%s>, discovery domain %s, priority %u\n",
 	     name, tipc_addr_string_fill(addr_string, bcast_scope), priority);
 	return 0;
@@ -672,7 +679,7 @@ void tipc_bearer_stop(void)
 	u32 i;
 
 	for (i = 0; i < MAX_BEARERS; i++) {
-		if (tipc_bearers[i].active)
+		if (tipc_bearers[i].state == BEARER_ENABLED)
 			bearer_disable(&tipc_bearers[i]);
 	}
 	media_count = 0;
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index a850b38..134c6cb 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -43,6 +43,10 @@
 #define MAX_BEARERS 8
 #define MAX_MEDIA 4
 
+/* Bearer state */
+#define BEARER_DISABLED	0
+#define BEARER_ENABLING	1
+#define BEARER_ENABLED		2
 
 /**
  * struct media - TIPC media information available to internal users
@@ -87,7 +91,7 @@ struct media {
  * @links: list of non-congested links associated with bearer
  * @cong_links: list of congested links associated with bearer
  * @continue_count: # of times bearer has resumed after congestion or blocking
- * @active: non-zero if bearer structure is represents a bearer
+ * @state: bearer state (disabled, enabling, enabled)
  * @net_plane: network plane ('A' through 'H') currently associated with bearer
  * @nodes: indicates which nodes in cluster can be reached through bearer
  */
@@ -102,7 +106,7 @@ struct bearer {
 	struct list_head links;
 	struct list_head cong_links;
 	u32 continue_count;
-	int active;
+	unsigned char state;
 	char net_plane;
 	struct tipc_node_map nodes;
 };
diff --git a/net/tipc/link.c b/net/tipc/link.c
index b8cf1e9..54bd99d 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1830,7 +1830,7 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *tb_ptr)
 
 		/* Ensure bearer is still enabled */
 
-		if (unlikely(!b_ptr->active))
+		if (unlikely(b_ptr->state != BEARER_ENABLED))
 			goto cont;
 
 		/* Ensure message is well-formed */
-- 
1.7.0.4

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Neil Horman]

On Tue, Oct 12, 2010 at 08:25:56PM -0400, Paul Gortmaker wrote:
> From: Allan Stephens <allan.stephens@windriver.com>
> 
> Introduces "enabling" state during activation of a new TIPC bearer,
> which supplements the existing "disabled" and "enabled" states.
> This change allows the new bearer to be added without having to
> temporarily block the processing of incoming packets on existing
> bearers during the binding of the new bearer to its associated
> interface. It also makes it unnecessary to zero out the entire
> bearer structure at the start of activation.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bcast.c  |    2 +-
>  net/tipc/bearer.c |   27 +++++++++++++++++----------
>  net/tipc/bearer.h |    8 ++++++--
>  net/tipc/link.c   |    2 +-
>  4 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index ecfaac1..ba6dcb2 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -645,7 +645,7 @@ void tipc_bcbearer_sort(void)
>  	for (b_index = 0; b_index < MAX_BEARERS; b_index++) {
>  		struct bearer *b = &tipc_bearers[b_index];
>  
> -		if (!b->active || !b->nodes.count)
> +		if ((b->state != BEARER_ENABLED) || !b->nodes.count)
>  			continue;
>  
>  		if (!bp_temp[b->priority].primary)
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 9969ec6..379338f 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -290,7 +290,7 @@ struct bearer *tipc_bearer_find_interface(const char *if_name)
>  	u32 i;
>  
>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
> -		if (!b_ptr->active)
> +		if (b_ptr->state != BEARER_ENABLED)
>  			continue;
>  		b_if_name = strchr(b_ptr->publ.name, ':') + 1;
>  		if (!strcmp(b_if_name, if_name))
> @@ -312,7 +312,8 @@ static struct bearer *bearer_find(const char *name)
>  		return NULL;
>  
>  	for (i = 0, b_ptr = tipc_bearers; i < MAX_BEARERS; i++, b_ptr++) {
> -		if (b_ptr->active && (!strcmp(b_ptr->publ.name, name)))
> +		if ((b_ptr->state == BEARER_ENABLED) &&
> +		    (!strcmp(b_ptr->publ.name, name)))
>  			return b_ptr;
>  	}
>  	return NULL;
> @@ -337,7 +338,8 @@ struct sk_buff *tipc_bearer_get_names(void)
>  	for (i = 0, m_ptr = media_list; i < media_count; i++, m_ptr++) {
>  		for (j = 0; j < MAX_BEARERS; j++) {
>  			b_ptr = &tipc_bearers[j];
> -			if (b_ptr->active && (b_ptr->media == m_ptr)) {
> +			if ((b_ptr->state == BEARER_ENABLED) &&
> +			    (b_ptr->media == m_ptr)) {
>  				tipc_cfg_append_tlv(buf, TIPC_TLV_BEARER_NAME,
>  						    b_ptr->publ.name,
>  						    strlen(b_ptr->publ.name) + 1);
> @@ -532,7 +534,7 @@ restart:
>  	bearer_id = MAX_BEARERS;
>  	with_this_prio = 1;
>  	for (i = MAX_BEARERS; i-- != 0; ) {
> -		if (!tipc_bearers[i].active) {
> +		if (tipc_bearers[i].state != BEARER_ENABLED) {
>  			bearer_id = i;
>  			continue;
>  		}
> @@ -559,21 +561,23 @@ restart:
>  	}
>  
>  	b_ptr = &tipc_bearers[bearer_id];
> -	memset(b_ptr, 0, sizeof(struct bearer));
> -
> +	b_ptr->state = BEARER_ENABLING;
>  	strcpy(b_ptr->publ.name, name);
> +	b_ptr->priority = priority;
> +
> +	write_unlock_bh(&tipc_net_lock);
Why the 3rd state?  Doesn't seem needed. 

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Paul Gortmaker]

[Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 13/10/2010 (Wed 10:58) Neil Horman wrote:

> On Tue, Oct 12, 2010 at 08:25:56PM -0400, Paul Gortmaker wrote:
> > From: Allan Stephens <allan.stephens@windriver.com>
> > 
> > Introduces "enabling" state during activation of a new TIPC bearer,
> > which supplements the existing "disabled" and "enabled" states.
> > This change allows the new bearer to be added without having to
> > temporarily block the processing of incoming packets on existing
> > bearers during the binding of the new bearer to its associated
> > interface. It also makes it unnecessary to zero out the entire
> > bearer structure at the start of activation.
> > 

[...]

> > +	b_ptr->state = BEARER_ENABLING;
> >  	strcpy(b_ptr->publ.name, name);
> > +	b_ptr->priority = priority;
> > +
> > +	write_unlock_bh(&tipc_net_lock);
> Why the 3rd state?  Doesn't seem needed. 

I'm a bit disappointed in myself for also not noticing that it
was set but never tested for.  The following should give the
same end result but without the obfuscation of an extra state.

This one also doesn't explicitly depend on any other changes,
so if it is now OK, the option is there for it to be applied
independently of the others that haven't been reworked yet.

Thanks,
Paul.


>From 86d0d5c92439d0a3f5a0f165aa8bd842d377dae9 Mon Sep 17 00:00:00 2001
From: Allan Stephens <allan.stephens@windriver.com>
Date: Thu, 14 Oct 2010 16:09:23 -0400
Subject: [PATCH] tipc: Optimizations to bearer enabling logic

Allow new bearers to be added without having to temporarily block
the processing of incoming packets on existing bearers during the
binding of the new bearer to its associated interface. Eliminates
zeroing out of the new bearer structure at the start of activation,
since it is already in that state.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bearer.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index fd9c06c..2ff8181 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -556,14 +556,15 @@ restart:
 	}
 
 	b_ptr = &tipc_bearers[bearer_id];
-	memset(b_ptr, 0, sizeof(struct bearer));
-
 	strcpy(b_ptr->publ.name, name);
+
+	write_unlock_bh(&tipc_net_lock);
 	res = m_ptr->enable_bearer(&b_ptr->publ);
 	if (res) {
 		warn("Bearer <%s> rejected, enable failure (%d)\n", name, -res);
-		goto failed;
+		return res;
 	}
+	write_lock_bh(&tipc_net_lock);
 
 	b_ptr->identity = bearer_id;
 	b_ptr->media = m_ptr;
-- 
1.7.2.1

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Neil Horman]

On Thu, Oct 14, 2010 at 09:11:51PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 13/10/2010 (Wed 10:58) Neil Horman wrote:
> 
> > On Tue, Oct 12, 2010 at 08:25:56PM -0400, Paul Gortmaker wrote:
> > > From: Allan Stephens <allan.stephens@windriver.com>
> > > 
> > > Introduces "enabling" state during activation of a new TIPC bearer,
> > > which supplements the existing "disabled" and "enabled" states.
> > > This change allows the new bearer to be added without having to
> > > temporarily block the processing of incoming packets on existing
> > > bearers during the binding of the new bearer to its associated
> > > interface. It also makes it unnecessary to zero out the entire
> > > bearer structure at the start of activation.
> > > 
> 
> [...]
> 
> > > +	b_ptr->state = BEARER_ENABLING;
> > >  	strcpy(b_ptr->publ.name, name);
> > > +	b_ptr->priority = priority;
> > > +
> > > +	write_unlock_bh(&tipc_net_lock);
> > Why the 3rd state?  Doesn't seem needed. 
> 
> I'm a bit disappointed in myself for also not noticing that it
> was set but never tested for.  The following should give the
> same end result but without the obfuscation of an extra state.
> 
> This one also doesn't explicitly depend on any other changes,
> so if it is now OK, the option is there for it to be applied
> independently of the others that haven't been reworked yet.
> 
> Thanks,
> Paul.
> 
> 
> From 86d0d5c92439d0a3f5a0f165aa8bd842d377dae9 Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: Optimizations to bearer enabling logic
> 
> Allow new bearers to be added without having to temporarily block
> the processing of incoming packets on existing bearers during the
> binding of the new bearer to its associated interface. Eliminates
> zeroing out of the new bearer structure at the start of activation,
> since it is already in that state.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index fd9c06c..2ff8181 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -556,14 +556,15 @@ restart:
>  	}
>  
>  	b_ptr = &tipc_bearers[bearer_id];
> -	memset(b_ptr, 0, sizeof(struct bearer));
> -
>  	strcpy(b_ptr->publ.name, name);
> +
> +	write_unlock_bh(&tipc_net_lock);
>  	res = m_ptr->enable_bearer(&b_ptr->publ);
>  	if (res) {
>  		warn("Bearer <%s> rejected, enable failure (%d)\n", name, -res);
> -		goto failed;
> +		return res;
>  	}
> +	write_lock_bh(&tipc_net_lock);
>  
This definately looks more concise, but I don't see why its necessecary to drop
the tipc_net_lock around the call to enable_bearer.  The only caler of
tipc_register_media sets the enable_bearer pointer to the enable_bearer
function, and  I' don't see any path through that function which would
potentially retake that lock.  In fact it seems dropping it might be dangerous,
if other paths (like from cfg_named_msg_event), tried to enable a bearer in
parallel with a user space directive from the netlink socket).  With out the
protection of tipc_net_lock, you could use the same eth_bearer twice, and
corrupt that array.

Neil

>  	b_ptr->identity = bearer_id;
>  	b_ptr->media = m_ptr;
> -- 
> 1.7.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Paul Gortmaker]

On 10-10-15 07:00 AM, Neil Horman wrote:

[...]

> This definately looks more concise, but I don't see why its necessecary to drop
> the tipc_net_lock around the call to enable_bearer.  The only caler of
> tipc_register_media sets the enable_bearer pointer to the enable_bearer
> function, and  I' don't see any path through that function which would
> potentially retake that lock.  In fact it seems dropping it might be dangerous,
> if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> parallel with a user space directive from the netlink socket).  With out the
> protection of tipc_net_lock, you could use the same eth_bearer twice, and
> corrupt that array.

I think it would be protected by config_lock. but in the end if it is
just an academic optimization that really isn't in a hot code path
anyway, and if it just adds more confusion than real world value, then
I'm fine with just dropping this on the floor (and deleting the extra
memset in a cleanup patch) -- it won't be the 1st time doing this with
these carry forward patches, and I'm sure it won't be the last.

Thanks,
Paul.

> 
> Neil
> 

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Neil Horman]

On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> On 10-10-15 07:00 AM, Neil Horman wrote:
> 
> [...]
> 
> > This definately looks more concise, but I don't see why its necessecary to drop
> > the tipc_net_lock around the call to enable_bearer.  The only caler of
> > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > function, and  I' don't see any path through that function which would
> > potentially retake that lock.  In fact it seems dropping it might be dangerous,
> > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > parallel with a user space directive from the netlink socket).  With out the
> > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > corrupt that array.
> 
> I think it would be protected by config_lock. but in the end if it is
> just an academic optimization that really isn't in a hot code path
> anyway, and if it just adds more confusion than real world value, then
> I'm fine with just dropping this on the floor (and deleting the extra
> memset in a cleanup patch) -- it won't be the 1st time doing this with
> these carry forward patches, and I'm sure it won't be the last.
> 

Copy that.


> 

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Paul Gortmaker]

[Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 18/10/2010 (Mon 06:50) Neil Horman wrote:

> On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> > On 10-10-15 07:00 AM, Neil Horman wrote:
> > 
> > [...]
> > 
> > > This definately looks more concise, but I don't see why its necessecary to drop
> > > the tipc_net_lock around the call to enable_bearer.  The only caler of
> > > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > > function, and  I' don't see any path through that function which would
> > > potentially retake that lock.  In fact it seems dropping it might be dangerous,
> > > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > > parallel with a user space directive from the netlink socket).  With out the
> > > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > > corrupt that array.
> > 
> > I think it would be protected by config_lock. but in the end if it is
> > just an academic optimization that really isn't in a hot code path
> > anyway, and if it just adds more confusion than real world value, then
> > I'm fine with just dropping this on the floor (and deleting the extra
> > memset in a cleanup patch) -- it won't be the 1st time doing this with
> > these carry forward patches, and I'm sure it won't be the last.
> > 
> 
> Copy that.

And here is all that is left once I drop all the above.  Not much. :)

Thanks again,
Paul.

>From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
From: Allan Stephens <allan.stephens@windriver.com>
Date: Thu, 14 Oct 2010 16:09:23 -0400
Subject: [PATCH] tipc: delete needless memset from bearer enabling.

Eliminates zeroing out of the new bearer structure at the start of
activation, since it is already in that state.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bearer.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index fd9c06c..9927d1d 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -556,8 +556,6 @@ restart:
 	}
 
 	b_ptr = &tipc_bearers[bearer_id];
-	memset(b_ptr, 0, sizeof(struct bearer));
-
 	strcpy(b_ptr->publ.name, name);
 	res = m_ptr->enable_bearer(&b_ptr->publ);
 	if (res) {
-- 
1.7.2.1

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[Neil Horman]

On Mon, Oct 18, 2010 at 05:43:56PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 18/10/2010 (Mon 06:50) Neil Horman wrote:
> 
> > On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> > > On 10-10-15 07:00 AM, Neil Horman wrote:
> > > 
> > > [...]
> > > 
> > > > This definately looks more concise, but I don't see why its necessecary to drop
> > > > the tipc_net_lock around the call to enable_bearer.  The only caler of
> > > > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > > > function, and  I' don't see any path through that function which would
> > > > potentially retake that lock.  In fact it seems dropping it might be dangerous,
> > > > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > > > parallel with a user space directive from the netlink socket).  With out the
> > > > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > > > corrupt that array.
> > > 
> > > I think it would be protected by config_lock. but in the end if it is
> > > just an academic optimization that really isn't in a hot code path
> > > anyway, and if it just adds more confusion than real world value, then
> > > I'm fine with just dropping this on the floor (and deleting the extra
> > > memset in a cleanup patch) -- it won't be the 1st time doing this with
> > > these carry forward patches, and I'm sure it won't be the last.
> > > 
> > 
> > Copy that.
> 
> And here is all that is left once I drop all the above.  Not much. :)
> 
> Thanks again,
> Paul.
> 
> From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: delete needless memset from bearer enabling.
> 
> Eliminates zeroing out of the new bearer structure at the start of
> activation, since it is already in that state.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bearer.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index fd9c06c..9927d1d 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -556,8 +556,6 @@ restart:
>  	}
>  
>  	b_ptr = &tipc_bearers[bearer_id];
> -	memset(b_ptr, 0, sizeof(struct bearer));
> -
>  	strcpy(b_ptr->publ.name, name);
>  	res = m_ptr->enable_bearer(&b_ptr->publ);
>  	if (res) {
> -- 
> 1.7.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic

[David Miller]

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Mon, 18 Oct 2010 17:43:56 -0400

>>From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: delete needless memset from bearer enabling.
> 
> Eliminates zeroing out of the new bearer structure at the start of
> activation, since it is already in that state.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Applied, thanks.

[PATCH net-next 4/5] tipc: Rework data structures that track neighboring nodes and links

[Paul Gortmaker]

From: Allan Stephens <allan.stephens@windriver.com>

Convert existing linked list of neighboring nodes to a standard
doubly-linked list. Add counters that track total number of nodes
in list and total number of links to these nodes, thereby allowing
configuration message replies to allocate only space based on
the actual number of nodes and links rather than the worst case.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |    5 ++-
 net/tipc/link.c  |    3 +-
 net/tipc/node.c  |   60 ++++++++++++++++++++++++++---------------------------
 net/tipc/node.h  |    5 +--
 4 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index ba6dcb2..e006678 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -454,10 +454,11 @@ void tipc_bclink_recv_pkt(struct sk_buff *buf)
 			tipc_node_unlock(node);
 			spin_lock_bh(&bc_lock);
 			bcl->stats.recv_nacks++;
-			bcl->owner->next = node;   /* remember requestor */
+			/* remember retransmit requester */
+			bcl->owner->node_list.next =
+				(struct list_head *)node;
 			bclink_retransmit_pkt(msg_bcgap_after(msg),
 					      msg_bcgap_to(msg));
-			bcl->owner->next = NULL;
 			spin_unlock_bh(&bc_lock);
 		} else {
 			tipc_bclink_peek_nack(msg_destnode(msg),
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 54bd99d..13bc0b6 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1652,7 +1652,8 @@ static void link_retransmit_failure(struct link *l_ptr, struct sk_buff *buf)
 		tipc_printf(TIPC_OUTPUT, "Outstanding acks: %lu\n",
 				     (unsigned long) TIPC_SKB_CB(buf)->handle);
 
-		n_ptr = l_ptr->owner->next;
+		/* recover retransmit requester */
+		n_ptr = (struct tipc_node *)l_ptr->owner->node_list.next;
 		tipc_node_lock(n_ptr);
 
 		tipc_addr_string_fill(addr_string, n_ptr->addr);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 7c49cd0..944b480 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -50,7 +50,9 @@ void node_print(struct print_buf *buf, struct tipc_node *n_ptr, char *str);
 static void node_lost_contact(struct tipc_node *n_ptr);
 static void node_established_contact(struct tipc_node *n_ptr);
 
-struct tipc_node *tipc_nodes = NULL;	/* sorted list of nodes within cluster */
+static LIST_HEAD(nodes_list);	/* sorted list of neighboring nodes */
+static int node_count;	/* number of neighboring nodes that exist */
+static int link_count;	/* number of unicast links node currently has */
 
 static DEFINE_SPINLOCK(node_create_lock);
 
@@ -70,11 +72,11 @@ struct tipc_node *tipc_node_create(u32 addr)
 {
 	struct cluster *c_ptr;
 	struct tipc_node *n_ptr;
-	struct tipc_node **curr_node;
+	struct tipc_node *new_n_ptr;
 
 	spin_lock_bh(&node_create_lock);
 
-	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
+	list_for_each_entry(n_ptr, &nodes_list, node_list) {
 		if (addr < n_ptr->addr)
 			break;
 		if (addr == n_ptr->addr) {
@@ -83,8 +85,8 @@ struct tipc_node *tipc_node_create(u32 addr)
 		}
 	}
 
-	n_ptr = kzalloc(sizeof(*n_ptr),GFP_ATOMIC);
-	if (!n_ptr) {
+	new_n_ptr = kzalloc(sizeof(*new_n_ptr), GFP_ATOMIC);
+	if (!new_n_ptr) {
 		spin_unlock_bh(&node_create_lock);
 		warn("Node creation failed, no memory\n");
 		return NULL;
@@ -96,28 +98,22 @@ struct tipc_node *tipc_node_create(u32 addr)
 	}
 	if (!c_ptr) {
 		spin_unlock_bh(&node_create_lock);
-		kfree(n_ptr);
+		kfree(new_n_ptr);
 		return NULL;
 	}
 
-	n_ptr->addr = addr;
-		spin_lock_init(&n_ptr->lock);
-	INIT_LIST_HEAD(&n_ptr->nsub);
-	n_ptr->owner = c_ptr;
-	tipc_cltr_attach_node(c_ptr, n_ptr);
-	n_ptr->last_router = -1;
+	new_n_ptr->addr = addr;
+	spin_lock_init(&new_n_ptr->lock);
+	INIT_LIST_HEAD(&new_n_ptr->nsub);
+	new_n_ptr->owner = c_ptr;
+	tipc_cltr_attach_node(c_ptr, new_n_ptr);
+	new_n_ptr->last_router = -1;
+
+	list_add_tail(&new_n_ptr->node_list, &n_ptr->node_list);
+	node_count++;
 
-	/* Insert node into ordered list */
-	for (curr_node = &tipc_nodes; *curr_node;
-	     curr_node = &(*curr_node)->next) {
-		if (addr < (*curr_node)->addr) {
-			n_ptr->next = *curr_node;
-			break;
-		}
-	}
-	(*curr_node) = n_ptr;
 	spin_unlock_bh(&node_create_lock);
-	return n_ptr;
+	return new_n_ptr;
 }
 
 void tipc_node_delete(struct tipc_node *n_ptr)
@@ -136,6 +132,8 @@ void tipc_node_delete(struct tipc_node *n_ptr)
 #endif
 
 	dbg("node %x deleted\n", n_ptr->addr);
+	node_count--;
+	list_del(&n_ptr->node_list);
 	kfree(n_ptr);
 }
 
@@ -275,6 +273,7 @@ struct tipc_node *tipc_node_attach_link(struct link *l_ptr)
 			n_ptr->links[bearer_id] = l_ptr;
 			tipc_net.zones[tipc_zone(l_ptr->addr)]->links++;
 			n_ptr->link_cnt++;
+			link_count++;
 			return n_ptr;
 		}
 		err("Attempt to establish second link on <%s> to %s\n",
@@ -289,6 +288,7 @@ void tipc_node_detach_link(struct tipc_node *n_ptr, struct link *l_ptr)
 	n_ptr->links[l_ptr->b_ptr->identity] = NULL;
 	tipc_net.zones[tipc_zone(l_ptr->addr)]->links--;
 	n_ptr->link_cnt--;
+	link_count--;
 }
 
 /*
@@ -619,7 +619,7 @@ u32 tipc_available_nodes(const u32 domain)
 	u32 cnt = 0;
 
 	read_lock_bh(&tipc_net_lock);
-	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
+	list_for_each_entry(n_ptr, &nodes_list, node_list) {
 		if (!tipc_in_scope(domain, n_ptr->addr))
 			continue;
 		if (tipc_node_is_up(n_ptr))
@@ -646,15 +646,14 @@ struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
 						   " (network address)");
 
 	read_lock_bh(&tipc_net_lock);
-	if (!tipc_nodes) {
+	if (!node_count) {
 		read_unlock_bh(&tipc_net_lock);
 		return tipc_cfg_reply_none();
 	}
 
-	/* For now, get space for all other nodes
-	   (will need to modify this when slave nodes are supported */
+	/* Get space for all neighboring nodes */
 
-	payload_size = TLV_SPACE(sizeof(node_info)) * (tipc_max_nodes - 1);
+	payload_size = TLV_SPACE(sizeof(node_info)) * node_count;
 	if (payload_size > 32768u) {
 		read_unlock_bh(&tipc_net_lock);
 		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
@@ -668,7 +667,7 @@ struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
 
 	/* Add TLVs for all nodes in scope */
 
-	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
+	list_for_each_entry(n_ptr, &nodes_list, node_list) {
 		if (!tipc_in_scope(domain, n_ptr->addr))
 			continue;
 		node_info.addr = htonl(n_ptr->addr);
@@ -704,8 +703,7 @@ struct sk_buff *tipc_node_get_links(const void *req_tlv_area, int req_tlv_space)
 
 	/* Get space for all unicast links + multicast link */
 
-	payload_size = TLV_SPACE(sizeof(link_info)) *
-		(tipc_net.zones[tipc_zone(tipc_own_addr)]->links + 1);
+	payload_size = TLV_SPACE(sizeof(link_info)) * (link_count + 1);
 	if (payload_size > 32768u) {
 		read_unlock_bh(&tipc_net_lock);
 		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
@@ -726,7 +724,7 @@ struct sk_buff *tipc_node_get_links(const void *req_tlv_area, int req_tlv_space)
 
 	/* Add TLVs for any other links in scope */
 
-	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
+	list_for_each_entry(n_ptr, &nodes_list, node_list) {
 		u32 i;
 
 		if (!tipc_in_scope(domain, n_ptr->addr))
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 45f3db3..26715dc 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -47,7 +47,7 @@
  * @addr: network address of node
  * @lock: spinlock governing access to structure
  * @owner: pointer to cluster that node belongs to
- * @next: pointer to next node in sorted list of cluster's nodes
+ * @node_list: adjacent entries in sorted list of nodes
  * @nsub: list of "node down" subscriptions monitoring node
  * @active_links: pointers to active links to node
  * @links: pointers to all links to node
@@ -73,7 +73,7 @@ struct tipc_node {
 	u32 addr;
 	spinlock_t lock;
 	struct cluster *owner;
-	struct tipc_node *next;
+	struct list_head node_list;
 	struct list_head nsub;
 	struct link *active_links[2];
 	struct link *links[MAX_BEARERS];
@@ -96,7 +96,6 @@ struct tipc_node {
 	} bclink;
 };
 
-extern struct tipc_node *tipc_nodes;
 extern u32 tipc_own_tag;
 
 struct tipc_node *tipc_node_create(u32 addr);
-- 
1.7.0.4

Re: [PATCH net-next 4/5] tipc: Rework data structures that track neighboring nodes and links

[Neil Horman]

On Tue, Oct 12, 2010 at 08:25:57PM -0400, Paul Gortmaker wrote:
> From: Allan Stephens <allan.stephens@windriver.com>
> 
> Convert existing linked list of neighboring nodes to a standard
> doubly-linked list. Add counters that track total number of nodes
> in list and total number of links to these nodes, thereby allowing
> configuration message replies to allocate only space based on
> the actual number of nodes and links rather than the worst case.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  net/tipc/bcast.c |    5 ++-
>  net/tipc/link.c  |    3 +-
>  net/tipc/node.c  |   60 ++++++++++++++++++++++++++---------------------------
>  net/tipc/node.h  |    5 +--
>  4 files changed, 36 insertions(+), 37 deletions(-)
> 
NAK, this is completely broken.  You're casting tipc_node structure as
struct list_heads.  That basically makes the first 64 (or 128 bits on 64 bit
arches) act like a list_heads prev and next pointers.  So if you use node like a
list head, you're corrupting those first 64 or 128 bits.  If you modify
node->addr, node->lock, etc, you're corrupting your list pointers.  What you
want is an empty list head pointer defined somewhere to serve as the head of
your list, and to use the node_list member that you added to tipc_node to serve
as your list member in the use of list_add, list_del and list_for_each_entry
calls.

Neil


> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index ba6dcb2..e006678 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -454,10 +454,11 @@ void tipc_bclink_recv_pkt(struct sk_buff *buf)
>  			tipc_node_unlock(node);
>  			spin_lock_bh(&bc_lock);
>  			bcl->stats.recv_nacks++;
> -			bcl->owner->next = node;   /* remember requestor */
> +			/* remember retransmit requester */
> +			bcl->owner->node_list.next =
> +				(struct list_head *)node;
>  			bclink_retransmit_pkt(msg_bcgap_after(msg),
>  					      msg_bcgap_to(msg));
> -			bcl->owner->next = NULL;
>  			spin_unlock_bh(&bc_lock);
>  		} else {
>  			tipc_bclink_peek_nack(msg_destnode(msg),
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index 54bd99d..13bc0b6 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -1652,7 +1652,8 @@ static void link_retransmit_failure(struct link *l_ptr, struct sk_buff *buf)
>  		tipc_printf(TIPC_OUTPUT, "Outstanding acks: %lu\n",
>  				     (unsigned long) TIPC_SKB_CB(buf)->handle);
>  
> -		n_ptr = l_ptr->owner->next;
> +		/* recover retransmit requester */
> +		n_ptr = (struct tipc_node *)l_ptr->owner->node_list.next;
>  		tipc_node_lock(n_ptr);
>  
>  		tipc_addr_string_fill(addr_string, n_ptr->addr);
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 7c49cd0..944b480 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -50,7 +50,9 @@ void node_print(struct print_buf *buf, struct tipc_node *n_ptr, char *str);
>  static void node_lost_contact(struct tipc_node *n_ptr);
>  static void node_established_contact(struct tipc_node *n_ptr);
>  
> -struct tipc_node *tipc_nodes = NULL;	/* sorted list of nodes within cluster */
> +static LIST_HEAD(nodes_list);	/* sorted list of neighboring nodes */
> +static int node_count;	/* number of neighboring nodes that exist */
> +static int link_count;	/* number of unicast links node currently has */
>  
>  static DEFINE_SPINLOCK(node_create_lock);
>  
> @@ -70,11 +72,11 @@ struct tipc_node *tipc_node_create(u32 addr)
>  {
>  	struct cluster *c_ptr;
>  	struct tipc_node *n_ptr;
> -	struct tipc_node **curr_node;
> +	struct tipc_node *new_n_ptr;
>  
>  	spin_lock_bh(&node_create_lock);
>  
> -	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
> +	list_for_each_entry(n_ptr, &nodes_list, node_list) {
>  		if (addr < n_ptr->addr)
>  			break;
>  		if (addr == n_ptr->addr) {
> @@ -83,8 +85,8 @@ struct tipc_node *tipc_node_create(u32 addr)
>  		}
>  	}
>  
> -	n_ptr = kzalloc(sizeof(*n_ptr),GFP_ATOMIC);
> -	if (!n_ptr) {
> +	new_n_ptr = kzalloc(sizeof(*new_n_ptr), GFP_ATOMIC);
> +	if (!new_n_ptr) {
>  		spin_unlock_bh(&node_create_lock);
>  		warn("Node creation failed, no memory\n");
>  		return NULL;
> @@ -96,28 +98,22 @@ struct tipc_node *tipc_node_create(u32 addr)
>  	}
>  	if (!c_ptr) {
>  		spin_unlock_bh(&node_create_lock);
> -		kfree(n_ptr);
> +		kfree(new_n_ptr);
>  		return NULL;
>  	}
>  
> -	n_ptr->addr = addr;
> -		spin_lock_init(&n_ptr->lock);
> -	INIT_LIST_HEAD(&n_ptr->nsub);
> -	n_ptr->owner = c_ptr;
> -	tipc_cltr_attach_node(c_ptr, n_ptr);
> -	n_ptr->last_router = -1;
> +	new_n_ptr->addr = addr;
> +	spin_lock_init(&new_n_ptr->lock);
> +	INIT_LIST_HEAD(&new_n_ptr->nsub);
> +	new_n_ptr->owner = c_ptr;
> +	tipc_cltr_attach_node(c_ptr, new_n_ptr);
> +	new_n_ptr->last_router = -1;
> +
> +	list_add_tail(&new_n_ptr->node_list, &n_ptr->node_list);
> +	node_count++;
>  
> -	/* Insert node into ordered list */
> -	for (curr_node = &tipc_nodes; *curr_node;
> -	     curr_node = &(*curr_node)->next) {
> -		if (addr < (*curr_node)->addr) {
> -			n_ptr->next = *curr_node;
> -			break;
> -		}
> -	}
> -	(*curr_node) = n_ptr;
>  	spin_unlock_bh(&node_create_lock);
> -	return n_ptr;
> +	return new_n_ptr;
>  }
>  
>  void tipc_node_delete(struct tipc_node *n_ptr)
> @@ -136,6 +132,8 @@ void tipc_node_delete(struct tipc_node *n_ptr)
>  #endif
>  
>  	dbg("node %x deleted\n", n_ptr->addr);
> +	node_count--;
> +	list_del(&n_ptr->node_list);
>  	kfree(n_ptr);
>  }
>  
> @@ -275,6 +273,7 @@ struct tipc_node *tipc_node_attach_link(struct link *l_ptr)
>  			n_ptr->links[bearer_id] = l_ptr;
>  			tipc_net.zones[tipc_zone(l_ptr->addr)]->links++;
>  			n_ptr->link_cnt++;
> +			link_count++;
>  			return n_ptr;
>  		}
>  		err("Attempt to establish second link on <%s> to %s\n",
> @@ -289,6 +288,7 @@ void tipc_node_detach_link(struct tipc_node *n_ptr, struct link *l_ptr)
>  	n_ptr->links[l_ptr->b_ptr->identity] = NULL;
>  	tipc_net.zones[tipc_zone(l_ptr->addr)]->links--;
>  	n_ptr->link_cnt--;
> +	link_count--;
>  }
>  
>  /*
> @@ -619,7 +619,7 @@ u32 tipc_available_nodes(const u32 domain)
>  	u32 cnt = 0;
>  
>  	read_lock_bh(&tipc_net_lock);
> -	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
> +	list_for_each_entry(n_ptr, &nodes_list, node_list) {
>  		if (!tipc_in_scope(domain, n_ptr->addr))
>  			continue;
>  		if (tipc_node_is_up(n_ptr))
> @@ -646,15 +646,14 @@ struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
>  						   " (network address)");
>  
>  	read_lock_bh(&tipc_net_lock);
> -	if (!tipc_nodes) {
> +	if (!node_count) {
>  		read_unlock_bh(&tipc_net_lock);
>  		return tipc_cfg_reply_none();
>  	}
>  
> -	/* For now, get space for all other nodes
> -	   (will need to modify this when slave nodes are supported */
> +	/* Get space for all neighboring nodes */
>  
> -	payload_size = TLV_SPACE(sizeof(node_info)) * (tipc_max_nodes - 1);
> +	payload_size = TLV_SPACE(sizeof(node_info)) * node_count;
>  	if (payload_size > 32768u) {
>  		read_unlock_bh(&tipc_net_lock);
>  		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
> @@ -668,7 +667,7 @@ struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
>  
>  	/* Add TLVs for all nodes in scope */
>  
> -	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
> +	list_for_each_entry(n_ptr, &nodes_list, node_list) {
>  		if (!tipc_in_scope(domain, n_ptr->addr))
>  			continue;
>  		node_info.addr = htonl(n_ptr->addr);
> @@ -704,8 +703,7 @@ struct sk_buff *tipc_node_get_links(const void *req_tlv_area, int req_tlv_space)
>  
>  	/* Get space for all unicast links + multicast link */
>  
> -	payload_size = TLV_SPACE(sizeof(link_info)) *
> -		(tipc_net.zones[tipc_zone(tipc_own_addr)]->links + 1);
> +	payload_size = TLV_SPACE(sizeof(link_info)) * (link_count + 1);
>  	if (payload_size > 32768u) {
>  		read_unlock_bh(&tipc_net_lock);
>  		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
> @@ -726,7 +724,7 @@ struct sk_buff *tipc_node_get_links(const void *req_tlv_area, int req_tlv_space)
>  
>  	/* Add TLVs for any other links in scope */
>  
> -	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
> +	list_for_each_entry(n_ptr, &nodes_list, node_list) {
>  		u32 i;
>  
>  		if (!tipc_in_scope(domain, n_ptr->addr))
> diff --git a/net/tipc/node.h b/net/tipc/node.h
> index 45f3db3..26715dc 100644
> --- a/net/tipc/node.h
> +++ b/net/tipc/node.h
> @@ -47,7 +47,7 @@
>   * @addr: network address of node
>   * @lock: spinlock governing access to structure
>   * @owner: pointer to cluster that node belongs to
> - * @next: pointer to next node in sorted list of cluster's nodes
> + * @node_list: adjacent entries in sorted list of nodes
>   * @nsub: list of "node down" subscriptions monitoring node
>   * @active_links: pointers to active links to node
>   * @links: pointers to all links to node
> @@ -73,7 +73,7 @@ struct tipc_node {
>  	u32 addr;
>  	spinlock_t lock;
>  	struct cluster *owner;
> -	struct tipc_node *next;
> +	struct list_head node_list;
>  	struct list_head nsub;
>  	struct link *active_links[2];
>  	struct link *links[MAX_BEARERS];
> @@ -96,7 +96,6 @@ struct tipc_node {
>  	} bclink;
>  };
>  
> -extern struct tipc_node *tipc_nodes;
>  extern u32 tipc_own_tag;
>  
>  struct tipc_node *tipc_node_create(u32 addr);
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH net-next 5/5] tipc: clean out all instances of #if 0'd unused code

[Paul Gortmaker]

Remove all instances of legacy, or as yet to be implemented code
that is currently living within an #if 0 ... #endif block.
In the rare instance that some of it be needed in the future,
it can still be dragged out of history, but there is no need
for it to sit in mainline.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/config.c     |  141 -------------------------------------------------
 net/tipc/discover.c   |   20 -------
 net/tipc/discover.h   |    3 -
 net/tipc/link.c       |  112 +--------------------------------------
 net/tipc/link.h       |    4 --
 net/tipc/name_table.c |   17 ------
 net/tipc/net.c        |    9 ---
 net/tipc/node.c       |   26 ---------
 net/tipc/port.c       |   44 ---------------
 9 files changed, 1 insertions(+), 375 deletions(-)

diff --git a/net/tipc/config.c b/net/tipc/config.c
index a43450c..1e2fd41 100644
--- a/net/tipc/config.c
+++ b/net/tipc/config.c
@@ -120,139 +120,6 @@ struct sk_buff *tipc_cfg_reply_string_type(u16 tlv_type, char *string)
 	return buf;
 }
 
-
-#if 0
-
-/* Now obsolete code for handling commands not yet implemented the new way */
-
-/*
- * Some of this code assumed that the manager structure contains two added
- * fields:
- *	u32 link_subscriptions;
- *	struct list_head link_subscribers;
- * which are currently not present.  These fields may need to be re-introduced
- * if and when support for link subscriptions is added.
- */
-
-void tipc_cfg_link_event(u32 addr, char *name, int up)
-{
-	/* TIPC DOESN'T HANDLE LINK EVENT SUBSCRIPTIONS AT THE MOMENT */
-}
-
-int tipc_cfg_cmd(const struct tipc_cmd_msg * msg,
-		 char *data,
-		 u32 sz,
-		 u32 *ret_size,
-		 struct tipc_portid *orig)
-{
-	int rv = -EINVAL;
-	u32 cmd = msg->cmd;
-
-	*ret_size = 0;
-	switch (cmd) {
-	case TIPC_REMOVE_LINK:
-	case TIPC_CMD_BLOCK_LINK:
-	case TIPC_CMD_UNBLOCK_LINK:
-		if (!cfg_check_connection(orig))
-			rv = link_control(msg->argv.link_name, msg->cmd, 0);
-		break;
-	case TIPC_ESTABLISH:
-		{
-			int connected;
-
-			tipc_isconnected(mng.conn_port_ref, &connected);
-			if (connected || !orig) {
-				rv = TIPC_FAILURE;
-				break;
-			}
-			rv = tipc_connect2port(mng.conn_port_ref, orig);
-			if (rv == TIPC_OK)
-				orig = 0;
-			break;
-		}
-	case TIPC_GET_PEER_ADDRESS:
-		*ret_size = link_peer_addr(msg->argv.link_name, data, sz);
-		break;
-	case TIPC_GET_ROUTES:
-		rv = TIPC_OK;
-		break;
-	default: {}
-	}
-	if (*ret_size)
-		rv = TIPC_OK;
-	return rv;
-}
-
-static void cfg_cmd_event(struct tipc_cmd_msg *msg,
-			  char *data,
-			  u32 sz,
-			  struct tipc_portid const *orig)
-{
-	int rv = -EINVAL;
-	struct tipc_cmd_result_msg rmsg;
-	struct iovec msg_sect[2];
-	int *arg;
-
-	msg->cmd = ntohl(msg->cmd);
-
-	cfg_prepare_res_msg(msg->cmd, msg->usr_handle, rv, &rmsg, msg_sect,
-			    data, 0);
-	if (ntohl(msg->magic) != TIPC_MAGIC)
-		goto exit;
-
-	switch (msg->cmd) {
-	case TIPC_CREATE_LINK:
-		if (!cfg_check_connection(orig))
-			rv = disc_create_link(&msg->argv.create_link);
-		break;
-	case TIPC_LINK_SUBSCRIBE:
-		{
-			struct subscr_data *sub;
-
-			if (mng.link_subscriptions > 64)
-				break;
-			sub = kmalloc(sizeof(*sub),
-							    GFP_ATOMIC);
-			if (sub == NULL) {
-				warn("Memory squeeze; dropped remote link subscription\n");
-				break;
-			}
-			INIT_LIST_HEAD(&sub->subd_list);
-			tipc_createport(mng.user_ref,
-					(void *)sub,
-					TIPC_HIGH_IMPORTANCE,
-					0,
-					0,
-					(tipc_conn_shutdown_event)cfg_linksubscr_cancel,
-					0,
-					0,
-					(tipc_conn_msg_event)cfg_linksubscr_cancel,
-					0,
-					&sub->port_ref);
-			if (!sub->port_ref) {
-				kfree(sub);
-				break;
-			}
-			memcpy(sub->usr_handle,msg->usr_handle,
-			       sizeof(sub->usr_handle));
-			sub->domain = msg->argv.domain;
-			list_add_tail(&sub->subd_list, &mng.link_subscribers);
-			tipc_connect2port(sub->port_ref, orig);
-			rmsg.retval = TIPC_OK;
-			tipc_send(sub->port_ref, 2u, msg_sect);
-			mng.link_subscriptions++;
-			return;
-		}
-	default:
-		rv = tipc_cfg_cmd(msg, data, sz, (u32 *)&msg_sect[1].iov_len, orig);
-	}
-exit:
-	rmsg.result_len = htonl(msg_sect[1].iov_len);
-	rmsg.retval = htonl(rv);
-	tipc_cfg_respond(msg_sect, 2u, orig);
-}
-#endif
-
 #define MAX_STATS_INFO 2000
 
 static struct sk_buff *tipc_show_stats(void)
@@ -546,14 +413,6 @@ struct sk_buff *tipc_cfg_do_cmd(u32 orig_node, u16 cmd, const void *request_area
 	case TIPC_CMD_SHOW_PORTS:
 		rep_tlv_buf = tipc_port_get_ports();
 		break;
-#if 0
-	case TIPC_CMD_SHOW_PORT_STATS:
-		rep_tlv_buf = port_show_stats(req_tlv_area, req_tlv_space);
-		break;
-	case TIPC_CMD_RESET_PORT_STATS:
-		rep_tlv_buf = tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED);
-		break;
-#endif
 	case TIPC_CMD_SET_LOG_SIZE:
 		rep_tlv_buf = tipc_log_resize_cmd(req_tlv_area, req_tlv_space);
 		break;
diff --git a/net/tipc/discover.c b/net/tipc/discover.c
index f28d1ae..dbd79c6 100644
--- a/net/tipc/discover.c
+++ b/net/tipc/discover.c
@@ -46,16 +46,6 @@
 #define TIPC_LINK_REQ_FAST	2000	/* normal delay if bearer has no links */
 #define TIPC_LINK_REQ_SLOW	600000	/* normal delay if bearer has links */
 
-#if 0
-#define  GET_NODE_INFO         300
-#define  GET_NODE_INFO_RESULT  301
-#define  FORWARD_LINK_PROBE    302
-#define  LINK_REQUEST_REJECTED 303
-#define  LINK_REQUEST_ACCEPTED 304
-#define  DROP_LINK_REQUEST     305
-#define  CHECK_LINK_COUNT      306
-#endif
-
 /*
  * TODO: Most of the inter-cluster setup stuff should be
  * rewritten, and be made conformant with specification.
@@ -79,16 +69,6 @@ struct link_req {
 };
 
 
-#if 0
-int disc_create_link(const struct tipc_link_create *argv)
-{
-	/*
-	 * Code for inter cluster link setup here
-	 */
-	return TIPC_OK;
-}
-#endif
-
 /*
  * disc_lost_link(): A link has lost contact
  */
diff --git a/net/tipc/discover.h b/net/tipc/discover.h
index c36eaeb..9d064c3 100644
--- a/net/tipc/discover.h
+++ b/net/tipc/discover.h
@@ -51,8 +51,5 @@ void tipc_disc_stop_link_req(struct link_req *req);
 void tipc_disc_recv_msg(struct sk_buff *buf, struct bearer *b_ptr);
 
 void tipc_disc_link_event(u32 addr, char *name, int up);
-#if 0
-int  disc_create_link(const struct tipc_link_create *argv);
-#endif
 
 #endif
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 13bc0b6..4284351 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -99,23 +99,6 @@ struct link_name {
 	char if_peer[TIPC_MAX_IF_NAME];
 };
 
-#if 0
-
-/* LINK EVENT CODE IS NOT SUPPORTED AT PRESENT */
-
-/**
- * struct link_event - link up/down event notification
- */
-
-struct link_event {
-	u32 addr;
-	int up;
-	void (*fcn)(u32, char *, int);
-	char name[TIPC_MAX_LINK_NAME];
-};
-
-#endif
-
 static void link_handle_out_of_seq_msg(struct link *l_ptr,
 				       struct sk_buff *buf);
 static void link_recv_proto_msg(struct link *l_ptr, struct sk_buff *buf);
@@ -634,39 +617,9 @@ void tipc_link_stop(struct link *l_ptr)
 	l_ptr->proto_msg_queue = NULL;
 }
 
-#if 0
-
 /* LINK EVENT CODE IS NOT SUPPORTED AT PRESENT */
-
-static void link_recv_event(struct link_event *ev)
-{
-	ev->fcn(ev->addr, ev->name, ev->up);
-	kfree(ev);
-}
-
-static void link_send_event(void (*fcn)(u32 a, char *n, int up),
-			    struct link *l_ptr, int up)
-{
-	struct link_event *ev;
-
-	ev = kmalloc(sizeof(*ev), GFP_ATOMIC);
-	if (!ev) {
-		warn("Link event allocation failure\n");
-		return;
-	}
-	ev->addr = l_ptr->addr;
-	ev->up = up;
-	ev->fcn = fcn;
-	memcpy(ev->name, l_ptr->name, TIPC_MAX_LINK_NAME);
-	tipc_k_signal((Handler)link_recv_event, (unsigned long)ev);
-}
-
-#else
-
 #define link_send_event(fcn, l_ptr, up) do { } while (0)
 
-#endif
-
 void tipc_link_reset(struct link *l_ptr)
 {
 	struct sk_buff *buf;
@@ -690,10 +643,7 @@ void tipc_link_reset(struct link *l_ptr)
 
 	tipc_node_link_down(l_ptr->owner, l_ptr);
 	tipc_bearer_remove_dest(l_ptr->b_ptr, l_ptr->addr);
-#if 0
-	tipc_printf(TIPC_CONS, "\nReset link <%s>\n", l_ptr->name);
-	dbg_link_dump();
-#endif
+
 	if (was_active_link && tipc_node_has_active_links(l_ptr->owner) &&
 	    l_ptr->owner->permit_changeover) {
 		l_ptr->reset_checkpoint = checkpoint;
@@ -3198,44 +3148,6 @@ struct sk_buff *tipc_link_cmd_show_stats(const void *req_tlv_area, int req_tlv_s
 	return buf;
 }
 
-#if 0
-int link_control(const char *name, u32 op, u32 val)
-{
-	int res = -EINVAL;
-	struct link *l_ptr;
-	u32 bearer_id;
-	struct tipc_node * node;
-	u32 a;
-
-	a = link_name2addr(name, &bearer_id);
-	read_lock_bh(&tipc_net_lock);
-	node = tipc_node_find(a);
-	if (node) {
-		tipc_node_lock(node);
-		l_ptr = node->links[bearer_id];
-		if (l_ptr) {
-			if (op == TIPC_REMOVE_LINK) {
-				struct bearer *b_ptr = l_ptr->b_ptr;
-				spin_lock_bh(&b_ptr->publ.lock);
-				tipc_link_delete(l_ptr);
-				spin_unlock_bh(&b_ptr->publ.lock);
-			}
-			if (op == TIPC_CMD_BLOCK_LINK) {
-				tipc_link_reset(l_ptr);
-				l_ptr->blocked = 1;
-			}
-			if (op == TIPC_CMD_UNBLOCK_LINK) {
-				l_ptr->blocked = 0;
-			}
-			res = 0;
-		}
-		tipc_node_unlock(node);
-	}
-	read_unlock_bh(&tipc_net_lock);
-	return res;
-}
-#endif
-
 /**
  * tipc_link_get_max_pkt - get maximum packet size to use when sending to destination
  * @dest: network address of destination node
@@ -3266,28 +3178,6 @@ u32 tipc_link_get_max_pkt(u32 dest, u32 selector)
 	return res;
 }
 
-#if 0
-static void link_dump_rec_queue(struct link *l_ptr)
-{
-	struct sk_buff *crs;
-
-	if (!l_ptr->oldest_deferred_in) {
-		info("Reception queue empty\n");
-		return;
-	}
-	info("Contents of Reception queue:\n");
-	crs = l_ptr->oldest_deferred_in;
-	while (crs) {
-		if (crs->data == (void *)0x0000a3a3) {
-			info("buffer %x invalid\n", crs);
-			return;
-		}
-		msg_dbg(buf_msg(crs), "In rec queue:\n");
-		crs = crs->next;
-	}
-}
-#endif
-
 static void link_dump_send_queue(struct link *l_ptr)
 {
 	if (l_ptr->next_out) {
diff --git a/net/tipc/link.h b/net/tipc/link.h
index 26151d3..4e944ef 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -210,10 +210,6 @@ struct link {
 		u32 msg_length_counts;
 		u32 msg_lengths_total;
 		u32 msg_length_profile[7];
-#if 0
-		u32 sent_tunneled;
-		u32 recv_tunneled;
-#endif
 	} stats;
 
 	struct print_buf print_buf;
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 9ca4b06..3a8de43 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -1009,16 +1009,6 @@ static void nametbl_list(struct print_buf *buf, u32 depth_info,
 	}
 }
 
-#if 0
-void tipc_nametbl_print(struct print_buf *buf, const char *str)
-{
-	tipc_printf(buf, str);
-	read_lock_bh(&tipc_nametbl_lock);
-	nametbl_list(buf, 0, 0, 0, 0);
-	read_unlock_bh(&tipc_nametbl_lock);
-}
-#endif
-
 #define MAX_NAME_TBL_QUERY 32768
 
 struct sk_buff *tipc_nametbl_get(const void *req_tlv_area, int req_tlv_space)
@@ -1051,13 +1041,6 @@ struct sk_buff *tipc_nametbl_get(const void *req_tlv_area, int req_tlv_space)
 	return buf;
 }
 
-#if 0
-void tipc_nametbl_dump(void)
-{
-	nametbl_list(TIPC_CONS, 0, 0, 0, 0);
-}
-#endif
-
 int tipc_nametbl_init(void)
 {
 	table.types = kcalloc(tipc_nametbl_size, sizeof(struct hlist_head),
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 7e05af4..1a621cf 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -129,15 +129,6 @@ u32 tipc_net_select_router(u32 addr, u32 ref)
 	return tipc_zone_select_router(tipc_net.zones[tipc_zone(addr)], addr, ref);
 }
 
-#if 0
-u32 tipc_net_next_node(u32 a)
-{
-	if (tipc_net.zones[tipc_zone(a)])
-		return tipc_zone_next_node(a);
-	return 0;
-}
-#endif
-
 void tipc_net_remove_as_router(u32 router)
 {
 	u32 z_num;
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 944b480..fab80b4 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -121,16 +121,6 @@ void tipc_node_delete(struct tipc_node *n_ptr)
 	if (!n_ptr)
 		return;
 
-#if 0
-	/* Not needed because links are already deleted via tipc_bearer_stop() */
-
-	u32 l_num;
-
-	for (l_num = 0; l_num < MAX_BEARERS; l_num++) {
-		link_delete(n_ptr->links[l_num]);
-	}
-#endif
-
 	dbg("node %x deleted\n", n_ptr->addr);
 	node_count--;
 	list_del(&n_ptr->node_list);
@@ -597,22 +587,6 @@ void tipc_node_remove_router(struct tipc_node *n_ptr, u32 router)
 		node_lost_contact(n_ptr);
 }
 
-#if 0
-void node_print(struct print_buf *buf, struct tipc_node *n_ptr, char *str)
-{
-	u32 i;
-
-	tipc_printf(buf, "\n\n%s", str);
-	for (i = 0; i < MAX_BEARERS; i++) {
-		if (!n_ptr->links[i])
-			continue;
-		tipc_printf(buf, "Links[%u]: %x, ", i, n_ptr->links[i]);
-	}
-	tipc_printf(buf, "Active links: [%x,%x]\n",
-		    n_ptr->active_links[0], n_ptr->active_links[1]);
-}
-#endif
-
 u32 tipc_available_nodes(const u32 domain)
 {
 	struct tipc_node *n_ptr;
diff --git a/net/tipc/port.c b/net/tipc/port.c
index d760336..5c4285b 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -710,50 +710,6 @@ struct sk_buff *tipc_port_get_ports(void)
 	return buf;
 }
 
-#if 0
-
-#define MAX_PORT_STATS 2000
-
-struct sk_buff *port_show_stats(const void *req_tlv_area, int req_tlv_space)
-{
-	u32 ref;
-	struct port *p_ptr;
-	struct sk_buff *buf;
-	struct tlv_desc *rep_tlv;
-	struct print_buf pb;
-	int str_len;
-
-	if (!TLV_CHECK(req_tlv_area, req_tlv_space, TIPC_TLV_PORT_REF))
-		return cfg_reply_error_string(TIPC_CFG_TLV_ERROR);
-
-	ref = *(u32 *)TLV_DATA(req_tlv_area);
-	ref = ntohl(ref);
-
-	p_ptr = tipc_port_lock(ref);
-	if (!p_ptr)
-		return cfg_reply_error_string("port not found");
-
-	buf = tipc_cfg_reply_alloc(TLV_SPACE(MAX_PORT_STATS));
-	if (!buf) {
-		tipc_port_unlock(p_ptr);
-		return NULL;
-	}
-	rep_tlv = (struct tlv_desc *)buf->data;
-
-	tipc_printbuf_init(&pb, TLV_DATA(rep_tlv), MAX_PORT_STATS);
-	port_print(p_ptr, &pb, 1);
-	/* NEED TO FILL IN ADDITIONAL PORT STATISTICS HERE */
-	tipc_port_unlock(p_ptr);
-	str_len = tipc_printbuf_validate(&pb);
-
-	skb_put(buf, TLV_SPACE(str_len));
-	TLV_SET(rep_tlv, TIPC_TLV_ULTRA_STRING, NULL, str_len);
-
-	return buf;
-}
-
-#endif
-
 void tipc_port_reinit(void)
 {
 	struct port *p_ptr;
-- 
1.7.0.4

Re: [PATCH net-next 5/5] tipc: clean out all instances of #if 0'd unused code

[Neil Horman]

On Tue, Oct 12, 2010 at 08:25:58PM -0400, Paul Gortmaker wrote:
> Remove all instances of legacy, or as yet to be implemented code
> that is currently living within an #if 0 ... #endif block.
> In the rare instance that some of it be needed in the future,
> it can still be dragged out of history, but there is no need
> for it to sit in mainline.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH net-next 5/5] tipc: clean out all instances of #if 0'd unused code

[Paul Gortmaker]

On 10-10-13 12:26 PM, Neil Horman wrote:
> On Tue, Oct 12, 2010 at 08:25:58PM -0400, Paul Gortmaker wrote:
>> Remove all instances of legacy, or as yet to be implemented code
>> that is currently living within an #if 0 ... #endif block.
>> In the rare instance that some of it be needed in the future,
>> it can still be dragged out of history, but there is no need
>> for it to sit in mainline.
>>
>> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>
> Acked-by: Neil Horman<nhorman@tuxdriver.com>
> 

Thanks for all the reviews.  As I'd indicated a while back, I
am by far no TIPC expert, so it may take me a bit to digest
it and rework things on 1 --> 4. That will probably put them
into the "for-38" net-next timeframe, as I'm guessing net-next
that is destined for 2.6.37 will close in a day or two.

This cleanup patch (patch #5) doesn't explicitly depend on
the other 4 bearer related patches, so it can be applied
at whatever time is most convenient for Dave.

Paul.

Re: [PATCH net-next 5/5] tipc: clean out all instances of #if 0'd unused code

[Neil Horman]

On Wed, Oct 13, 2010 at 01:08:43PM -0400, Paul Gortmaker wrote:
> On 10-10-13 12:26 PM, Neil Horman wrote:
> > On Tue, Oct 12, 2010 at 08:25:58PM -0400, Paul Gortmaker wrote:
> >> Remove all instances of legacy, or as yet to be implemented code
> >> that is currently living within an #if 0 ... #endif block.
> >> In the rare instance that some of it be needed in the future,
> >> it can still be dragged out of history, but there is no need
> >> for it to sit in mainline.
> >>
> >> Signed-off-by: Paul Gortmaker<paul.gortmaker@windriver.com>
> > Acked-by: Neil Horman<nhorman@tuxdriver.com>
> > 
> 
> Thanks for all the reviews.  As I'd indicated a while back, I
> am by far no TIPC expert, so it may take me a bit to digest
> it and rework things on 1 --> 4. That will probably put them
> into the "for-38" net-next timeframe, as I'm guessing net-next
> that is destined for 2.6.37 will close in a day or two.
> 
No problem,  Most of what I identified I don't think is strictly a tipc related
issue, they're more just correct usage of various primitives (i.e. don't
initalize a work queue every time right before you schedule it), and use
list_add/list_del, instead of just munging data structures onto a list).  Most
looks pretty easy to correct I think.  Allan can probably help you out if tipc
specific knoweldge is needed.

Neil

> This cleanup patch (patch #5) doesn't explicitly depend on
> the other 4 bearer related patches, so it can be applied
> at whatever time is most convenient for Dave.
> 
> Paul.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 5/5] tipc: clean out all instances of #if 0'd unused code

[David Miller]

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Wed, 13 Oct 2010 13:08:43 -0400

> This cleanup patch (patch #5) doesn't explicitly depend on
> the other 4 bearer related patches, so it can be applied
> at whatever time is most convenient for Dave.

I've added patch #5 to net-next-2.6, thanks.

[PATCH net-next] tipc: cleanup function namespace

[Stephen Hemminger]

Do some cleanups of TIPC based on make namespacecheck
  1. Don't export unused symbols
  2. Eliminate dead code
  3. Make functions and variables local
  4. Rename buf_acquire to tipc_buf_acquire since it is used in several files

Compile tested only.
This make break out of tree kernel modules that depend on TIPC routines.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 include/net/tipc/tipc.h      |   71 -------------
 include/net/tipc/tipc_port.h |    2 
 net/tipc/addr.c              |    5 
 net/tipc/bcast.c             |   10 +
 net/tipc/bcast.h             |    3 
 net/tipc/cluster.c           |   21 ---
 net/tipc/cluster.h           |    2 
 net/tipc/config.c            |    7 +
 net/tipc/config.h            |    6 -
 net/tipc/core.c              |   28 +----
 net/tipc/core.h              |    9 -
 net/tipc/dbg.c               |   13 +-
 net/tipc/dbg.h               |    3 
 net/tipc/discover.c          |   16 --
 net/tipc/discover.h          |    2 
 net/tipc/link.c              |   45 ++++----
 net/tipc/link.h              |    4 
 net/tipc/msg.c               |    2 
 net/tipc/name_distr.c        |    2 
 net/tipc/node.c              |   19 ---
 net/tipc/node.h              |    1 
 net/tipc/port.c              |  234 ++++---------------------------------------
 net/tipc/port.h              |    2 
 net/tipc/ref.c               |   17 ---
 net/tipc/ref.h               |    1 
 net/tipc/subscr.c            |    9 -
 net/tipc/zone.c              |   11 --
 net/tipc/zone.h              |    1 
 28 files changed, 84 insertions(+), 462 deletions(-)

--- a/include/net/tipc/tipc.h	2010-10-13 15:43:50.025580173 -0700
+++ b/include/net/tipc/tipc.h	2010-10-13 16:14:33.731207018 -0700
@@ -50,8 +50,6 @@
  * TIPC operating mode routines
  */
 
-u32 tipc_get_addr(void);
-
 #define TIPC_NOT_RUNNING  0
 #define TIPC_NODE_MODE    1
 #define TIPC_NET_MODE     2
@@ -62,8 +60,6 @@ int tipc_attach(unsigned int *userref, t
 
 void tipc_detach(unsigned int userref);
 
-int tipc_get_mode(void);
-
 /*
  * TIPC port manipulation routines
  */
@@ -153,12 +149,6 @@ int tipc_disconnect(u32 portref);
 
 int tipc_shutdown(u32 ref);
 
-int tipc_isconnected(u32 portref, int *isconnected);
-
-int tipc_peer(u32 portref, struct tipc_portid *peer);
-
-int tipc_ref_valid(u32 portref); 
-
 /*
  * TIPC messaging routines
  */
@@ -170,38 +160,12 @@ int tipc_send(u32 portref,
 	      unsigned int num_sect,
 	      struct iovec const *msg_sect);
 
-int tipc_send_buf(u32 portref,
-		  struct sk_buff *buf,
-		  unsigned int dsz);
-
 int tipc_send2name(u32 portref, 
 		   struct tipc_name const *name, 
 		   u32 domain,
 		   unsigned int num_sect,
 		   struct iovec const *msg_sect);
 
-int tipc_send_buf2name(u32 portref,
-		       struct tipc_name const *name,
-		       u32 domain,
-		       struct sk_buff *buf,
-		       unsigned int dsz);
-
-int tipc_forward2name(u32 portref, 
-		      struct tipc_name const *name, 
-		      u32 domain,
-		      unsigned int section_count,
-		      struct iovec const *msg_sect,
-		      struct tipc_portid const *origin,
-		      unsigned int importance);
-
-int tipc_forward_buf2name(u32 portref,
-			  struct tipc_name const *name,
-			  u32 domain,
-			  struct sk_buff *buf,
-			  unsigned int dsz,
-			  struct tipc_portid const *orig,
-			  unsigned int importance);
-
 int tipc_send2port(u32 portref,
 		   struct tipc_portid const *dest,
 		   unsigned int num_sect,
@@ -212,46 +176,11 @@ int tipc_send_buf2port(u32 portref,
 		       struct sk_buff *buf,
 		       unsigned int dsz);
 
-int tipc_forward2port(u32 portref,
-		      struct tipc_portid const *dest,
-		      unsigned int num_sect,
-		      struct iovec const *msg_sect,
-		      struct tipc_portid const *origin,
-		      unsigned int importance);
-
-int tipc_forward_buf2port(u32 portref,
-			  struct tipc_portid const *dest,
-			  struct sk_buff *buf,
-			  unsigned int dsz,
-			  struct tipc_portid const *orig,
-			  unsigned int importance);
-
 int tipc_multicast(u32 portref, 
 		   struct tipc_name_seq const *seq, 
 		   u32 domain,	/* currently unused */
 		   unsigned int section_count,
 		   struct iovec const *msg);
-
-#if 0
-int tipc_multicast_buf(u32 portref, 
-		       struct tipc_name_seq const *seq, 
-		       u32 domain,
-		       void *buf,
-		       unsigned int size);
-#endif
-
-/*
- * TIPC subscription routines
- */
-
-int tipc_ispublished(struct tipc_name const *name);
-
-/*
- * Get number of available nodes within specified domain (excluding own node)
- */
-
-unsigned int tipc_available_nodes(const u32 domain);
-
 #endif
 
 #endif
--- a/net/tipc/bcast.c	2010-10-13 15:34:09.359325773 -0700
+++ b/net/tipc/bcast.c	2010-10-13 15:58:43.362005611 -0700
@@ -121,6 +121,9 @@ static DEFINE_SPINLOCK(bc_lock);
 
 const char tipc_bclink_name[] = "broadcast-link";
 
+static void tipc_nmap_diff(struct tipc_node_map *nm_a,
+			   struct tipc_node_map *nm_b,
+			   struct tipc_node_map *nm_diff);
 
 static u32 buf_seqno(struct sk_buff *buf)
 {
@@ -287,7 +290,7 @@ static void bclink_send_nack(struct tipc
 	if (!less(n_ptr->bclink.gap_after, n_ptr->bclink.gap_to))
 		return;
 
-	buf = buf_acquire(INT_H_SIZE);
+	buf = tipc_buf_acquire(INT_H_SIZE);
 	if (buf) {
 		msg = buf_msg(buf);
 		tipc_msg_init(msg, BCAST_PROTOCOL, STATE_MSG,
@@ -871,8 +874,9 @@ void tipc_nmap_remove(struct tipc_node_m
  * @nm_diff: output node map A-B (i.e. nodes of A that are not in B)
  */
 
-void tipc_nmap_diff(struct tipc_node_map *nm_a, struct tipc_node_map *nm_b,
-				  struct tipc_node_map *nm_diff)
+static void tipc_nmap_diff(struct tipc_node_map *nm_a,
+			   struct tipc_node_map *nm_b,
+			   struct tipc_node_map *nm_diff)
 {
 	int stop = ARRAY_SIZE(nm_a->map);
 	int w;
--- a/net/tipc/bcast.h	2010-10-13 15:34:39.167747046 -0700
+++ b/net/tipc/bcast.h	2010-10-13 15:42:10.321560582 -0700
@@ -84,9 +84,6 @@ static inline int tipc_nmap_equal(struct
 	return !memcmp(nm_a, nm_b, sizeof(*nm_a));
 }
 
-void tipc_nmap_diff(struct tipc_node_map *nm_a, struct tipc_node_map *nm_b,
-				  struct tipc_node_map *nm_diff);
-
 void tipc_port_list_add(struct port_list *pl_ptr, u32 port);
 void tipc_port_list_free(struct port_list *pl_ptr);
 
--- a/net/tipc/cluster.c	2010-10-13 15:44:43.583143953 -0700
+++ b/net/tipc/cluster.c	2010-10-13 16:03:22.148331782 -0700
@@ -113,25 +113,6 @@ void tipc_cltr_delete(struct cluster *c_
 	kfree(c_ptr);
 }
 
-u32 tipc_cltr_next_node(struct cluster *c_ptr, u32 addr)
-{
-	struct tipc_node *n_ptr;
-	u32 n_num = tipc_node(addr) + 1;
-
-	if (!c_ptr)
-		return addr;
-	for (; n_num <= c_ptr->highest_node; n_num++) {
-		n_ptr = c_ptr->nodes[n_num];
-		if (n_ptr && tipc_node_has_active_links(n_ptr))
-			return n_ptr->addr;
-	}
-	for (n_num = 1; n_num < tipc_node(addr); n_num++) {
-		n_ptr = c_ptr->nodes[n_num];
-		if (n_ptr && tipc_node_has_active_links(n_ptr))
-			return n_ptr->addr;
-	}
-	return 0;
-}
 
 void tipc_cltr_attach_node(struct cluster *c_ptr, struct tipc_node *n_ptr)
 {
@@ -232,7 +213,7 @@ struct tipc_node *tipc_cltr_select_node(
 static struct sk_buff *tipc_cltr_prepare_routing_msg(u32 data_size, u32 dest)
 {
 	u32 size = INT_H_SIZE + data_size;
-	struct sk_buff *buf = buf_acquire(size);
+	struct sk_buff *buf = tipc_buf_acquire(size);
 	struct tipc_msg *msg;
 
 	if (buf) {
--- a/net/tipc/config.c	2010-10-13 15:35:16.359281659 -0700
+++ b/net/tipc/config.c	2010-10-13 16:07:20.471544155 -0700
@@ -95,7 +95,7 @@ int tipc_cfg_append_tlv(struct sk_buff *
 	return 1;
 }
 
-struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value)
+static struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value)
 {
 	struct sk_buff *buf;
 	__be32 value_net;
@@ -109,6 +109,11 @@ struct sk_buff *tipc_cfg_reply_unsigned_
 	return buf;
 }
 
+static struct sk_buff *tipc_cfg_reply_unsigned(u32 value)
+{
+	return tipc_cfg_reply_unsigned_type(TIPC_TLV_UNSIGNED, value);
+}
+
 struct sk_buff *tipc_cfg_reply_string_type(u16 tlv_type, char *string)
 {
 	struct sk_buff *buf;
--- a/net/tipc/config.h	2010-10-13 15:43:36.031261839 -0700
+++ b/net/tipc/config.h	2010-10-13 15:49:35.868009966 -0700
@@ -45,7 +45,6 @@
 struct sk_buff *tipc_cfg_reply_alloc(int payload_size);
 int tipc_cfg_append_tlv(struct sk_buff *buf, int tlv_type,
 			void *tlv_data, int tlv_data_size);
-struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value);
 struct sk_buff *tipc_cfg_reply_string_type(u16 tlv_type, char *string);
 
 static inline struct sk_buff *tipc_cfg_reply_none(void)
@@ -53,11 +52,6 @@ static inline struct sk_buff *tipc_cfg_r
 	return tipc_cfg_reply_alloc(0);
 }
 
-static inline struct sk_buff *tipc_cfg_reply_unsigned(u32 value)
-{
-	return tipc_cfg_reply_unsigned_type(TIPC_TLV_UNSIGNED, value);
-}
-
 static inline struct sk_buff *tipc_cfg_reply_error_string(char *string)
 {
 	return tipc_cfg_reply_string_type(TIPC_TLV_ERROR_STRING, string);
--- a/net/tipc/core.c	2010-10-13 15:32:35.750562985 -0700
+++ b/net/tipc/core.c	2010-10-13 16:00:58.197719377 -0700
@@ -96,13 +96,13 @@ int tipc_net_id;
 int tipc_remote_management;
 
 
-int tipc_get_mode(void)
+static int tipc_get_mode(void)
 {
 	return tipc_mode;
 }
 
 /**
- * buf_acquire - creates a TIPC message buffer
+ * tipc_buf_acquire - creates a TIPC message buffer
  * @size: message size (including TIPC header)
  *
  * Returns a new buffer with data pointers set to the specified size.
@@ -111,7 +111,7 @@ int tipc_get_mode(void)
  *       There may also be unrequested tailroom present at the buffer's end.
  */
 
-struct sk_buff *buf_acquire(u32 size)
+struct sk_buff *tipc_buf_acquire(u32 size)
 {
 	struct sk_buff *skb;
 	unsigned int buf_size = (BUF_HEADROOM + size + 3) & ~3u;
@@ -129,7 +129,7 @@ struct sk_buff *buf_acquire(u32 size)
  * tipc_core_stop_net - shut down TIPC networking sub-systems
  */
 
-void tipc_core_stop_net(void)
+static void tipc_core_stop_net(void)
 {
 	tipc_eth_media_stop();
 	tipc_net_stop();
@@ -154,7 +154,7 @@ int tipc_core_start_net(unsigned long ad
  * tipc_core_stop - switch TIPC from SINGLE NODE to NOT RUNNING mode
  */
 
-void tipc_core_stop(void)
+static void tipc_core_stop(void)
 {
 	if (tipc_mode != TIPC_NODE_MODE)
 		return;
@@ -176,7 +176,7 @@ void tipc_core_stop(void)
  * tipc_core_start - switch TIPC from NOT RUNNING to SINGLE NODE mode
  */
 
-int tipc_core_start(void)
+static int tipc_core_start(void)
 {
 	int res;
 
@@ -246,7 +246,6 @@ MODULE_VERSION(TIPC_MOD_VER);
 
 EXPORT_SYMBOL(tipc_attach);
 EXPORT_SYMBOL(tipc_detach);
-EXPORT_SYMBOL(tipc_get_addr);
 EXPORT_SYMBOL(tipc_get_mode);
 EXPORT_SYMBOL(tipc_createport);
 EXPORT_SYMBOL(tipc_deleteport);
@@ -262,23 +261,10 @@ EXPORT_SYMBOL(tipc_withdraw);
 EXPORT_SYMBOL(tipc_connect2port);
 EXPORT_SYMBOL(tipc_disconnect);
 EXPORT_SYMBOL(tipc_shutdown);
-EXPORT_SYMBOL(tipc_isconnected);
-EXPORT_SYMBOL(tipc_peer);
-EXPORT_SYMBOL(tipc_ref_valid);
 EXPORT_SYMBOL(tipc_send);
-EXPORT_SYMBOL(tipc_send_buf);
 EXPORT_SYMBOL(tipc_send2name);
-EXPORT_SYMBOL(tipc_forward2name);
-EXPORT_SYMBOL(tipc_send_buf2name);
-EXPORT_SYMBOL(tipc_forward_buf2name);
 EXPORT_SYMBOL(tipc_send2port);
-EXPORT_SYMBOL(tipc_forward2port);
-EXPORT_SYMBOL(tipc_send_buf2port);
-EXPORT_SYMBOL(tipc_forward_buf2port);
 EXPORT_SYMBOL(tipc_multicast);
-/* EXPORT_SYMBOL(tipc_multicast_buf); not available yet */
-EXPORT_SYMBOL(tipc_ispublished);
-EXPORT_SYMBOL(tipc_available_nodes);
 
 /* TIPC API for external bearers (see tipc_bearer.h) */
 
@@ -295,6 +281,4 @@ EXPORT_SYMBOL(tipc_createport_raw);
 EXPORT_SYMBOL(tipc_reject_msg);
 EXPORT_SYMBOL(tipc_send_buf_fast);
 EXPORT_SYMBOL(tipc_acknowledge);
-EXPORT_SYMBOL(tipc_get_port);
-EXPORT_SYMBOL(tipc_get_handle);
 
--- a/net/tipc/core.h	2010-10-13 15:36:36.237690119 -0700
+++ b/net/tipc/core.h	2010-10-13 16:07:56.727164658 -0700
@@ -83,9 +83,7 @@
  * Note: TIPC_LOG is configured to echo its output to the system console;
  *       user-defined buffers can be configured to do the same thing.
  */
-
 extern struct print_buf *const TIPC_NULL;
-extern struct print_buf *const TIPC_CONS;
 extern struct print_buf *const TIPC_LOG;
 
 void tipc_printf(struct print_buf *, const char *fmt, ...);
@@ -204,10 +202,7 @@ extern atomic_t tipc_user_count;
  * Routines available to privileged subsystems
  */
 
-extern int  tipc_core_start(void);
-extern void tipc_core_stop(void);
-extern int  tipc_core_start_net(unsigned long addr);
-extern void tipc_core_stop_net(void);
+extern int tipc_core_start_net(unsigned long);
 extern int  tipc_handler_start(void);
 extern void tipc_handler_stop(void);
 extern int  tipc_netlink_start(void);
@@ -328,7 +323,7 @@ static inline struct tipc_msg *buf_msg(s
 	return (struct tipc_msg *)skb->data;
 }
 
-extern struct sk_buff *buf_acquire(u32 size);
+extern struct sk_buff *tipc_buf_acquire(u32 size);
 
 /**
  * buf_discard - frees a TIPC message buffer
--- a/net/tipc/dbg.c	2010-10-13 15:35:34.049157629 -0700
+++ b/net/tipc/dbg.c	2010-10-13 15:58:43.362005611 -0700
@@ -52,7 +52,7 @@ static struct print_buf null_buf = { NUL
 struct print_buf *const TIPC_NULL = &null_buf;
 
 static struct print_buf cons_buf = { NULL, 0, NULL, 1 };
-struct print_buf *const TIPC_CONS = &cons_buf;
+static struct print_buf *const TIPC_CONS = &cons_buf;
 
 static struct print_buf log_buf = { NULL, 0, NULL, 1 };
 struct print_buf *const TIPC_LOG = &log_buf;
@@ -76,6 +76,10 @@ struct print_buf *const TIPC_LOG = &log_
 static char print_string[TIPC_PB_MAX_STR];
 static DEFINE_SPINLOCK(print_lock);
 
+static void tipc_printbuf_reset(struct print_buf *pb);
+static int  tipc_printbuf_empty(struct print_buf *pb);
+static void tipc_printbuf_move(struct print_buf *pb_to,
+			       struct print_buf *pb_from);
 
 #define FORMAT(PTR,LEN,FMT) \
 {\
@@ -116,7 +120,7 @@ void tipc_printbuf_init(struct print_buf
  * @pb: pointer to print buffer structure
  */
 
-void tipc_printbuf_reset(struct print_buf *pb)
+static void tipc_printbuf_reset(struct print_buf *pb)
 {
 	if (pb->buf) {
 		pb->crs = pb->buf;
@@ -132,7 +136,7 @@ void tipc_printbuf_reset(struct print_bu
  * Returns non-zero if print buffer is empty.
  */
 
-int tipc_printbuf_empty(struct print_buf *pb)
+static int tipc_printbuf_empty(struct print_buf *pb)
 {
 	return !pb->buf || (pb->crs == pb->buf);
 }
@@ -181,7 +185,8 @@ int tipc_printbuf_validate(struct print_
  * Source print buffer becomes empty if a successful move occurs.
  */
 
-void tipc_printbuf_move(struct print_buf *pb_to, struct print_buf *pb_from)
+static void tipc_printbuf_move(struct print_buf *pb_to,
+			       struct print_buf *pb_from)
 {
 	int len;
 
--- a/net/tipc/dbg.h	2010-10-13 15:37:37.134376966 -0700
+++ b/net/tipc/dbg.h	2010-10-13 15:42:09.489660536 -0700
@@ -56,10 +56,7 @@ struct print_buf {
 #define TIPC_PB_MAX_STR 512	/* max printable string (with trailing NUL) */
 
 void tipc_printbuf_init(struct print_buf *pb, char *buf, u32 size);
-void tipc_printbuf_reset(struct print_buf *pb);
-int  tipc_printbuf_empty(struct print_buf *pb);
 int  tipc_printbuf_validate(struct print_buf *pb);
-void tipc_printbuf_move(struct print_buf *pb_to, struct print_buf *pb_from);
 
 int tipc_log_resize(int log_size);
 
--- a/net/tipc/discover.c	2010-10-13 15:38:16.357666201 -0700
+++ b/net/tipc/discover.c	2010-10-13 15:52:14.740906596 -0700
@@ -68,20 +68,6 @@ struct link_req {
 	unsigned int timer_intv;
 };
 
-
-/*
- * disc_lost_link(): A link has lost contact
- */
-
-void tipc_disc_link_event(u32 addr, char *name, int up)
-{
-	if (in_own_cluster(addr))
-		return;
-	/*
-	 * Code for inter cluster link setup here
-	 */
-}
-
 /**
  * tipc_disc_init_msg - initialize a link setup message
  * @type: message type (request or response)
@@ -95,7 +81,7 @@ static struct sk_buff *tipc_disc_init_ms
 					  u32 dest_domain,
 					  struct bearer *b_ptr)
 {
-	struct sk_buff *buf = buf_acquire(DSC_H_SIZE);
+	struct sk_buff *buf = tipc_buf_acquire(DSC_H_SIZE);
 	struct tipc_msg *msg;
 
 	if (buf) {
--- a/net/tipc/discover.h	2010-10-13 15:47:41.609746110 -0700
+++ b/net/tipc/discover.h	2010-10-13 15:47:50.604664821 -0700
@@ -50,6 +50,4 @@ void tipc_disc_stop_link_req(struct link
 
 void tipc_disc_recv_msg(struct sk_buff *buf, struct bearer *b_ptr);
 
-void tipc_disc_link_event(u32 addr, char *name, int up);
-
 #endif
--- a/net/tipc/link.c	2010-10-13 15:38:30.243998362 -0700
+++ b/net/tipc/link.c	2010-10-13 15:53:47.961684397 -0700
@@ -112,6 +112,9 @@ static void link_state_event(struct link
 static void link_reset_statistics(struct link *l_ptr);
 static void link_print(struct link *l_ptr, struct print_buf *buf,
 		       const char *str);
+static void link_start(struct link *l_ptr);
+static int link_send_long_buf(struct link *l_ptr, struct sk_buff *buf);
+
 
 /*
  * Debugging code used by link routines only
@@ -442,7 +445,7 @@ struct link *tipc_link_create(struct bea
 
 	k_init_timer(&l_ptr->timer, (Handler)link_timeout, (unsigned long)l_ptr);
 	list_add_tail(&l_ptr->link_list, &b_ptr->links);
-	tipc_k_signal((Handler)tipc_link_start, (unsigned long)l_ptr);
+	tipc_k_signal((Handler)link_start, (unsigned long)l_ptr);
 
 	dbg("tipc_link_create(): tolerance = %u,cont intv = %u, abort_limit = %u\n",
 	    l_ptr->tolerance, l_ptr->continuity_interval, l_ptr->abort_limit);
@@ -482,9 +485,9 @@ void tipc_link_delete(struct link *l_ptr
 	kfree(l_ptr);
 }
 
-void tipc_link_start(struct link *l_ptr)
+static void link_start(struct link *l_ptr)
 {
-	dbg("tipc_link_start %x\n", l_ptr);
+	dbg("link_start %x\n", l_ptr);
 	link_state_event(l_ptr, STARTING_EVT);
 }
 
@@ -1000,7 +1003,7 @@ int tipc_link_send_buf(struct link *l_pt
 	/* Fragmentation needed ? */
 
 	if (size > max_packet)
-		return tipc_link_send_long_buf(l_ptr, buf);
+		return link_send_long_buf(l_ptr, buf);
 
 	/* Packet can be queued or sent: */
 
@@ -1036,7 +1039,7 @@ int tipc_link_send_buf(struct link *l_pt
 		/* Try creating a new bundle */
 
 		if (size <= max_packet * 2 / 3) {
-			struct sk_buff *bundler = buf_acquire(max_packet);
+			struct sk_buff *bundler = tipc_buf_acquire(max_packet);
 			struct tipc_msg bundler_hdr;
 
 			if (bundler) {
@@ -1312,7 +1315,7 @@ again:
 
 	/* Prepare header of first fragment: */
 
-	buf_chain = buf = buf_acquire(max_pkt);
+	buf_chain = buf = tipc_buf_acquire(max_pkt);
 	if (!buf)
 		return -ENOMEM;
 	buf->next = NULL;
@@ -1369,7 +1372,7 @@ error:
 			msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE);
 			msg_set_fragm_no(&fragm_hdr, ++fragm_no);
 			prev = buf;
-			buf = buf_acquire(fragm_sz + INT_H_SIZE);
+			buf = tipc_buf_acquire(fragm_sz + INT_H_SIZE);
 			if (!buf)
 				goto error;
 
@@ -2145,7 +2148,7 @@ void tipc_link_send_proto_msg(struct lin
 	if (tipc_bearer_congested(l_ptr->b_ptr, l_ptr)) {
 		if (!l_ptr->proto_msg_queue) {
 			l_ptr->proto_msg_queue =
-				buf_acquire(sizeof(l_ptr->proto_msg));
+				tipc_buf_acquire(sizeof(l_ptr->proto_msg));
 		}
 		buf = l_ptr->proto_msg_queue;
 		if (!buf)
@@ -2159,7 +2162,7 @@ void tipc_link_send_proto_msg(struct lin
 
 	msg_dbg(msg, ">>");
 
-	buf = buf_acquire(msg_size);
+	buf = tipc_buf_acquire(msg_size);
 	if (!buf)
 		return;
 
@@ -2318,10 +2321,10 @@ exit:
  * tipc_link_tunnel(): Send one message via a link belonging to
  * another bearer. Owner node is locked.
  */
-void tipc_link_tunnel(struct link *l_ptr,
-		      struct tipc_msg *tunnel_hdr,
-		      struct tipc_msg  *msg,
-		      u32 selector)
+static void tipc_link_tunnel(struct link *l_ptr,
+			     struct tipc_msg *tunnel_hdr,
+			     struct tipc_msg  *msg,
+			     u32 selector)
 {
 	struct link *tunnel;
 	struct sk_buff *buf;
@@ -2334,7 +2337,7 @@ void tipc_link_tunnel(struct link *l_ptr
 		return;
 	}
 	msg_set_size(tunnel_hdr, length + INT_H_SIZE);
-	buf = buf_acquire(length + INT_H_SIZE);
+	buf = tipc_buf_acquire(length + INT_H_SIZE);
 	if (!buf) {
 		warn("Link changeover error, "
 		     "unable to send tunnel msg\n");
@@ -2380,7 +2383,7 @@ void tipc_link_changeover(struct link *l
 	if (!l_ptr->first_out) {
 		struct sk_buff *buf;
 
-		buf = buf_acquire(INT_H_SIZE);
+		buf = tipc_buf_acquire(INT_H_SIZE);
 		if (buf) {
 			skb_copy_to_linear_data(buf, &tunnel_hdr, INT_H_SIZE);
 			msg_set_size(&tunnel_hdr, INT_H_SIZE);
@@ -2441,7 +2444,7 @@ void tipc_link_send_duplicate(struct lin
 		msg_set_ack(msg, mod(l_ptr->next_in_no - 1));	/* Update */
 		msg_set_bcast_ack(msg, l_ptr->owner->bclink.last_in);
 		msg_set_size(&tunnel_hdr, length + INT_H_SIZE);
-		outbuf = buf_acquire(length + INT_H_SIZE);
+		outbuf = tipc_buf_acquire(length + INT_H_SIZE);
 		if (outbuf == NULL) {
 			warn("Link changeover error, "
 			     "unable to send duplicate msg\n");
@@ -2477,7 +2480,7 @@ static struct sk_buff *buf_extract(struc
 	u32 size = msg_size(msg);
 	struct sk_buff *eb;
 
-	eb = buf_acquire(size);
+	eb = tipc_buf_acquire(size);
 	if (eb)
 		skb_copy_to_linear_data(eb, msg, size);
 	return eb;
@@ -2605,11 +2608,11 @@ void tipc_link_recv_bundle(struct sk_buf
 
 
 /*
- * tipc_link_send_long_buf: Entry for buffers needing fragmentation.
+ * link_send_long_buf: Entry for buffers needing fragmentation.
  * The buffer is complete, inclusive total message length.
  * Returns user data length.
  */
-int tipc_link_send_long_buf(struct link *l_ptr, struct sk_buff *buf)
+static int link_send_long_buf(struct link *l_ptr, struct sk_buff *buf)
 {
 	struct tipc_msg *inmsg = buf_msg(buf);
 	struct tipc_msg fragm_hdr;
@@ -2648,7 +2651,7 @@ int tipc_link_send_long_buf(struct link 
 			fragm_sz = rest;
 			msg_set_type(&fragm_hdr, LAST_FRAGMENT);
 		}
-		fragm = buf_acquire(fragm_sz + INT_H_SIZE);
+		fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE);
 		if (fragm == NULL) {
 			warn("Link unable to fragment message\n");
 			dsz = -ENOMEM;
@@ -2753,7 +2756,7 @@ int tipc_link_recv_fragment(struct sk_bu
 			buf_discard(fbuf);
 			return 0;
 		}
-		pbuf = buf_acquire(msg_size(imsg));
+		pbuf = tipc_buf_acquire(msg_size(imsg));
 		if (pbuf != NULL) {
 			pbuf->next = *pending;
 			*pending = pbuf;
--- a/net/tipc/link.h	2010-10-13 15:51:49.403953468 -0700
+++ b/net/tipc/link.h	2010-10-13 15:52:06.877852172 -0700
@@ -225,7 +225,6 @@ void tipc_link_send_duplicate(struct lin
 void tipc_link_reset_fragments(struct link *l_ptr);
 int tipc_link_is_up(struct link *l_ptr);
 int tipc_link_is_active(struct link *l_ptr);
-void tipc_link_start(struct link *l_ptr);
 u32 tipc_link_push_packet(struct link *l_ptr);
 void tipc_link_stop(struct link *l_ptr);
 struct sk_buff *tipc_link_cmd_config(const void *req_tlv_area, int req_tlv_space, u16 cmd);
@@ -239,9 +238,6 @@ int tipc_link_send_sections_fast(struct 
 				 struct iovec const *msg_sect,
 				 const u32 num_sect,
 				 u32 destnode);
-int tipc_link_send_long_buf(struct link *l_ptr, struct sk_buff *buf);
-void tipc_link_tunnel(struct link *l_ptr, struct tipc_msg *tnl_hdr,
-		      struct tipc_msg *msg, u32 selector);
 void tipc_link_recv_bundle(struct sk_buff *buf);
 int  tipc_link_recv_fragment(struct sk_buff **pending,
 			     struct sk_buff **fb,
--- a/net/tipc/msg.c	2010-10-13 15:45:42.760031835 -0700
+++ b/net/tipc/msg.c	2010-10-13 15:45:52.410871897 -0700
@@ -112,7 +112,7 @@ int tipc_msg_build(struct tipc_msg *hdr,
 		return dsz;
 	}
 
-	*buf = buf_acquire(sz);
+	*buf = tipc_buf_acquire(sz);
 	if (!(*buf))
 		return -ENOMEM;
 	skb_copy_to_linear_data(*buf, hdr, hsz);
--- a/net/tipc/name_distr.c	2010-10-13 15:45:56.098428684 -0700
+++ b/net/tipc/name_distr.c	2010-10-13 15:46:05.505298048 -0700
@@ -98,7 +98,7 @@ static void publ_to_item(struct distr_it
 
 static struct sk_buff *named_prepare_buf(u32 type, u32 size, u32 dest)
 {
-	struct sk_buff *buf = buf_acquire(LONG_H_SIZE + size);
+	struct sk_buff *buf = tipc_buf_acquire(LONG_H_SIZE + size);
 	struct tipc_msg *msg;
 
 	if (buf != NULL) {
--- a/net/tipc/node.c	2010-10-13 15:39:25.165401568 -0700
+++ b/net/tipc/node.c	2010-10-13 16:03:21.564402314 -0700
@@ -50,7 +50,8 @@ void node_print(struct print_buf *buf, s
 static void node_lost_contact(struct tipc_node *n_ptr);
 static void node_established_contact(struct tipc_node *n_ptr);
 
-struct tipc_node *tipc_nodes = NULL;	/* sorted list of nodes within cluster */
+/* sorted list of nodes within cluster */
+static struct tipc_node *tipc_nodes = NULL;
 
 static DEFINE_SPINLOCK(node_create_lock);
 
@@ -587,22 +588,6 @@ void tipc_node_remove_router(struct tipc
 		node_lost_contact(n_ptr);
 }
 
-u32 tipc_available_nodes(const u32 domain)
-{
-	struct tipc_node *n_ptr;
-	u32 cnt = 0;
-
-	read_lock_bh(&tipc_net_lock);
-	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
-		if (!tipc_in_scope(domain, n_ptr->addr))
-			continue;
-		if (tipc_node_is_up(n_ptr))
-			cnt++;
-	}
-	read_unlock_bh(&tipc_net_lock);
-	return cnt;
-}
-
 struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
 {
 	u32 domain;
--- a/net/tipc/node.h	2010-10-13 15:54:01.616035253 -0700
+++ b/net/tipc/node.h	2010-10-13 15:54:11.830801530 -0700
@@ -96,7 +96,6 @@ struct tipc_node {
 	} bclink;
 };
 
-extern struct tipc_node *tipc_nodes;
 extern u32 tipc_own_tag;
 
 struct tipc_node *tipc_node_create(u32 addr);
--- a/net/tipc/port.c	2010-10-13 15:40:52.362926772 -0700
+++ b/net/tipc/port.c	2010-10-13 16:16:41.435779874 -0700
@@ -293,34 +293,6 @@ int tipc_deleteport(u32 ref)
 	return 0;
 }
 
-/**
- * tipc_get_port() - return port associated with 'ref'
- *
- * Note: Port is not locked.
- */
-
-struct tipc_port *tipc_get_port(const u32 ref)
-{
-	return (struct tipc_port *)tipc_ref_deref(ref);
-}
-
-/**
- * tipc_get_handle - return user handle associated to port 'ref'
- */
-
-void *tipc_get_handle(const u32 ref)
-{
-	struct port *p_ptr;
-	void * handle;
-
-	p_ptr = tipc_port_lock(ref);
-	if (!p_ptr)
-		return NULL;
-	handle = p_ptr->publ.usr_handle;
-	tipc_port_unlock(p_ptr);
-	return handle;
-}
-
 static int port_unreliable(struct port *p_ptr)
 {
 	return msg_src_droppable(&p_ptr->publ.phdr);
@@ -392,7 +364,7 @@ static struct sk_buff *port_build_proto_
 	struct sk_buff *buf;
 	struct tipc_msg *msg;
 
-	buf = buf_acquire(LONG_H_SIZE);
+	buf = tipc_buf_acquire(LONG_H_SIZE);
 	if (buf) {
 		msg = buf_msg(buf);
 		tipc_msg_init(msg, usr, type, LONG_H_SIZE, destnode);
@@ -433,7 +405,7 @@ int tipc_reject_msg(struct sk_buff *buf,
 		hdr_sz = MCAST_H_SIZE;
 	else
 		hdr_sz = LONG_H_SIZE;
-	rbuf = buf_acquire(data_sz + hdr_sz);
+	rbuf = tipc_buf_acquire(data_sz + hdr_sz);
 	if (rbuf == NULL) {
 		buf_discard(buf);
 		return data_sz;
@@ -1242,50 +1214,13 @@ int tipc_shutdown(u32 ref)
 	return tipc_disconnect(ref);
 }
 
-int tipc_isconnected(u32 ref, int *isconnected)
-{
-	struct port *p_ptr;
-
-	p_ptr = tipc_port_lock(ref);
-	if (!p_ptr)
-		return -EINVAL;
-	*isconnected = p_ptr->publ.connected;
-	tipc_port_unlock(p_ptr);
-	return 0;
-}
-
-int tipc_peer(u32 ref, struct tipc_portid *peer)
-{
-	struct port *p_ptr;
-	int res;
-
-	p_ptr = tipc_port_lock(ref);
-	if (!p_ptr)
-		return -EINVAL;
-	if (p_ptr->publ.connected) {
-		peer->ref = port_peerport(p_ptr);
-		peer->node = port_peernode(p_ptr);
-		res = 0;
-	} else
-		res = -ENOTCONN;
-	tipc_port_unlock(p_ptr);
-	return res;
-}
-
-int tipc_ref_valid(u32 ref)
-{
-	/* Works irrespective of type */
-	return !!tipc_ref_deref(ref);
-}
-
-
 /*
  *  tipc_port_recv_sections(): Concatenate and deliver sectioned
  *                        message for this node.
  */
 
-int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
-		       struct iovec const *msg_sect)
+static int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
+				   struct iovec const *msg_sect)
 {
 	struct sk_buff *buf;
 	int res;
@@ -1336,65 +1271,16 @@ int tipc_send(u32 ref, unsigned int num_
 }
 
 /**
- * tipc_send_buf - send message buffer on connection
- */
-
-int tipc_send_buf(u32 ref, struct sk_buff *buf, unsigned int dsz)
-{
-	struct port *p_ptr;
-	struct tipc_msg *msg;
-	u32 destnode;
-	u32 hsz;
-	u32 sz;
-	u32 res;
-
-	p_ptr = tipc_port_deref(ref);
-	if (!p_ptr || !p_ptr->publ.connected)
-		return -EINVAL;
-
-	msg = &p_ptr->publ.phdr;
-	hsz = msg_hdr_sz(msg);
-	sz = hsz + dsz;
-	msg_set_size(msg, sz);
-	if (skb_cow(buf, hsz))
-		return -ENOMEM;
-
-	skb_push(buf, hsz);
-	skb_copy_to_linear_data(buf, msg, hsz);
-	destnode = msg_destnode(msg);
-	p_ptr->publ.congested = 1;
-	if (!tipc_port_congested(p_ptr)) {
-		if (likely(destnode != tipc_own_addr))
-			res = tipc_send_buf_fast(buf, destnode);
-		else {
-			tipc_port_recv_msg(buf);
-			res = sz;
-		}
-		if (likely(res != -ELINKCONG)) {
-			port_incr_out_seqno(p_ptr);
-			p_ptr->sent++;
-			p_ptr->publ.congested = 0;
-			return res;
-		}
-	}
-	if (port_unreliable(p_ptr)) {
-		p_ptr->publ.congested = 0;
-		return dsz;
-	}
-	return -ELINKCONG;
-}
-
-/**
  * tipc_forward2name - forward message sections to port name
  */
 
-int tipc_forward2name(u32 ref,
-		      struct tipc_name const *name,
-		      u32 domain,
-		      u32 num_sect,
-		      struct iovec const *msg_sect,
-		      struct tipc_portid const *orig,
-		      unsigned int importance)
+static int tipc_forward2name(u32 ref,
+			     struct tipc_name const *name,
+			     u32 domain,
+			     u32 num_sect,
+			     struct iovec const *msg_sect,
+			     struct tipc_portid const *orig,
+			     unsigned int importance)
 {
 	struct port *p_ptr;
 	struct tipc_msg *msg;
@@ -1457,89 +1343,15 @@ int tipc_send2name(u32 ref,
 }
 
 /**
- * tipc_forward_buf2name - forward message buffer to port name
- */
-
-int tipc_forward_buf2name(u32 ref,
-			  struct tipc_name const *name,
-			  u32 domain,
-			  struct sk_buff *buf,
-			  unsigned int dsz,
-			  struct tipc_portid const *orig,
-			  unsigned int importance)
-{
-	struct port *p_ptr;
-	struct tipc_msg *msg;
-	u32 destnode = domain;
-	u32 destport;
-	int res;
-
-	p_ptr = (struct port *)tipc_ref_deref(ref);
-	if (!p_ptr || p_ptr->publ.connected)
-		return -EINVAL;
-
-	msg = &p_ptr->publ.phdr;
-	if (importance <= TIPC_CRITICAL_IMPORTANCE)
-		msg_set_importance(msg, importance);
-	msg_set_type(msg, TIPC_NAMED_MSG);
-	msg_set_orignode(msg, orig->node);
-	msg_set_origport(msg, orig->ref);
-	msg_set_nametype(msg, name->type);
-	msg_set_nameinst(msg, name->instance);
-	msg_set_lookup_scope(msg, tipc_addr_scope(domain));
-	msg_set_hdr_sz(msg, LONG_H_SIZE);
-	msg_set_size(msg, LONG_H_SIZE + dsz);
-	destport = tipc_nametbl_translate(name->type, name->instance, &destnode);
-	msg_set_destnode(msg, destnode);
-	msg_set_destport(msg, destport);
-	msg_dbg(msg, "forw2name ==> ");
-	if (skb_cow(buf, LONG_H_SIZE))
-		return -ENOMEM;
-	skb_push(buf, LONG_H_SIZE);
-	skb_copy_to_linear_data(buf, msg, LONG_H_SIZE);
-	msg_dbg(buf_msg(buf),"PREP:");
-	if (likely(destport)) {
-		p_ptr->sent++;
-		if (destnode == tipc_own_addr)
-			return tipc_port_recv_msg(buf);
-		res = tipc_send_buf_fast(buf, destnode);
-		if (likely(res != -ELINKCONG))
-			return res;
-		if (port_unreliable(p_ptr))
-			return dsz;
-		return -ELINKCONG;
-	}
-	return tipc_reject_msg(buf, TIPC_ERR_NO_NAME);
-}
-
-/**
- * tipc_send_buf2name - send message buffer to port name
- */
-
-int tipc_send_buf2name(u32 ref,
-		       struct tipc_name const *dest,
-		       u32 domain,
-		       struct sk_buff *buf,
-		       unsigned int dsz)
-{
-	struct tipc_portid orig;
-
-	orig.ref = ref;
-	orig.node = tipc_own_addr;
-	return tipc_forward_buf2name(ref, dest, domain, buf, dsz, &orig,
-				     TIPC_PORT_IMPORTANCE);
-}
-
-/**
  * tipc_forward2port - forward message sections to port identity
  */
 
-int tipc_forward2port(u32 ref,
-		      struct tipc_portid const *dest,
-		      unsigned int num_sect,
-		      struct iovec const *msg_sect,
-		      struct tipc_portid const *orig,
-		      unsigned int importance)
+static int tipc_forward2port(u32 ref,
+			     struct tipc_portid const *dest,
+			     unsigned int num_sect,
+			     struct iovec const *msg_sect,
+			     struct tipc_portid const *orig,
+			     unsigned int importance)
 {
 	struct port *p_ptr;
 	struct tipc_msg *msg;
@@ -1591,12 +1403,12 @@ int tipc_send2port(u32 ref,
 /**
  * tipc_forward_buf2port - forward message buffer to port identity
  */
-int tipc_forward_buf2port(u32 ref,
-			  struct tipc_portid const *dest,
-			  struct sk_buff *buf,
-			  unsigned int dsz,
-			  struct tipc_portid const *orig,
-			  unsigned int importance)
+static int tipc_forward_buf2port(u32 ref,
+				 struct tipc_portid const *dest,
+				 struct sk_buff *buf,
+				 unsigned int dsz,
+				 struct tipc_portid const *orig,
+				 unsigned int importance)
 {
 	struct port *p_ptr;
 	struct tipc_msg *msg;
--- a/net/tipc/port.h	2010-10-13 15:48:09.502393058 -0700
+++ b/net/tipc/port.h	2010-10-13 16:14:00.819182858 -0700
@@ -109,8 +109,6 @@ struct port {
 extern spinlock_t tipc_port_list_lock;
 struct port_list;
 
-int tipc_port_recv_sections(struct port *p_ptr, u32 num_sect,
-			    struct iovec const *msg_sect);
 int tipc_port_reject_sections(struct port *p_ptr, struct tipc_msg *hdr,
 			      struct iovec const *msg_sect, u32 num_sect,
 			      int err);
--- a/net/tipc/ref.c	2010-10-13 15:41:13.468391212 -0700
+++ b/net/tipc/ref.c	2010-10-13 16:03:20.908481542 -0700
@@ -282,23 +282,6 @@ void *tipc_ref_lock(u32 ref)
 	return NULL;
 }
 
-/**
- * tipc_ref_unlock - unlock referenced object
- */
-
-void tipc_ref_unlock(u32 ref)
-{
-	if (likely(tipc_ref_table.entries)) {
-		struct reference *entry;
-
-		entry = &tipc_ref_table.entries[ref &
-						tipc_ref_table.index_mask];
-		if (likely((entry->ref == ref) && (entry->object)))
-			spin_unlock_bh(&entry->lock);
-		else
-			err("Attempt to unlock non-existent reference\n");
-	}
-}
 
 /**
  * tipc_ref_deref - return pointer referenced object (without locking it)
--- a/net/tipc/ref.h	2010-10-13 15:47:56.028012866 -0700
+++ b/net/tipc/ref.h	2010-10-13 15:48:04.642977233 -0700
@@ -44,7 +44,6 @@ u32 tipc_ref_acquire(void *object, spinl
 void tipc_ref_discard(u32 ref);
 
 void *tipc_ref_lock(u32 ref);
-void tipc_ref_unlock(u32 ref);
 void *tipc_ref_deref(u32 ref);
 
 #endif
--- a/net/tipc/zone.c	2010-10-13 15:41:49.324083397 -0700
+++ b/net/tipc/zone.c	2010-10-13 15:54:26.329050453 -0700
@@ -160,14 +160,3 @@ u32 tipc_zone_select_router(struct _zone
 	}
 	return 0;
 }
-
-
-u32 tipc_zone_next_node(u32 addr)
-{
-	struct cluster *c_ptr = tipc_cltr_find(addr);
-
-	if (c_ptr)
-		return tipc_cltr_next_node(c_ptr, addr);
-	return 0;
-}
-
--- a/net/tipc/zone.h	2010-10-13 15:48:24.460594837 -0700
+++ b/net/tipc/zone.h	2010-10-13 15:48:33.715482229 -0700
@@ -61,7 +61,6 @@ void tipc_zone_send_external_routes(stru
 struct _zone *tipc_zone_create(u32 addr);
 void tipc_zone_delete(struct _zone *z_ptr);
 void tipc_zone_attach_cluster(struct _zone *z_ptr, struct cluster *c_ptr);
-u32 tipc_zone_next_node(u32 addr);
 
 static inline struct _zone *tipc_zone_find(u32 addr)
 {
--- a/net/tipc/addr.c	2010-10-13 15:33:41.742641197 -0700
+++ b/net/tipc/addr.c	2010-10-13 16:02:50.348172914 -0700
@@ -41,11 +41,6 @@
 #include "cluster.h"
 #include "net.h"
 
-u32 tipc_get_addr(void)
-{
-	return tipc_own_addr;
-}
-
 /**
  * tipc_addr_domain_valid - validates a network domain address
  *
--- a/net/tipc/cluster.h	2010-10-13 16:01:44.276153671 -0700
+++ b/net/tipc/cluster.h	2010-10-13 16:01:56.218711152 -0700
@@ -75,7 +75,7 @@ void tipc_cltr_attach_node(struct cluste
 void tipc_cltr_send_slave_routes(struct cluster *c_ptr, u32 dest);
 void tipc_cltr_broadcast(struct sk_buff *buf);
 int tipc_cltr_init(void);
-u32 tipc_cltr_next_node(struct cluster *c_ptr, u32 addr);
+
 void tipc_cltr_bcast_new_route(struct cluster *c_ptr, u32 dest, u32 lo, u32 hi);
 void tipc_cltr_send_local_routes(struct cluster *c_ptr, u32 dest);
 void tipc_cltr_bcast_lost_route(struct cluster *c_ptr, u32 dest, u32 lo, u32 hi);
--- a/net/tipc/subscr.c	2010-10-13 16:01:08.020532905 -0700
+++ b/net/tipc/subscr.c	2010-10-13 16:03:03.330604772 -0700
@@ -598,12 +598,3 @@ void tipc_subscr_stop(void)
 		topsrv.user_ref = 0;
 	}
 }
-
-
-int tipc_ispublished(struct tipc_name const *name)
-{
-	u32 domain = 0;
-
-	return tipc_nametbl_translate(name->type, name->instance, &domain) != 0;
-}
-
--- a/include/net/tipc/tipc_port.h	2010-10-13 16:05:35.080274751 -0700
+++ b/include/net/tipc/tipc_port.h	2010-10-13 16:06:14.247543618 -0700
@@ -88,8 +88,6 @@ void tipc_acknowledge(u32 port_ref,u32 a
 
 struct tipc_port *tipc_get_port(const u32 ref);
 
-void *tipc_get_handle(const u32 ref);
-
 /*
  * The following routines require that the port be locked on entry
  */

Re: [PATCH net-next] tipc: cleanup function namespace

[Paul Gortmaker]

On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> Do some cleanups of TIPC based on make namespacecheck
>    1. Don't export unused symbols
>    2. Eliminate dead code
>    3. Make functions and variables local
>    4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> 
> Compile tested only.
> This make break out of tree kernel modules that depend on TIPC routines.

Hi Stephen,

When I first started looking at TIPC code, I too came to the
same conclusion as you did and was about to do #1,2,3 -- but
then I was told that the exported symbols were part of an API
and might be in use by folks here and there as per this thread:

http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html

I'm generally the 1st one to agree that the kernel should not
be libc, and that exporting all sorts of functions without a
clearly defined use case so that one can insert all sorts of
brewed up modules is *not* the way to go -- and was asking if
we could phase this API out if nobody was using it:

http://sourceforge.net/mailarchive/message.php?msg_name=29C1DC0826876849BDD9F1C67ABA294308C1FEB6%40ala-mail09.corp.ad.wrs.com

...but apparently there are a couple of API users out there.

I'd like to better understand their use case(es) and what parts
of this API they use, but I haven't got that far yet, since
there are a bunch of other TIPC bugfixes and changes queued on
sourceforge which need cleaning and integration into mainline.

I was thinking one idea would be to wrap them in a TIPC specific
Kconfig (off by default) so that it would at least highlight
this atypical use case for EXPORT_SYMBOL -- which might help
bring these users to the surface so we can learn about their
use cases.  Having it as a Kconfig option would also help in
giving us something to point our finger at for the feature
removal file, if indeed we could find a better way for these
users to get done whatever it is that they are doing.

In any event, I think your #2 (dead code) and #3 (add static)
are items considered dead or candidates for static because
of #1, i.e. tossing the API exports out.

I've already tossed out the explicitly dead code in #if 0
blocks -- Dave just merged that today.   But the #4 from your
list seems to make sense, and we can do that as a standalone
item of course.

Thanks,
Paul.

> 
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> 
> ---
>   include/net/tipc/tipc.h      |   71 -------------
>   include/net/tipc/tipc_port.h |    2
>   net/tipc/addr.c              |    5
>   net/tipc/bcast.c             |   10 +
>   net/tipc/bcast.h             |    3
>   net/tipc/cluster.c           |   21 ---
>   net/tipc/cluster.h           |    2
>   net/tipc/config.c            |    7 +
>   net/tipc/config.h            |    6 -
>   net/tipc/core.c              |   28 +----
>   net/tipc/core.h              |    9 -
>   net/tipc/dbg.c               |   13 +-
>   net/tipc/dbg.h               |    3
>   net/tipc/discover.c          |   16 --
>   net/tipc/discover.h          |    2
>   net/tipc/link.c              |   45 ++++----
>   net/tipc/link.h              |    4
>   net/tipc/msg.c               |    2
>   net/tipc/name_distr.c        |    2
>   net/tipc/node.c              |   19 ---
>   net/tipc/node.h              |    1
>   net/tipc/port.c              |  234 ++++---------------------------------------
>   net/tipc/port.h              |    2
>   net/tipc/ref.c               |   17 ---
>   net/tipc/ref.h               |    1
>   net/tipc/subscr.c            |    9 -
>   net/tipc/zone.c              |   11 --
>   net/tipc/zone.h              |    1
>   28 files changed, 84 insertions(+), 462 deletions(-)
> 
> --- a/include/net/tipc/tipc.h	2010-10-13 15:43:50.025580173 -0700
> +++ b/include/net/tipc/tipc.h	2010-10-13 16:14:33.731207018 -0700
> @@ -50,8 +50,6 @@
>    * TIPC operating mode routines
>    */
> 
> -u32 tipc_get_addr(void);
> -
>   #define TIPC_NOT_RUNNING  0
>   #define TIPC_NODE_MODE    1
>   #define TIPC_NET_MODE     2
> @@ -62,8 +60,6 @@ int tipc_attach(unsigned int *userref, t
> 
>   void tipc_detach(unsigned int userref);
> 
> -int tipc_get_mode(void);
> -
>   /*
>    * TIPC port manipulation routines
>    */
> @@ -153,12 +149,6 @@ int tipc_disconnect(u32 portref);
> 
>   int tipc_shutdown(u32 ref);
> 
> -int tipc_isconnected(u32 portref, int *isconnected);
> -
> -int tipc_peer(u32 portref, struct tipc_portid *peer);
> -
> -int tipc_ref_valid(u32 portref);
> -
>   /*
>    * TIPC messaging routines
>    */
> @@ -170,38 +160,12 @@ int tipc_send(u32 portref,
>   	      unsigned int num_sect,
>   	      struct iovec const *msg_sect);
> 
> -int tipc_send_buf(u32 portref,
> -		  struct sk_buff *buf,
> -		  unsigned int dsz);
> -
>   int tipc_send2name(u32 portref,
>   		   struct tipc_name const *name,
>   		   u32 domain,
>   		   unsigned int num_sect,
>   		   struct iovec const *msg_sect);
> 
> -int tipc_send_buf2name(u32 portref,
> -		       struct tipc_name const *name,
> -		       u32 domain,
> -		       struct sk_buff *buf,
> -		       unsigned int dsz);
> -
> -int tipc_forward2name(u32 portref,
> -		      struct tipc_name const *name,
> -		      u32 domain,
> -		      unsigned int section_count,
> -		      struct iovec const *msg_sect,
> -		      struct tipc_portid const *origin,
> -		      unsigned int importance);
> -
> -int tipc_forward_buf2name(u32 portref,
> -			  struct tipc_name const *name,
> -			  u32 domain,
> -			  struct sk_buff *buf,
> -			  unsigned int dsz,
> -			  struct tipc_portid const *orig,
> -			  unsigned int importance);
> -
>   int tipc_send2port(u32 portref,
>   		   struct tipc_portid const *dest,
>   		   unsigned int num_sect,
> @@ -212,46 +176,11 @@ int tipc_send_buf2port(u32 portref,
>   		       struct sk_buff *buf,
>   		       unsigned int dsz);
> 
> -int tipc_forward2port(u32 portref,
> -		      struct tipc_portid const *dest,
> -		      unsigned int num_sect,
> -		      struct iovec const *msg_sect,
> -		      struct tipc_portid const *origin,
> -		      unsigned int importance);
> -
> -int tipc_forward_buf2port(u32 portref,
> -			  struct tipc_portid const *dest,
> -			  struct sk_buff *buf,
> -			  unsigned int dsz,
> -			  struct tipc_portid const *orig,
> -			  unsigned int importance);
> -
>   int tipc_multicast(u32 portref,
>   		   struct tipc_name_seq const *seq,
>   		   u32 domain,	/* currently unused */
>   		   unsigned int section_count,
>   		   struct iovec const *msg);
> -
> -#if 0
> -int tipc_multicast_buf(u32 portref,
> -		       struct tipc_name_seq const *seq,
> -		       u32 domain,
> -		       void *buf,
> -		       unsigned int size);
> -#endif
> -
> -/*
> - * TIPC subscription routines
> - */
> -
> -int tipc_ispublished(struct tipc_name const *name);
> -
> -/*
> - * Get number of available nodes within specified domain (excluding own node)
> - */
> -
> -unsigned int tipc_available_nodes(const u32 domain);
> -
>   #endif
> 
>   #endif
> --- a/net/tipc/bcast.c	2010-10-13 15:34:09.359325773 -0700
> +++ b/net/tipc/bcast.c	2010-10-13 15:58:43.362005611 -0700
> @@ -121,6 +121,9 @@ static DEFINE_SPINLOCK(bc_lock);
> 
>   const char tipc_bclink_name[] = "broadcast-link";
> 
> +static void tipc_nmap_diff(struct tipc_node_map *nm_a,
> +			   struct tipc_node_map *nm_b,
> +			   struct tipc_node_map *nm_diff);
> 
>   static u32 buf_seqno(struct sk_buff *buf)
>   {
> @@ -287,7 +290,7 @@ static void bclink_send_nack(struct tipc
>   	if (!less(n_ptr->bclink.gap_after, n_ptr->bclink.gap_to))
>   		return;
> 
> -	buf = buf_acquire(INT_H_SIZE);
> +	buf = tipc_buf_acquire(INT_H_SIZE);
>   	if (buf) {
>   		msg = buf_msg(buf);
>   		tipc_msg_init(msg, BCAST_PROTOCOL, STATE_MSG,
> @@ -871,8 +874,9 @@ void tipc_nmap_remove(struct tipc_node_m
>    * @nm_diff: output node map A-B (i.e. nodes of A that are not in B)
>    */
> 
> -void tipc_nmap_diff(struct tipc_node_map *nm_a, struct tipc_node_map *nm_b,
> -				  struct tipc_node_map *nm_diff)
> +static void tipc_nmap_diff(struct tipc_node_map *nm_a,
> +			   struct tipc_node_map *nm_b,
> +			   struct tipc_node_map *nm_diff)
>   {
>   	int stop = ARRAY_SIZE(nm_a->map);
>   	int w;
> --- a/net/tipc/bcast.h	2010-10-13 15:34:39.167747046 -0700
> +++ b/net/tipc/bcast.h	2010-10-13 15:42:10.321560582 -0700
> @@ -84,9 +84,6 @@ static inline int tipc_nmap_equal(struct
>   	return !memcmp(nm_a, nm_b, sizeof(*nm_a));
>   }
> 
> -void tipc_nmap_diff(struct tipc_node_map *nm_a, struct tipc_node_map *nm_b,
> -				  struct tipc_node_map *nm_diff);
> -
>   void tipc_port_list_add(struct port_list *pl_ptr, u32 port);
>   void tipc_port_list_free(struct port_list *pl_ptr);
> 
> --- a/net/tipc/cluster.c	2010-10-13 15:44:43.583143953 -0700
> +++ b/net/tipc/cluster.c	2010-10-13 16:03:22.148331782 -0700
> @@ -113,25 +113,6 @@ void tipc_cltr_delete(struct cluster *c_
>   	kfree(c_ptr);
>   }
> 
> -u32 tipc_cltr_next_node(struct cluster *c_ptr, u32 addr)
> -{
> -	struct tipc_node *n_ptr;
> -	u32 n_num = tipc_node(addr) + 1;
> -
> -	if (!c_ptr)
> -		return addr;
> -	for (; n_num<= c_ptr->highest_node; n_num++) {
> -		n_ptr = c_ptr->nodes[n_num];
> -		if (n_ptr&&  tipc_node_has_active_links(n_ptr))
> -			return n_ptr->addr;
> -	}
> -	for (n_num = 1; n_num<  tipc_node(addr); n_num++) {
> -		n_ptr = c_ptr->nodes[n_num];
> -		if (n_ptr&&  tipc_node_has_active_links(n_ptr))
> -			return n_ptr->addr;
> -	}
> -	return 0;
> -}
> 
>   void tipc_cltr_attach_node(struct cluster *c_ptr, struct tipc_node *n_ptr)
>   {
> @@ -232,7 +213,7 @@ struct tipc_node *tipc_cltr_select_node(
>   static struct sk_buff *tipc_cltr_prepare_routing_msg(u32 data_size, u32 dest)
>   {
>   	u32 size = INT_H_SIZE + data_size;
> -	struct sk_buff *buf = buf_acquire(size);
> +	struct sk_buff *buf = tipc_buf_acquire(size);
>   	struct tipc_msg *msg;
> 
>   	if (buf) {
> --- a/net/tipc/config.c	2010-10-13 15:35:16.359281659 -0700
> +++ b/net/tipc/config.c	2010-10-13 16:07:20.471544155 -0700
> @@ -95,7 +95,7 @@ int tipc_cfg_append_tlv(struct sk_buff *
>   	return 1;
>   }
> 
> -struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value)
> +static struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value)
>   {
>   	struct sk_buff *buf;
>   	__be32 value_net;
> @@ -109,6 +109,11 @@ struct sk_buff *tipc_cfg_reply_unsigned_
>   	return buf;
>   }
> 
> +static struct sk_buff *tipc_cfg_reply_unsigned(u32 value)
> +{
> +	return tipc_cfg_reply_unsigned_type(TIPC_TLV_UNSIGNED, value);
> +}
> +
>   struct sk_buff *tipc_cfg_reply_string_type(u16 tlv_type, char *string)
>   {
>   	struct sk_buff *buf;
> --- a/net/tipc/config.h	2010-10-13 15:43:36.031261839 -0700
> +++ b/net/tipc/config.h	2010-10-13 15:49:35.868009966 -0700
> @@ -45,7 +45,6 @@
>   struct sk_buff *tipc_cfg_reply_alloc(int payload_size);
>   int tipc_cfg_append_tlv(struct sk_buff *buf, int tlv_type,
>   			void *tlv_data, int tlv_data_size);
> -struct sk_buff *tipc_cfg_reply_unsigned_type(u16 tlv_type, u32 value);
>   struct sk_buff *tipc_cfg_reply_string_type(u16 tlv_type, char *string);
> 
>   static inline struct sk_buff *tipc_cfg_reply_none(void)
> @@ -53,11 +52,6 @@ static inline struct sk_buff *tipc_cfg_r
>   	return tipc_cfg_reply_alloc(0);
>   }
> 
> -static inline struct sk_buff *tipc_cfg_reply_unsigned(u32 value)
> -{
> -	return tipc_cfg_reply_unsigned_type(TIPC_TLV_UNSIGNED, value);
> -}
> -
>   static inline struct sk_buff *tipc_cfg_reply_error_string(char *string)
>   {
>   	return tipc_cfg_reply_string_type(TIPC_TLV_ERROR_STRING, string);
> --- a/net/tipc/core.c	2010-10-13 15:32:35.750562985 -0700
> +++ b/net/tipc/core.c	2010-10-13 16:00:58.197719377 -0700
> @@ -96,13 +96,13 @@ int tipc_net_id;
>   int tipc_remote_management;
> 
> 
> -int tipc_get_mode(void)
> +static int tipc_get_mode(void)
>   {
>   	return tipc_mode;
>   }
> 
>   /**
> - * buf_acquire - creates a TIPC message buffer
> + * tipc_buf_acquire - creates a TIPC message buffer
>    * @size: message size (including TIPC header)
>    *
>    * Returns a new buffer with data pointers set to the specified size.
> @@ -111,7 +111,7 @@ int tipc_get_mode(void)
>    *       There may also be unrequested tailroom present at the buffer's end.
>    */
> 
> -struct sk_buff *buf_acquire(u32 size)
> +struct sk_buff *tipc_buf_acquire(u32 size)
>   {
>   	struct sk_buff *skb;
>   	unsigned int buf_size = (BUF_HEADROOM + size + 3)&  ~3u;
> @@ -129,7 +129,7 @@ struct sk_buff *buf_acquire(u32 size)
>    * tipc_core_stop_net - shut down TIPC networking sub-systems
>    */
> 
> -void tipc_core_stop_net(void)
> +static void tipc_core_stop_net(void)
>   {
>   	tipc_eth_media_stop();
>   	tipc_net_stop();
> @@ -154,7 +154,7 @@ int tipc_core_start_net(unsigned long ad
>    * tipc_core_stop - switch TIPC from SINGLE NODE to NOT RUNNING mode
>    */
> 
> -void tipc_core_stop(void)
> +static void tipc_core_stop(void)
>   {
>   	if (tipc_mode != TIPC_NODE_MODE)
>   		return;
> @@ -176,7 +176,7 @@ void tipc_core_stop(void)
>    * tipc_core_start - switch TIPC from NOT RUNNING to SINGLE NODE mode
>    */
> 
> -int tipc_core_start(void)
> +static int tipc_core_start(void)
>   {
>   	int res;
> 
> @@ -246,7 +246,6 @@ MODULE_VERSION(TIPC_MOD_VER);
> 
>   EXPORT_SYMBOL(tipc_attach);
>   EXPORT_SYMBOL(tipc_detach);
> -EXPORT_SYMBOL(tipc_get_addr);
>   EXPORT_SYMBOL(tipc_get_mode);
>   EXPORT_SYMBOL(tipc_createport);
>   EXPORT_SYMBOL(tipc_deleteport);
> @@ -262,23 +261,10 @@ EXPORT_SYMBOL(tipc_withdraw);
>   EXPORT_SYMBOL(tipc_connect2port);
>   EXPORT_SYMBOL(tipc_disconnect);
>   EXPORT_SYMBOL(tipc_shutdown);
> -EXPORT_SYMBOL(tipc_isconnected);
> -EXPORT_SYMBOL(tipc_peer);
> -EXPORT_SYMBOL(tipc_ref_valid);
>   EXPORT_SYMBOL(tipc_send);
> -EXPORT_SYMBOL(tipc_send_buf);
>   EXPORT_SYMBOL(tipc_send2name);
> -EXPORT_SYMBOL(tipc_forward2name);
> -EXPORT_SYMBOL(tipc_send_buf2name);
> -EXPORT_SYMBOL(tipc_forward_buf2name);
>   EXPORT_SYMBOL(tipc_send2port);
> -EXPORT_SYMBOL(tipc_forward2port);
> -EXPORT_SYMBOL(tipc_send_buf2port);
> -EXPORT_SYMBOL(tipc_forward_buf2port);
>   EXPORT_SYMBOL(tipc_multicast);
> -/* EXPORT_SYMBOL(tipc_multicast_buf); not available yet */
> -EXPORT_SYMBOL(tipc_ispublished);
> -EXPORT_SYMBOL(tipc_available_nodes);
> 
>   /* TIPC API for external bearers (see tipc_bearer.h) */
> 
> @@ -295,6 +281,4 @@ EXPORT_SYMBOL(tipc_createport_raw);
>   EXPORT_SYMBOL(tipc_reject_msg);
>   EXPORT_SYMBOL(tipc_send_buf_fast);
>   EXPORT_SYMBOL(tipc_acknowledge);
> -EXPORT_SYMBOL(tipc_get_port);
> -EXPORT_SYMBOL(tipc_get_handle);
> 
> --- a/net/tipc/core.h	2010-10-13 15:36:36.237690119 -0700
> +++ b/net/tipc/core.h	2010-10-13 16:07:56.727164658 -0700
> @@ -83,9 +83,7 @@
>    * Note: TIPC_LOG is configured to echo its output to the system console;
>    *       user-defined buffers can be configured to do the same thing.
>    */
> -
>   extern struct print_buf *const TIPC_NULL;
> -extern struct print_buf *const TIPC_CONS;
>   extern struct print_buf *const TIPC_LOG;
> 
>   void tipc_printf(struct print_buf *, const char *fmt, ...);
> @@ -204,10 +202,7 @@ extern atomic_t tipc_user_count;
>    * Routines available to privileged subsystems
>    */
> 
> -extern int  tipc_core_start(void);
> -extern void tipc_core_stop(void);
> -extern int  tipc_core_start_net(unsigned long addr);
> -extern void tipc_core_stop_net(void);
> +extern int tipc_core_start_net(unsigned long);
>   extern int  tipc_handler_start(void);
>   extern void tipc_handler_stop(void);
>   extern int  tipc_netlink_start(void);
> @@ -328,7 +323,7 @@ static inline struct tipc_msg *buf_msg(s
>   	return (struct tipc_msg *)skb->data;
>   }
> 
> -extern struct sk_buff *buf_acquire(u32 size);
> +extern struct sk_buff *tipc_buf_acquire(u32 size);
> 
>   /**
>    * buf_discard - frees a TIPC message buffer
> --- a/net/tipc/dbg.c	2010-10-13 15:35:34.049157629 -0700
> +++ b/net/tipc/dbg.c	2010-10-13 15:58:43.362005611 -0700
> @@ -52,7 +52,7 @@ static struct print_buf null_buf = { NUL
>   struct print_buf *const TIPC_NULL =&null_buf;
> 
>   static struct print_buf cons_buf = { NULL, 0, NULL, 1 };
> -struct print_buf *const TIPC_CONS =&cons_buf;
> +static struct print_buf *const TIPC_CONS =&cons_buf;
> 
>   static struct print_buf log_buf = { NULL, 0, NULL, 1 };
>   struct print_buf *const TIPC_LOG =&log_buf;
> @@ -76,6 +76,10 @@ struct print_buf *const TIPC_LOG =&log_
>   static char print_string[TIPC_PB_MAX_STR];
>   static DEFINE_SPINLOCK(print_lock);
> 
> +static void tipc_printbuf_reset(struct print_buf *pb);
> +static int  tipc_printbuf_empty(struct print_buf *pb);
> +static void tipc_printbuf_move(struct print_buf *pb_to,
> +			       struct print_buf *pb_from);
> 
>   #define FORMAT(PTR,LEN,FMT) \
>   {\
> @@ -116,7 +120,7 @@ void tipc_printbuf_init(struct print_buf
>    * @pb: pointer to print buffer structure
>    */
> 
> -void tipc_printbuf_reset(struct print_buf *pb)
> +static void tipc_printbuf_reset(struct print_buf *pb)
>   {
>   	if (pb->buf) {
>   		pb->crs = pb->buf;
> @@ -132,7 +136,7 @@ void tipc_printbuf_reset(struct print_bu
>    * Returns non-zero if print buffer is empty.
>    */
> 
> -int tipc_printbuf_empty(struct print_buf *pb)
> +static int tipc_printbuf_empty(struct print_buf *pb)
>   {
>   	return !pb->buf || (pb->crs == pb->buf);
>   }
> @@ -181,7 +185,8 @@ int tipc_printbuf_validate(struct print_
>    * Source print buffer becomes empty if a successful move occurs.
>    */
> 
> -void tipc_printbuf_move(struct print_buf *pb_to, struct print_buf *pb_from)
> +static void tipc_printbuf_move(struct print_buf *pb_to,
> +			       struct print_buf *pb_from)
>   {
>   	int len;
> 
> --- a/net/tipc/dbg.h	2010-10-13 15:37:37.134376966 -0700
> +++ b/net/tipc/dbg.h	2010-10-13 15:42:09.489660536 -0700
> @@ -56,10 +56,7 @@ struct print_buf {
>   #define TIPC_PB_MAX_STR 512	/* max printable string (with trailing NUL) */
> 
>   void tipc_printbuf_init(struct print_buf *pb, char *buf, u32 size);
> -void tipc_printbuf_reset(struct print_buf *pb);
> -int  tipc_printbuf_empty(struct print_buf *pb);
>   int  tipc_printbuf_validate(struct print_buf *pb);
> -void tipc_printbuf_move(struct print_buf *pb_to, struct print_buf *pb_from);
> 
>   int tipc_log_resize(int log_size);
> 
> --- a/net/tipc/discover.c	2010-10-13 15:38:16.357666201 -0700
> +++ b/net/tipc/discover.c	2010-10-13 15:52:14.740906596 -0700
> @@ -68,20 +68,6 @@ struct link_req {
>   	unsigned int timer_intv;
>   };
> 
> -
> -/*
> - * disc_lost_link(): A link has lost contact
> - */
> -
> -void tipc_disc_link_event(u32 addr, char *name, int up)
> -{
> -	if (in_own_cluster(addr))
> -		return;
> -	/*
> -	 * Code for inter cluster link setup here
> -	 */
> -}
> -
>   /**
>    * tipc_disc_init_msg - initialize a link setup message
>    * @type: message type (request or response)
> @@ -95,7 +81,7 @@ static struct sk_buff *tipc_disc_init_ms
>   					  u32 dest_domain,
>   					  struct bearer *b_ptr)
>   {
> -	struct sk_buff *buf = buf_acquire(DSC_H_SIZE);
> +	struct sk_buff *buf = tipc_buf_acquire(DSC_H_SIZE);
>   	struct tipc_msg *msg;
> 
>   	if (buf) {
> --- a/net/tipc/discover.h	2010-10-13 15:47:41.609746110 -0700
> +++ b/net/tipc/discover.h	2010-10-13 15:47:50.604664821 -0700
> @@ -50,6 +50,4 @@ void tipc_disc_stop_link_req(struct link
> 
>   void tipc_disc_recv_msg(struct sk_buff *buf, struct bearer *b_ptr);
> 
> -void tipc_disc_link_event(u32 addr, char *name, int up);
> -
>   #endif
> --- a/net/tipc/link.c	2010-10-13 15:38:30.243998362 -0700
> +++ b/net/tipc/link.c	2010-10-13 15:53:47.961684397 -0700
> @@ -112,6 +112,9 @@ static void link_state_event(struct link
>   static void link_reset_statistics(struct link *l_ptr);
>   static void link_print(struct link *l_ptr, struct print_buf *buf,
>   		       const char *str);
> +static void link_start(struct link *l_ptr);
> +static int link_send_long_buf(struct link *l_ptr, struct sk_buff *buf);
> +
> 
>   /*
>    * Debugging code used by link routines only
> @@ -442,7 +445,7 @@ struct link *tipc_link_create(struct bea
> 
>   	k_init_timer(&l_ptr->timer, (Handler)link_timeout, (unsigned long)l_ptr);
>   	list_add_tail(&l_ptr->link_list,&b_ptr->links);
> -	tipc_k_signal((Handler)tipc_link_start, (unsigned long)l_ptr);
> +	tipc_k_signal((Handler)link_start, (unsigned long)l_ptr);
> 
>   	dbg("tipc_link_create(): tolerance = %u,cont intv = %u, abort_limit = %u\n",
>   	    l_ptr->tolerance, l_ptr->continuity_interval, l_ptr->abort_limit);
> @@ -482,9 +485,9 @@ void tipc_link_delete(struct link *l_ptr
>   	kfree(l_ptr);
>   }
> 
> -void tipc_link_start(struct link *l_ptr)
> +static void link_start(struct link *l_ptr)
>   {
> -	dbg("tipc_link_start %x\n", l_ptr);
> +	dbg("link_start %x\n", l_ptr);
>   	link_state_event(l_ptr, STARTING_EVT);
>   }
> 
> @@ -1000,7 +1003,7 @@ int tipc_link_send_buf(struct link *l_pt
>   	/* Fragmentation needed ? */
> 
>   	if (size>  max_packet)
> -		return tipc_link_send_long_buf(l_ptr, buf);
> +		return link_send_long_buf(l_ptr, buf);
> 
>   	/* Packet can be queued or sent: */
> 
> @@ -1036,7 +1039,7 @@ int tipc_link_send_buf(struct link *l_pt
>   		/* Try creating a new bundle */
> 
>   		if (size<= max_packet * 2 / 3) {
> -			struct sk_buff *bundler = buf_acquire(max_packet);
> +			struct sk_buff *bundler = tipc_buf_acquire(max_packet);
>   			struct tipc_msg bundler_hdr;
> 
>   			if (bundler) {
> @@ -1312,7 +1315,7 @@ again:
> 
>   	/* Prepare header of first fragment: */
> 
> -	buf_chain = buf = buf_acquire(max_pkt);
> +	buf_chain = buf = tipc_buf_acquire(max_pkt);
>   	if (!buf)
>   		return -ENOMEM;
>   	buf->next = NULL;
> @@ -1369,7 +1372,7 @@ error:
>   			msg_set_size(&fragm_hdr, fragm_sz + INT_H_SIZE);
>   			msg_set_fragm_no(&fragm_hdr, ++fragm_no);
>   			prev = buf;
> -			buf = buf_acquire(fragm_sz + INT_H_SIZE);
> +			buf = tipc_buf_acquire(fragm_sz + INT_H_SIZE);
>   			if (!buf)
>   				goto error;
> 
> @@ -2145,7 +2148,7 @@ void tipc_link_send_proto_msg(struct lin
>   	if (tipc_bearer_congested(l_ptr->b_ptr, l_ptr)) {
>   		if (!l_ptr->proto_msg_queue) {
>   			l_ptr->proto_msg_queue =
> -				buf_acquire(sizeof(l_ptr->proto_msg));
> +				tipc_buf_acquire(sizeof(l_ptr->proto_msg));
>   		}
>   		buf = l_ptr->proto_msg_queue;
>   		if (!buf)
> @@ -2159,7 +2162,7 @@ void tipc_link_send_proto_msg(struct lin
> 
>   	msg_dbg(msg, ">>");
> 
> -	buf = buf_acquire(msg_size);
> +	buf = tipc_buf_acquire(msg_size);
>   	if (!buf)
>   		return;
> 
> @@ -2318,10 +2321,10 @@ exit:
>    * tipc_link_tunnel(): Send one message via a link belonging to
>    * another bearer. Owner node is locked.
>    */
> -void tipc_link_tunnel(struct link *l_ptr,
> -		      struct tipc_msg *tunnel_hdr,
> -		      struct tipc_msg  *msg,
> -		      u32 selector)
> +static void tipc_link_tunnel(struct link *l_ptr,
> +			     struct tipc_msg *tunnel_hdr,
> +			     struct tipc_msg  *msg,
> +			     u32 selector)
>   {
>   	struct link *tunnel;
>   	struct sk_buff *buf;
> @@ -2334,7 +2337,7 @@ void tipc_link_tunnel(struct link *l_ptr
>   		return;
>   	}
>   	msg_set_size(tunnel_hdr, length + INT_H_SIZE);
> -	buf = buf_acquire(length + INT_H_SIZE);
> +	buf = tipc_buf_acquire(length + INT_H_SIZE);
>   	if (!buf) {
>   		warn("Link changeover error, "
>   		     "unable to send tunnel msg\n");
> @@ -2380,7 +2383,7 @@ void tipc_link_changeover(struct link *l
>   	if (!l_ptr->first_out) {
>   		struct sk_buff *buf;
> 
> -		buf = buf_acquire(INT_H_SIZE);
> +		buf = tipc_buf_acquire(INT_H_SIZE);
>   		if (buf) {
>   			skb_copy_to_linear_data(buf,&tunnel_hdr, INT_H_SIZE);
>   			msg_set_size(&tunnel_hdr, INT_H_SIZE);
> @@ -2441,7 +2444,7 @@ void tipc_link_send_duplicate(struct lin
>   		msg_set_ack(msg, mod(l_ptr->next_in_no - 1));	/* Update */
>   		msg_set_bcast_ack(msg, l_ptr->owner->bclink.last_in);
>   		msg_set_size(&tunnel_hdr, length + INT_H_SIZE);
> -		outbuf = buf_acquire(length + INT_H_SIZE);
> +		outbuf = tipc_buf_acquire(length + INT_H_SIZE);
>   		if (outbuf == NULL) {
>   			warn("Link changeover error, "
>   			     "unable to send duplicate msg\n");
> @@ -2477,7 +2480,7 @@ static struct sk_buff *buf_extract(struc
>   	u32 size = msg_size(msg);
>   	struct sk_buff *eb;
> 
> -	eb = buf_acquire(size);
> +	eb = tipc_buf_acquire(size);
>   	if (eb)
>   		skb_copy_to_linear_data(eb, msg, size);
>   	return eb;
> @@ -2605,11 +2608,11 @@ void tipc_link_recv_bundle(struct sk_buf
> 
> 
>   /*
> - * tipc_link_send_long_buf: Entry for buffers needing fragmentation.
> + * link_send_long_buf: Entry for buffers needing fragmentation.
>    * The buffer is complete, inclusive total message length.
>    * Returns user data length.
>    */
> -int tipc_link_send_long_buf(struct link *l_ptr, struct sk_buff *buf)
> +static int link_send_long_buf(struct link *l_ptr, struct sk_buff *buf)
>   {
>   	struct tipc_msg *inmsg = buf_msg(buf);
>   	struct tipc_msg fragm_hdr;
> @@ -2648,7 +2651,7 @@ int tipc_link_send_long_buf(struct link
>   			fragm_sz = rest;
>   			msg_set_type(&fragm_hdr, LAST_FRAGMENT);
>   		}
> -		fragm = buf_acquire(fragm_sz + INT_H_SIZE);
> +		fragm = tipc_buf_acquire(fragm_sz + INT_H_SIZE);
>   		if (fragm == NULL) {
>   			warn("Link unable to fragment message\n");
>   			dsz = -ENOMEM;
> @@ -2753,7 +2756,7 @@ int tipc_link_recv_fragment(struct sk_bu
>   			buf_discard(fbuf);
>   			return 0;
>   		}
> -		pbuf = buf_acquire(msg_size(imsg));
> +		pbuf = tipc_buf_acquire(msg_size(imsg));
>   		if (pbuf != NULL) {
>   			pbuf->next = *pending;
>   			*pending = pbuf;
> --- a/net/tipc/link.h	2010-10-13 15:51:49.403953468 -0700
> +++ b/net/tipc/link.h	2010-10-13 15:52:06.877852172 -0700
> @@ -225,7 +225,6 @@ void tipc_link_send_duplicate(struct lin
>   void tipc_link_reset_fragments(struct link *l_ptr);
>   int tipc_link_is_up(struct link *l_ptr);
>   int tipc_link_is_active(struct link *l_ptr);
> -void tipc_link_start(struct link *l_ptr);
>   u32 tipc_link_push_packet(struct link *l_ptr);
>   void tipc_link_stop(struct link *l_ptr);
>   struct sk_buff *tipc_link_cmd_config(const void *req_tlv_area, int req_tlv_space, u16 cmd);
> @@ -239,9 +238,6 @@ int tipc_link_send_sections_fast(struct
>   				 struct iovec const *msg_sect,
>   				 const u32 num_sect,
>   				 u32 destnode);
> -int tipc_link_send_long_buf(struct link *l_ptr, struct sk_buff *buf);
> -void tipc_link_tunnel(struct link *l_ptr, struct tipc_msg *tnl_hdr,
> -		      struct tipc_msg *msg, u32 selector);
>   void tipc_link_recv_bundle(struct sk_buff *buf);
>   int  tipc_link_recv_fragment(struct sk_buff **pending,
>   			     struct sk_buff **fb,
> --- a/net/tipc/msg.c	2010-10-13 15:45:42.760031835 -0700
> +++ b/net/tipc/msg.c	2010-10-13 15:45:52.410871897 -0700
> @@ -112,7 +112,7 @@ int tipc_msg_build(struct tipc_msg *hdr,
>   		return dsz;
>   	}
> 
> -	*buf = buf_acquire(sz);
> +	*buf = tipc_buf_acquire(sz);
>   	if (!(*buf))
>   		return -ENOMEM;
>   	skb_copy_to_linear_data(*buf, hdr, hsz);
> --- a/net/tipc/name_distr.c	2010-10-13 15:45:56.098428684 -0700
> +++ b/net/tipc/name_distr.c	2010-10-13 15:46:05.505298048 -0700
> @@ -98,7 +98,7 @@ static void publ_to_item(struct distr_it
> 
>   static struct sk_buff *named_prepare_buf(u32 type, u32 size, u32 dest)
>   {
> -	struct sk_buff *buf = buf_acquire(LONG_H_SIZE + size);
> +	struct sk_buff *buf = tipc_buf_acquire(LONG_H_SIZE + size);
>   	struct tipc_msg *msg;
> 
>   	if (buf != NULL) {
> --- a/net/tipc/node.c	2010-10-13 15:39:25.165401568 -0700
> +++ b/net/tipc/node.c	2010-10-13 16:03:21.564402314 -0700
> @@ -50,7 +50,8 @@ void node_print(struct print_buf *buf, s
>   static void node_lost_contact(struct tipc_node *n_ptr);
>   static void node_established_contact(struct tipc_node *n_ptr);
> 
> -struct tipc_node *tipc_nodes = NULL;	/* sorted list of nodes within cluster */
> +/* sorted list of nodes within cluster */
> +static struct tipc_node *tipc_nodes = NULL;
> 
>   static DEFINE_SPINLOCK(node_create_lock);
> 
> @@ -587,22 +588,6 @@ void tipc_node_remove_router(struct tipc
>   		node_lost_contact(n_ptr);
>   }
> 
> -u32 tipc_available_nodes(const u32 domain)
> -{
> -	struct tipc_node *n_ptr;
> -	u32 cnt = 0;
> -
> -	read_lock_bh(&tipc_net_lock);
> -	for (n_ptr = tipc_nodes; n_ptr; n_ptr = n_ptr->next) {
> -		if (!tipc_in_scope(domain, n_ptr->addr))
> -			continue;
> -		if (tipc_node_is_up(n_ptr))
> -			cnt++;
> -	}
> -	read_unlock_bh(&tipc_net_lock);
> -	return cnt;
> -}
> -
>   struct sk_buff *tipc_node_get_nodes(const void *req_tlv_area, int req_tlv_space)
>   {
>   	u32 domain;
> --- a/net/tipc/node.h	2010-10-13 15:54:01.616035253 -0700
> +++ b/net/tipc/node.h	2010-10-13 15:54:11.830801530 -0700
> @@ -96,7 +96,6 @@ struct tipc_node {
>   	} bclink;
>   };
> 
> -extern struct tipc_node *tipc_nodes;
>   extern u32 tipc_own_tag;
> 
>   struct tipc_node *tipc_node_create(u32 addr);
> --- a/net/tipc/port.c	2010-10-13 15:40:52.362926772 -0700
> +++ b/net/tipc/port.c	2010-10-13 16:16:41.435779874 -0700
> @@ -293,34 +293,6 @@ int tipc_deleteport(u32 ref)
>   	return 0;
>   }
> 
> -/**
> - * tipc_get_port() - return port associated with 'ref'
> - *
> - * Note: Port is not locked.
> - */
> -
> -struct tipc_port *tipc_get_port(const u32 ref)
> -{
> -	return (struct tipc_port *)tipc_ref_deref(ref);
> -}
> -
> -/**
> - * tipc_get_handle - return user handle associated to port 'ref'
> - */
> -
> -void *tipc_get_handle(const u32 ref)
> -{
> -	struct port *p_ptr;
> -	void * handle;
> -
> -	p_ptr = tipc_port_lock(ref);
> -	if (!p_ptr)
> -		return NULL;
> -	handle = p_ptr->publ.usr_handle;
> -	tipc_port_unlock(p_ptr);
> -	return handle;
> -}
> -
>   static int port_unreliable(struct port *p_ptr)
>   {
>   	return msg_src_droppable(&p_ptr->publ.phdr);
> @@ -392,7 +364,7 @@ static struct sk_buff *port_build_proto_
>   	struct sk_buff *buf;
>   	struct tipc_msg *msg;
> 
> -	buf = buf_acquire(LONG_H_SIZE);
> +	buf = tipc_buf_acquire(LONG_H_SIZE);
>   	if (buf) {
>   		msg = buf_msg(buf);
>   		tipc_msg_init(msg, usr, type, LONG_H_SIZE, destnode);
> @@ -433,7 +405,7 @@ int tipc_reject_msg(struct sk_buff *buf,
>   		hdr_sz = MCAST_H_SIZE;
>   	else
>   		hdr_sz = LONG_H_SIZE;
> -	rbuf = buf_acquire(data_sz + hdr_sz);
> +	rbuf = tipc_buf_acquire(data_sz + hdr_sz);
>   	if (rbuf == NULL) {
>   		buf_discard(buf);
>   		return data_sz;
> @@ -1242,50 +1214,13 @@ int tipc_shutdown(u32 ref)
>   	return tipc_disconnect(ref);
>   }
> 
> -int tipc_isconnected(u32 ref, int *isconnected)
> -{
> -	struct port *p_ptr;
> -
> -	p_ptr = tipc_port_lock(ref);
> -	if (!p_ptr)
> -		return -EINVAL;
> -	*isconnected = p_ptr->publ.connected;
> -	tipc_port_unlock(p_ptr);
> -	return 0;
> -}
> -
> -int tipc_peer(u32 ref, struct tipc_portid *peer)
> -{
> -	struct port *p_ptr;
> -	int res;
> -
> -	p_ptr = tipc_port_lock(ref);
> -	if (!p_ptr)
> -		return -EINVAL;
> -	if (p_ptr->publ.connected) {
> -		peer->ref = port_peerport(p_ptr);
> -		peer->node = port_peernode(p_ptr);
> -		res = 0;
> -	} else
> -		res = -ENOTCONN;
> -	tipc_port_unlock(p_ptr);
> -	return res;
> -}
> -
> -int tipc_ref_valid(u32 ref)
> -{
> -	/* Works irrespective of type */
> -	return !!tipc_ref_deref(ref);
> -}
> -
> -
>   /*
>    *  tipc_port_recv_sections(): Concatenate and deliver sectioned
>    *                        message for this node.
>    */
> 
> -int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
> -		       struct iovec const *msg_sect)
> +static int tipc_port_recv_sections(struct port *sender, unsigned int num_sect,
> +				   struct iovec const *msg_sect)
>   {
>   	struct sk_buff *buf;
>   	int res;
> @@ -1336,65 +1271,16 @@ int tipc_send(u32 ref, unsigned int num_
>   }
> 
>   /**
> - * tipc_send_buf - send message buffer on connection
> - */
> -
> -int tipc_send_buf(u32 ref, struct sk_buff *buf, unsigned int dsz)
> -{
> -	struct port *p_ptr;
> -	struct tipc_msg *msg;
> -	u32 destnode;
> -	u32 hsz;
> -	u32 sz;
> -	u32 res;
> -
> -	p_ptr = tipc_port_deref(ref);
> -	if (!p_ptr || !p_ptr->publ.connected)
> -		return -EINVAL;
> -
> -	msg =&p_ptr->publ.phdr;
> -	hsz = msg_hdr_sz(msg);
> -	sz = hsz + dsz;
> -	msg_set_size(msg, sz);
> -	if (skb_cow(buf, hsz))
> -		return -ENOMEM;
> -
> -	skb_push(buf, hsz);
> -	skb_copy_to_linear_data(buf, msg, hsz);
> -	destnode = msg_destnode(msg);
> -	p_ptr->publ.congested = 1;
> -	if (!tipc_port_congested(p_ptr)) {
> -		if (likely(destnode != tipc_own_addr))
> -			res = tipc_send_buf_fast(buf, destnode);
> -		else {
> -			tipc_port_recv_msg(buf);
> -			res = sz;
> -		}
> -		if (likely(res != -ELINKCONG)) {
> -			port_incr_out_seqno(p_ptr);
> -			p_ptr->sent++;
> -			p_ptr->publ.congested = 0;
> -			return res;
> -		}
> -	}
> -	if (port_unreliable(p_ptr)) {
> -		p_ptr->publ.congested = 0;
> -		return dsz;
> -	}
> -	return -ELINKCONG;
> -}
> -
> -/**
>    * tipc_forward2name - forward message sections to port name
>    */
> 
> -int tipc_forward2name(u32 ref,
> -		      struct tipc_name const *name,
> -		      u32 domain,
> -		      u32 num_sect,
> -		      struct iovec const *msg_sect,
> -		      struct tipc_portid const *orig,
> -		      unsigned int importance)
> +static int tipc_forward2name(u32 ref,
> +			     struct tipc_name const *name,
> +			     u32 domain,
> +			     u32 num_sect,
> +			     struct iovec const *msg_sect,
> +			     struct tipc_portid const *orig,
> +			     unsigned int importance)
>   {
>   	struct port *p_ptr;
>   	struct tipc_msg *msg;
> @@ -1457,89 +1343,15 @@ int tipc_send2name(u32 ref,
>   }
> 
>   /**
> - * tipc_forward_buf2name - forward message buffer to port name
> - */
> -
> -int tipc_forward_buf2name(u32 ref,
> -			  struct tipc_name const *name,
> -			  u32 domain,
> -			  struct sk_buff *buf,
> -			  unsigned int dsz,
> -			  struct tipc_portid const *orig,
> -			  unsigned int importance)
> -{
> -	struct port *p_ptr;
> -	struct tipc_msg *msg;
> -	u32 destnode = domain;
> -	u32 destport;
> -	int res;
> -
> -	p_ptr = (struct port *)tipc_ref_deref(ref);
> -	if (!p_ptr || p_ptr->publ.connected)
> -		return -EINVAL;
> -
> -	msg =&p_ptr->publ.phdr;
> -	if (importance<= TIPC_CRITICAL_IMPORTANCE)
> -		msg_set_importance(msg, importance);
> -	msg_set_type(msg, TIPC_NAMED_MSG);
> -	msg_set_orignode(msg, orig->node);
> -	msg_set_origport(msg, orig->ref);
> -	msg_set_nametype(msg, name->type);
> -	msg_set_nameinst(msg, name->instance);
> -	msg_set_lookup_scope(msg, tipc_addr_scope(domain));
> -	msg_set_hdr_sz(msg, LONG_H_SIZE);
> -	msg_set_size(msg, LONG_H_SIZE + dsz);
> -	destport = tipc_nametbl_translate(name->type, name->instance,&destnode);
> -	msg_set_destnode(msg, destnode);
> -	msg_set_destport(msg, destport);
> -	msg_dbg(msg, "forw2name ==>  ");
> -	if (skb_cow(buf, LONG_H_SIZE))
> -		return -ENOMEM;
> -	skb_push(buf, LONG_H_SIZE);
> -	skb_copy_to_linear_data(buf, msg, LONG_H_SIZE);
> -	msg_dbg(buf_msg(buf),"PREP:");
> -	if (likely(destport)) {
> -		p_ptr->sent++;
> -		if (destnode == tipc_own_addr)
> -			return tipc_port_recv_msg(buf);
> -		res = tipc_send_buf_fast(buf, destnode);
> -		if (likely(res != -ELINKCONG))
> -			return res;
> -		if (port_unreliable(p_ptr))
> -			return dsz;
> -		return -ELINKCONG;
> -	}
> -	return tipc_reject_msg(buf, TIPC_ERR_NO_NAME);
> -}
> -
> -/**
> - * tipc_send_buf2name - send message buffer to port name
> - */
> -
> -int tipc_send_buf2name(u32 ref,
> -		       struct tipc_name const *dest,
> -		       u32 domain,
> -		       struct sk_buff *buf,
> -		       unsigned int dsz)
> -{
> -	struct tipc_portid orig;
> -
> -	orig.ref = ref;
> -	orig.node = tipc_own_addr;
> -	return tipc_forward_buf2name(ref, dest, domain, buf, dsz,&orig,
> -				     TIPC_PORT_IMPORTANCE);
> -}
> -
> -/**
>    * tipc_forward2port - forward message sections to port identity
>    */
> 
> -int tipc_forward2port(u32 ref,
> -		      struct tipc_portid const *dest,
> -		      unsigned int num_sect,
> -		      struct iovec const *msg_sect,
> -		      struct tipc_portid const *orig,
> -		      unsigned int importance)
> +static int tipc_forward2port(u32 ref,
> +			     struct tipc_portid const *dest,
> +			     unsigned int num_sect,
> +			     struct iovec const *msg_sect,
> +			     struct tipc_portid const *orig,
> +			     unsigned int importance)
>   {
>   	struct port *p_ptr;
>   	struct tipc_msg *msg;
> @@ -1591,12 +1403,12 @@ int tipc_send2port(u32 ref,
>   /**
>    * tipc_forward_buf2port - forward message buffer to port identity
>    */
> -int tipc_forward_buf2port(u32 ref,
> -			  struct tipc_portid const *dest,
> -			  struct sk_buff *buf,
> -			  unsigned int dsz,
> -			  struct tipc_portid const *orig,
> -			  unsigned int importance)
> +static int tipc_forward_buf2port(u32 ref,
> +				 struct tipc_portid const *dest,
> +				 struct sk_buff *buf,
> +				 unsigned int dsz,
> +				 struct tipc_portid const *orig,
> +				 unsigned int importance)
>   {
>   	struct port *p_ptr;
>   	struct tipc_msg *msg;
> --- a/net/tipc/port.h	2010-10-13 15:48:09.502393058 -0700
> +++ b/net/tipc/port.h	2010-10-13 16:14:00.819182858 -0700
> @@ -109,8 +109,6 @@ struct port {
>   extern spinlock_t tipc_port_list_lock;
>   struct port_list;
> 
> -int tipc_port_recv_sections(struct port *p_ptr, u32 num_sect,
> -			    struct iovec const *msg_sect);
>   int tipc_port_reject_sections(struct port *p_ptr, struct tipc_msg *hdr,
>   			      struct iovec const *msg_sect, u32 num_sect,
>   			      int err);
> --- a/net/tipc/ref.c	2010-10-13 15:41:13.468391212 -0700
> +++ b/net/tipc/ref.c	2010-10-13 16:03:20.908481542 -0700
> @@ -282,23 +282,6 @@ void *tipc_ref_lock(u32 ref)
>   	return NULL;
>   }
> 
> -/**
> - * tipc_ref_unlock - unlock referenced object
> - */
> -
> -void tipc_ref_unlock(u32 ref)
> -{
> -	if (likely(tipc_ref_table.entries)) {
> -		struct reference *entry;
> -
> -		entry =&tipc_ref_table.entries[ref&
> -						tipc_ref_table.index_mask];
> -		if (likely((entry->ref == ref)&&  (entry->object)))
> -			spin_unlock_bh(&entry->lock);
> -		else
> -			err("Attempt to unlock non-existent reference\n");
> -	}
> -}
> 
>   /**
>    * tipc_ref_deref - return pointer referenced object (without locking it)
> --- a/net/tipc/ref.h	2010-10-13 15:47:56.028012866 -0700
> +++ b/net/tipc/ref.h	2010-10-13 15:48:04.642977233 -0700
> @@ -44,7 +44,6 @@ u32 tipc_ref_acquire(void *object, spinl
>   void tipc_ref_discard(u32 ref);
> 
>   void *tipc_ref_lock(u32 ref);
> -void tipc_ref_unlock(u32 ref);
>   void *tipc_ref_deref(u32 ref);
> 
>   #endif
> --- a/net/tipc/zone.c	2010-10-13 15:41:49.324083397 -0700
> +++ b/net/tipc/zone.c	2010-10-13 15:54:26.329050453 -0700
> @@ -160,14 +160,3 @@ u32 tipc_zone_select_router(struct _zone
>   	}
>   	return 0;
>   }
> -
> -
> -u32 tipc_zone_next_node(u32 addr)
> -{
> -	struct cluster *c_ptr = tipc_cltr_find(addr);
> -
> -	if (c_ptr)
> -		return tipc_cltr_next_node(c_ptr, addr);
> -	return 0;
> -}
> -
> --- a/net/tipc/zone.h	2010-10-13 15:48:24.460594837 -0700
> +++ b/net/tipc/zone.h	2010-10-13 15:48:33.715482229 -0700
> @@ -61,7 +61,6 @@ void tipc_zone_send_external_routes(stru
>   struct _zone *tipc_zone_create(u32 addr);
>   void tipc_zone_delete(struct _zone *z_ptr);
>   void tipc_zone_attach_cluster(struct _zone *z_ptr, struct cluster *c_ptr);
> -u32 tipc_zone_next_node(u32 addr);
> 
>   static inline struct _zone *tipc_zone_find(u32 addr)
>   {
> --- a/net/tipc/addr.c	2010-10-13 15:33:41.742641197 -0700
> +++ b/net/tipc/addr.c	2010-10-13 16:02:50.348172914 -0700
> @@ -41,11 +41,6 @@
>   #include "cluster.h"
>   #include "net.h"
> 
> -u32 tipc_get_addr(void)
> -{
> -	return tipc_own_addr;
> -}
> -
>   /**
>    * tipc_addr_domain_valid - validates a network domain address
>    *
> --- a/net/tipc/cluster.h	2010-10-13 16:01:44.276153671 -0700
> +++ b/net/tipc/cluster.h	2010-10-13 16:01:56.218711152 -0700
> @@ -75,7 +75,7 @@ void tipc_cltr_attach_node(struct cluste
>   void tipc_cltr_send_slave_routes(struct cluster *c_ptr, u32 dest);
>   void tipc_cltr_broadcast(struct sk_buff *buf);
>   int tipc_cltr_init(void);
> -u32 tipc_cltr_next_node(struct cluster *c_ptr, u32 addr);
> +
>   void tipc_cltr_bcast_new_route(struct cluster *c_ptr, u32 dest, u32 lo, u32 hi);
>   void tipc_cltr_send_local_routes(struct cluster *c_ptr, u32 dest);
>   void tipc_cltr_bcast_lost_route(struct cluster *c_ptr, u32 dest, u32 lo, u32 hi);
> --- a/net/tipc/subscr.c	2010-10-13 16:01:08.020532905 -0700
> +++ b/net/tipc/subscr.c	2010-10-13 16:03:03.330604772 -0700
> @@ -598,12 +598,3 @@ void tipc_subscr_stop(void)
>   		topsrv.user_ref = 0;
>   	}
>   }
> -
> -
> -int tipc_ispublished(struct tipc_name const *name)
> -{
> -	u32 domain = 0;
> -
> -	return tipc_nametbl_translate(name->type, name->instance,&domain) != 0;
> -}
> -
> --- a/include/net/tipc/tipc_port.h	2010-10-13 16:05:35.080274751 -0700
> +++ b/include/net/tipc/tipc_port.h	2010-10-13 16:06:14.247543618 -0700
> @@ -88,8 +88,6 @@ void tipc_acknowledge(u32 port_ref,u32 a
> 
>   struct tipc_port *tipc_get_port(const u32 ref);
> 
> -void *tipc_get_handle(const u32 ref);
> -
>   /*
>    * The following routines require that the port be locked on entry
>    */
> 
> 

Re: [PATCH net-next] tipc: cleanup function namespace

[Stephen Hemminger]

On Wed, 13 Oct 2010 20:23:24 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > Do some cleanups of TIPC based on make namespacecheck
> >    1. Don't export unused symbols
> >    2. Eliminate dead code
> >    3. Make functions and variables local
> >    4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > 
> > Compile tested only.
> > This make break out of tree kernel modules that depend on TIPC routines.
> 
> Hi Stephen,
> 
> When I first started looking at TIPC code, I too came to the
> same conclusion as you did and was about to do #1,2,3 -- but
> then I was told that the exported symbols were part of an API
> and might be in use by folks here and there as per this thread:
> 
> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> 
> I'm generally the 1st one to agree that the kernel should not
> be libc, and that exporting all sorts of functions without a
> clearly defined use case so that one can insert all sorts of
> brewed up modules is *not* the way to go -- and was asking if
> we could phase this API out if nobody was using it:
> 
> http://sourceforge.net/mailarchive/message.php?msg_name=29C1DC0826876849BDD9F1C67ABA294308C1FEB6%40ala-mail09.corp.ad.wrs.com
> 
> ...but apparently there are a couple of API users out there.
> 
> I'd like to better understand their use case(es) and what parts
> of this API they use, but I haven't got that far yet, since
> there are a bunch of other TIPC bugfixes and changes queued on
> sourceforge which need cleaning and integration into mainline.
> 
> I was thinking one idea would be to wrap them in a TIPC specific
> Kconfig (off by default) so that it would at least highlight
> this atypical use case for EXPORT_SYMBOL -- which might help
> bring these users to the surface so we can learn about their
> use cases.  Having it as a Kconfig option would also help in
> giving us something to point our finger at for the feature
> removal file, if indeed we could find a better way for these
> users to get done whatever it is that they are doing.
> 
> In any event, I think your #2 (dead code) and #3 (add static)
> are items considered dead or candidates for static because
> of #1, i.e. tossing the API exports out.
> 
> I've already tossed out the explicitly dead code in #if 0
> blocks -- Dave just merged that today.   But the #4 from your
> list seems to make sense, and we can do that as a standalone
> item of course.
> 
> Thanks,
> Paul.
> 
> > 
> > Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> > 

The kernel is does not have or support API's just for out
of tree code. Any code that needs the API should be submitted for
inclusion or risks getting broken at any time!

Re: [PATCH net-next] tipc: cleanup function namespace

[Neil Horman]

On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > Do some cleanups of TIPC based on make namespacecheck
> >    1. Don't export unused symbols
> >    2. Eliminate dead code
> >    3. Make functions and variables local
> >    4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > 
> > Compile tested only.
> > This make break out of tree kernel modules that depend on TIPC routines.
> 
> Hi Stephen,
> 
> When I first started looking at TIPC code, I too came to the
> same conclusion as you did and was about to do #1,2,3 -- but
> then I was told that the exported symbols were part of an API
> and might be in use by folks here and there as per this thread:
> 
> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> 
I think its telling the the argument in the above thread for keeping the API
were that users of it were out there and 'likely to contribute' in the future.
That thread was 3 years ago.  They might be using the API from outside the
kernel tree, but they're not planning on contributing.  As Christoph noted,
they're freeloaders.  The community really doesn't need or want to maintain an
API like that.  If these users are your customers, and removing the API is
unacceptable, perhaps its time to move the entire TIPC module out of tree.

Neil

Re: [PATCH net-next] tipc: cleanup function namespace

[Paul Gortmaker]

On 10-10-13 09:29 PM, Neil Horman wrote:
> On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
>> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
>>> Do some cleanups of TIPC based on make namespacecheck
>>>     1. Don't export unused symbols
>>>     2. Eliminate dead code
>>>     3. Make functions and variables local
>>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
>>>
>>> Compile tested only.
>>> This make break out of tree kernel modules that depend on TIPC routines.
>>
>> Hi Stephen,
>>
>> When I first started looking at TIPC code, I too came to the
>> same conclusion as you did and was about to do #1,2,3 -- but
>> then I was told that the exported symbols were part of an API
>> and might be in use by folks here and there as per this thread:
>>
>> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
>>
> I think its telling the the argument in the above thread for keeping the API
> were that users of it were out there and 'likely to contribute' in the future.
> That thread was 3 years ago.  They might be using the API from outside the
> kernel tree, but they're not planning on contributing.  As Christoph noted,
> they're freeloaders.  The community really doesn't need or want to maintain an
> API like that.  If these users are your customers, and removing the API is
> unacceptable, perhaps its time to move the entire TIPC module out of tree.

As I'd said -- I don't know what the use cases of these API users are,
and so as far as I know they aren't customers either.  For what it is
worth, know that I personally wouldn't try and use a business case to
justify a technically wrong decision here on netdev anyway.

I was just describing the history of the situation, and suggesting
one possible slower approach of phasing it out as a courtesy to those
users, in the same way that the kernel community has extended that
same courtesy with other things in feature-removal.txt

In the end, since Jon is OK with the removal, and is in the process of
communicating this to the API users he is aware of, I sure don't have
any reason to try and save the API.  If folks are good with having it
just go away overnight, then great -- I'll be just as happy to see it
disappear as you and Stephen.  So, a long winded way of saying...

Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Paul.

> 
> Neil
> 

Re: [PATCH net-next] tipc: cleanup function namespace

[Stephen Hemminger]

On Thu, 14 Oct 2010 13:53:21 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> On 10-10-13 09:29 PM, Neil Horman wrote:
> > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> >>> Do some cleanups of TIPC based on make namespacecheck
> >>>     1. Don't export unused symbols
> >>>     2. Eliminate dead code
> >>>     3. Make functions and variables local
> >>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> >>>
> >>> Compile tested only.
> >>> This make break out of tree kernel modules that depend on TIPC routines.
> >>
> >> Hi Stephen,
> >>
> >> When I first started looking at TIPC code, I too came to the
> >> same conclusion as you did and was about to do #1,2,3 -- but
> >> then I was told that the exported symbols were part of an API
> >> and might be in use by folks here and there as per this thread:
> >>
> >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> >>
> > I think its telling the the argument in the above thread for keeping the API
> > were that users of it were out there and 'likely to contribute' in the future.
> > That thread was 3 years ago.  They might be using the API from outside the
> > kernel tree, but they're not planning on contributing.  As Christoph noted,
> > they're freeloaders.  The community really doesn't need or want to maintain an
> > API like that.  If these users are your customers, and removing the API is
> > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> 
> As I'd said -- I don't know what the use cases of these API users are,
> and so as far as I know they aren't customers either.  For what it is
> worth, know that I personally wouldn't try and use a business case to
> justify a technically wrong decision here on netdev anyway.
> 
> I was just describing the history of the situation, and suggesting
> one possible slower approach of phasing it out as a courtesy to those
> users, in the same way that the kernel community has extended that
> same courtesy with other things in feature-removal.txt
> 
> In the end, since Jon is OK with the removal, and is in the process of
> communicating this to the API users he is aware of, I sure don't have
> any reason to try and save the API.  If folks are good with having it
> just go away overnight, then great -- I'll be just as happy to see it
> disappear as you and Stephen.  So, a long winded way of saying...
> 
> Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>

How about putting an entry in feature-removal.txt with a short (6 month)
window?


-- 

RE: [PATCH net-next] tipc: cleanup function namespace

[Jon Maloy]

<...> 
> > justify a technically wrong decision here on netdev anyway.
> > 
> > I was just describing the history of the situation, and 
> suggesting one 
> > possible slower approach of phasing it out as a courtesy to those 
> > users, in the same way that the kernel community has extended that 
> > same courtesy with other things in feature-removal.txt
> > 
> > In the end, since Jon is OK with the removal, and is in the 
> process of 
> > communicating this to the API users he is aware of, I sure 
> don't have 
> > any reason to try and save the API.  If folks are good with 
> having it 
> > just go away overnight, then great -- I'll be just as happy 
> to see it 
> > disappear as you and Stephen.  So, a long winded way of saying...
> > 
> > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> How about putting an entry in feature-removal.txt with a 
> short (6 month) window?

I think that is a very good idea. That would give the users a reasonable time
to find other solutions.

> 
> 
> --
> --
> To unsubscribe from this list: send the line "unsubscribe 
> netdev" in the body of a message to majordomo@vger.kernel.org 
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next] tipc: cleanup function namespace

[Paul Gortmaker]

[Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:

> On Thu, 14 Oct 2010 13:53:21 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > >>> Do some cleanups of TIPC based on make namespacecheck
> > >>>     1. Don't export unused symbols
> > >>>     2. Eliminate dead code
> > >>>     3. Make functions and variables local
> > >>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > >>>
> > >>> Compile tested only.
> > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > >>
> > >> Hi Stephen,
> > >>
> > >> When I first started looking at TIPC code, I too came to the
> > >> same conclusion as you did and was about to do #1,2,3 -- but
> > >> then I was told that the exported symbols were part of an API
> > >> and might be in use by folks here and there as per this thread:
> > >>
> > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > >>
> > > I think its telling the the argument in the above thread for keeping the API
> > > were that users of it were out there and 'likely to contribute' in the future.
> > > That thread was 3 years ago.  They might be using the API from outside the
> > > kernel tree, but they're not planning on contributing.  As Christoph noted,
> > > they're freeloaders.  The community really doesn't need or want to maintain an
> > > API like that.  If these users are your customers, and removing the API is
> > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> > 
> > As I'd said -- I don't know what the use cases of these API users are,
> > and so as far as I know they aren't customers either.  For what it is
> > worth, know that I personally wouldn't try and use a business case to
> > justify a technically wrong decision here on netdev anyway.
> > 
> > I was just describing the history of the situation, and suggesting
> > one possible slower approach of phasing it out as a courtesy to those
> > users, in the same way that the kernel community has extended that
> > same courtesy with other things in feature-removal.txt
> > 
> > In the end, since Jon is OK with the removal, and is in the process of
> > communicating this to the API users he is aware of, I sure don't have
> > any reason to try and save the API.  If folks are good with having it
> > just go away overnight, then great -- I'll be just as happy to see it
> > disappear as you and Stephen.  So, a long winded way of saying...
> > 
> > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> How about putting an entry in feature-removal.txt with a short (6 month)
> window?

I'm fine with that too.

P.

>From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Thu, 14 Oct 2010 17:29:08 -0400
Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011

The native API in the TIPC code exists as a bunch of functions
and exported symbols that aren't actually used by any currently
in-tree kernel code/modules.

Since this code is anomalous to the general guiding principle that
the kernel should not be libc, coverage tools and people intending
to do general cleanups keep finding this code and suggesting that
it be removed.

It seems the right thing to do is to just finally delete it once
and for all, after giving a reasonable window for any existing
users to find alternative solutions to their custom use case(s).

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 Documentation/feature-removal-schedule.txt |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index f456389..1def37e 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -573,3 +573,15 @@ Why:	Hareware scan is the prefer method for iwlwifi devices for
 Who:	Wey-Yi Guy <wey-yi.w.guy@intel.com>
 
 ----------------------------
+
+What:	TIPC: Delete all code and exported symbols specific to Native API
+When:	March 2011
+Why:	The TIPC Native API, as described here:
+	http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
+	is implemented by exporting a bunch of otherwise unused functions
+	for possible modular linkage by custom end-user code.  This goes
+	against the general concept that the kernel should not be libc.
+
+Who:	Paul Gortmaker <paul.gortmaker@windriver.com>
+	
+----------------------------
-- 
1.7.2.1

Re: [PATCH net-next] tipc: cleanup function namespace

[Stephen Hemminger]

On Thu, 14 Oct 2010 17:44:27 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> [Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:
> 
> > On Thu, 14 Oct 2010 13:53:21 -0400
> > Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > 
> > > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > > >>> Do some cleanups of TIPC based on make namespacecheck
> > > >>>     1. Don't export unused symbols
> > > >>>     2. Eliminate dead code
> > > >>>     3. Make functions and variables local
> > > >>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > > >>>
> > > >>> Compile tested only.
> > > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > > >>
> > > >> Hi Stephen,
> > > >>
> > > >> When I first started looking at TIPC code, I too came to the
> > > >> same conclusion as you did and was about to do #1,2,3 -- but
> > > >> then I was told that the exported symbols were part of an API
> > > >> and might be in use by folks here and there as per this thread:
> > > >>
> > > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > > >>
> > > > I think its telling the the argument in the above thread for keeping the API
> > > > were that users of it were out there and 'likely to contribute' in the future.
> > > > That thread was 3 years ago.  They might be using the API from outside the
> > > > kernel tree, but they're not planning on contributing.  As Christoph noted,
> > > > they're freeloaders.  The community really doesn't need or want to maintain an
> > > > API like that.  If these users are your customers, and removing the API is
> > > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> > > 
> > > As I'd said -- I don't know what the use cases of these API users are,
> > > and so as far as I know they aren't customers either.  For what it is
> > > worth, know that I personally wouldn't try and use a business case to
> > > justify a technically wrong decision here on netdev anyway.
> > > 
> > > I was just describing the history of the situation, and suggesting
> > > one possible slower approach of phasing it out as a courtesy to those
> > > users, in the same way that the kernel community has extended that
> > > same courtesy with other things in feature-removal.txt
> > > 
> > > In the end, since Jon is OK with the removal, and is in the process of
> > > communicating this to the API users he is aware of, I sure don't have
> > > any reason to try and save the API.  If folks are good with having it
> > > just go away overnight, then great -- I'll be just as happy to see it
> > > disappear as you and Stephen.  So, a long winded way of saying...
> > > 
> > > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > 
> > How about putting an entry in feature-removal.txt with a short (6 month)
> > window?
> 
> I'm fine with that too.
> 
> P.
> 
> From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Thu, 14 Oct 2010 17:29:08 -0400
> Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011
> 
> The native API in the TIPC code exists as a bunch of functions
> and exported symbols that aren't actually used by any currently
> in-tree kernel code/modules.
> 
> Since this code is anomalous to the general guiding principle that
> the kernel should not be libc, coverage tools and people intending
> to do general cleanups keep finding this code and suggesting that
> it be removed.
> 
> It seems the right thing to do is to just finally delete it once
> and for all, after giving a reasonable window for any existing
> users to find alternative solutions to their custom use case(s).
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  Documentation/feature-removal-schedule.txt |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index f456389..1def37e 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -573,3 +573,15 @@ Why:	Hareware scan is the prefer method for iwlwifi devices for
>  Who:	Wey-Yi Guy <wey-yi.w.guy@intel.com>
>  
>  ----------------------------
> +
> +What:	TIPC: Delete all code and exported symbols specific to Native API
> +When:	March 2011
> +Why:	The TIPC Native API, as described here:
> +	http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
> +	is implemented by exporting a bunch of otherwise unused functions
> +	for possible modular linkage by custom end-user code.  This goes
> +	against the general concept that the kernel should not be libc.
> +
> +Who:	Paul Gortmaker <paul.gortmaker@windriver.com>
> +	
> +----------------------------

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

-- 

Re: [PATCH net-next] tipc: cleanup function namespace

[Neil Horman]

On Thu, Oct 14, 2010 at 03:13:33PM -0700, Stephen Hemminger wrote:
> On Thu, 14 Oct 2010 17:44:27 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> 
> > [Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:
> > 
> > > On Thu, 14 Oct 2010 13:53:21 -0400
> > > Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > > 
> > > > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > > > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > > > >>> Do some cleanups of TIPC based on make namespacecheck
> > > > >>>     1. Don't export unused symbols
> > > > >>>     2. Eliminate dead code
> > > > >>>     3. Make functions and variables local
> > > > >>>     4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > > > >>>
> > > > >>> Compile tested only.
> > > > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > > > >>
> > > > >> Hi Stephen,
> > > > >>
> > > > >> When I first started looking at TIPC code, I too came to the
> > > > >> same conclusion as you did and was about to do #1,2,3 -- but
> > > > >> then I was told that the exported symbols were part of an API
> > > > >> and might be in use by folks here and there as per this thread:
> > > > >>
> > > > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > > > >>
> > > > > I think its telling the the argument in the above thread for keeping the API
> > > > > were that users of it were out there and 'likely to contribute' in the future.
> > > > > That thread was 3 years ago.  They might be using the API from outside the
> > > > > kernel tree, but they're not planning on contributing.  As Christoph noted,
> > > > > they're freeloaders.  The community really doesn't need or want to maintain an
> > > > > API like that.  If these users are your customers, and removing the API is
> > > > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> > > > 
> > > > As I'd said -- I don't know what the use cases of these API users are,
> > > > and so as far as I know they aren't customers either.  For what it is
> > > > worth, know that I personally wouldn't try and use a business case to
> > > > justify a technically wrong decision here on netdev anyway.
> > > > 
> > > > I was just describing the history of the situation, and suggesting
> > > > one possible slower approach of phasing it out as a courtesy to those
> > > > users, in the same way that the kernel community has extended that
> > > > same courtesy with other things in feature-removal.txt
> > > > 
> > > > In the end, since Jon is OK with the removal, and is in the process of
> > > > communicating this to the API users he is aware of, I sure don't have
> > > > any reason to try and save the API.  If folks are good with having it
> > > > just go away overnight, then great -- I'll be just as happy to see it
> > > > disappear as you and Stephen.  So, a long winded way of saying...
> > > > 
> > > > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > 
> > > How about putting an entry in feature-removal.txt with a short (6 month)
> > > window?
> > 
> > I'm fine with that too.
> > 
> > P.
> > 
> > From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Date: Thu, 14 Oct 2010 17:29:08 -0400
> > Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011
> > 
> > The native API in the TIPC code exists as a bunch of functions
> > and exported symbols that aren't actually used by any currently
> > in-tree kernel code/modules.
> > 
> > Since this code is anomalous to the general guiding principle that
> > the kernel should not be libc, coverage tools and people intending
> > to do general cleanups keep finding this code and suggesting that
> > it be removed.
> > 
> > It seems the right thing to do is to just finally delete it once
> > and for all, after giving a reasonable window for any existing
> > users to find alternative solutions to their custom use case(s).
> > 
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > ---
> >  Documentation/feature-removal-schedule.txt |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > index f456389..1def37e 100644
> > --- a/Documentation/feature-removal-schedule.txt
> > +++ b/Documentation/feature-removal-schedule.txt
> > @@ -573,3 +573,15 @@ Why:	Hareware scan is the prefer method for iwlwifi devices for
> >  Who:	Wey-Yi Guy <wey-yi.w.guy@intel.com>
> >  
> >  ----------------------------
> > +
> > +What:	TIPC: Delete all code and exported symbols specific to Native API
> > +When:	March 2011
> > +Why:	The TIPC Native API, as described here:
> > +	http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
> > +	is implemented by exporting a bunch of otherwise unused functions
> > +	for possible modular linkage by custom end-user code.  This goes
> > +	against the general concept that the kernel should not be libc.
> > +
> > +Who:	Paul Gortmaker <paul.gortmaker@windriver.com>
> > +	
> > +----------------------------
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

RE: [PATCH net-next] tipc: cleanup function namespace

[Jon Maloy]

<...>
> > > diff --git a/Documentation/feature-removal-schedule.txt 
> > > b/Documentation/feature-removal-schedule.txt
> > > index f456389..1def37e 100644
> > > --- a/Documentation/feature-removal-schedule.txt
> > > +++ b/Documentation/feature-removal-schedule.txt
> > > @@ -573,3 +573,15 @@ Why:	Hareware scan is the prefer 
> method for iwlwifi devices for
> > >  Who:	Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > >  
> > >  ----------------------------
> > > +
> > > +What:	TIPC: Delete all code and exported symbols 
> specific to Native API
> > > +When:	March 2011
> > > +Why:	The TIPC Native API, as described here:
> > > +	
> http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
> > > +	is implemented by exporting a bunch of otherwise unused 
> functions
> > > +	for possible modular linkage by custom end-user code.  This goes
> > > +	against the general concept that the kernel should not be libc.
> > > +
> > > +Who:	Paul Gortmaker <paul.gortmaker@windriver.com>
> > > +	
> > > +----------------------------
> > 
> > Acked-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>

RE: [PATCH net-next] tipc: cleanup function namespace

[Jon Maloy]

> Do some cleanups of TIPC based on make namespacecheck
>   1. Don't export unused symbols
>   2. Eliminate dead code
>   3. Make functions and variables local
>   4. Rename buf_acquire to tipc_buf_acquire since it is used 
> in several files
> 
> Compile tested only.
> This make break out of tree kernel modules that depend on 
> TIPC routines.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>

Re: [PATCH net-next] tipc: cleanup function namespace

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 13 Oct 2010 16:20:35 -0700

> Do some cleanups of TIPC based on make namespacecheck
>   1. Don't export unused symbols
>   2. Eliminate dead code
>   3. Make functions and variables local
>   4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> 
> Compile tested only.
> This make break out of tree kernel modules that depend on TIPC routines.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

I really think we can and should do this now, so I've
applied this to net-next-2.6

Thanks everyone.

Re: [PATCH net-next 1/5] tipc: Enhance enabling and disabling of Ethernet bearers

[Neil Horman]

On Tue, Oct 12, 2010 at 08:25:54PM -0400, Paul Gortmaker wrote:
> From: Allan Stephens <allan.stephens@windriver.com>
> 
> Use work queue to eliminate release of TIPC's configuration lock when
> registering for device notifications while activating Ethernet media
> support. Optimize code that locates an unused bearer entry when enabling
> an Ethernet bearer. Use work queue to break the association between a
> newly disabled Ethernet bearer and its device driver, thereby allowing
> quicker reuse of the bearer entry.
> 
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

> ---
>  net/tipc/config.c    |   13 +------
>  net/tipc/eth_media.c |   93 ++++++++++++++++++++++++++++++-------------------
>  2 files changed, 58 insertions(+), 48 deletions(-)
> 
> diff --git a/net/tipc/config.c b/net/tipc/config.c
> index 961d1b0..a43450c 100644
> --- a/net/tipc/config.c
> +++ b/net/tipc/config.c
> @@ -332,19 +332,8 @@ static struct sk_buff *cfg_set_own_addr(void)
>  		return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
>  						   " (cannot change node address once assigned)");
>  
> -	/*
> -	 * Must release all spinlocks before calling start_net() because
> -	 * Linux version of TIPC calls eth_media_start() which calls
> -	 * register_netdevice_notifier() which may block!
> -	 *
> -	 * Temporarily releasing the lock should be harmless for non-Linux TIPC,
> -	 * but Linux version of eth_media_start() should really be reworked
> -	 * so that it can be called with spinlocks held.
> -	 */
> -
> -	spin_unlock_bh(&config_lock);
>  	tipc_core_start_net(addr);
> -	spin_lock_bh(&config_lock);
> +
>  	return tipc_cfg_reply_none();
>  }
>  
Why is the work queue needed at all?  Looking at this path, the only entry to it
is from:
genl_rcv_msg
 handle_cmd
  tipc_cfg_do_cmd
   cfg_set_own_addr

That path looks to only be callable from a user space context, so sleeping on
the rtnl lock in when you call register_netdevice_notifier in eth_media_start
should be perfectly fine.  So you should just be able to remove the lock without
any additional work.  Does doing so cause other problems?

> diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
> index 6e988ba..479dbc0 100644
> --- a/net/tipc/eth_media.c
> +++ b/net/tipc/eth_media.c
> @@ -51,17 +51,20 @@
>   * @bearer: ptr to associated "generic" bearer structure
>   * @dev: ptr to associated Ethernet network device
>   * @tipc_packet_type: used in binding TIPC to Ethernet driver
> + * @cleanup: work item used when disabling bearer
>   */
>  
>  struct eth_bearer {
>  	struct tipc_bearer *bearer;
>  	struct net_device *dev;
>  	struct packet_type tipc_packet_type;
> +	struct work_struct cleanup;
>  };
>  
>  static struct eth_bearer eth_bearers[MAX_ETH_BEARERS];
>  static int eth_started = 0;
>  static struct notifier_block notifier;
> +static struct work_struct reg_notifier;
>  
>  /**
>   * send_msg - send a TIPC message out over an Ethernet interface
> @@ -157,22 +160,22 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
>  	if (!dev)
>  		return -ENODEV;
>  
> -	/* Find Ethernet bearer for device (or create one) */
> -
> -	for (;(eb_ptr != stop) && eb_ptr->dev && (eb_ptr->dev != dev); eb_ptr++);
> -	if (eb_ptr == stop)
> -		return -EDQUOT;
> -	if (!eb_ptr->dev) {
> -		eb_ptr->dev = dev;
> -		eb_ptr->tipc_packet_type.type = htons(ETH_P_TIPC);
> -		eb_ptr->tipc_packet_type.dev = dev;
> -		eb_ptr->tipc_packet_type.func = recv_msg;
> -		eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
> -		INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list));
> -		dev_hold(dev);
> -		dev_add_pack(&eb_ptr->tipc_packet_type);
> +	/* Create Ethernet bearer for device */
> +
> +	while (eb_ptr->dev != NULL) {
> +		if (++eb_ptr == stop)
> +			return -EDQUOT;
>  	}
>  
> +	eb_ptr->dev = dev;
> +	eb_ptr->tipc_packet_type.type = __constant_htons(ETH_P_TIPC);
> +	eb_ptr->tipc_packet_type.dev = dev;
> +	eb_ptr->tipc_packet_type.func = recv_msg;
> +	eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
> +	INIT_LIST_HEAD(&eb_ptr->tipc_packet_type.list);
> +	dev_hold(dev);
> +	dev_add_pack(&eb_ptr->tipc_packet_type);
> +
>  	/* Associate TIPC bearer with Ethernet bearer */
>  
>  	eb_ptr->bearer = tb_ptr;
> @@ -185,16 +188,36 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
>  }
>  
>  /**
> + * cleanup_bearer - break association between Ethernet bearer and interface
> + *
> + * This routine must be invoked from a work queue because it can sleep.
> + */
> +
> +static void cleanup_bearer(struct work_struct *work)
> +{
> +	struct eth_bearer *eb_ptr =
> +		container_of(work, struct eth_bearer, cleanup);
> +
> +	dev_remove_pack(&eb_ptr->tipc_packet_type);
> +	dev_put(eb_ptr->dev);
> +	eb_ptr->dev = NULL;
> +}
> +
> +/**
>   * disable_bearer - detach TIPC bearer from an Ethernet interface
>   *
> - * We really should do dev_remove_pack() here, but this function can not be
> - * called at tasklet level. => Use eth_bearer->bearer as a flag to throw away
> - * incoming buffers, & postpone dev_remove_pack() to eth_media_stop() on exit.
> + * Mark Ethernet bearer as inactive so that incoming buffers are thrown away,
> + * then get worker thread to complete bearer cleanup.  (Can't do cleanup
> + * here because cleanup code needs to sleep and caller holds spinlocks.)
>   */
>  
>  static void disable_bearer(struct tipc_bearer *tb_ptr)
>  {
> -	((struct eth_bearer *)tb_ptr->usr_handle)->bearer = NULL;
> +	struct eth_bearer *eb_ptr = (struct eth_bearer *)tb_ptr->usr_handle;
> +
> +	eb_ptr->bearer = NULL;
> +	INIT_WORK(&eb_ptr->cleanup, cleanup_bearer);
If you do really need a workqueue for this, you should move this to someplace
like tipc_enable_bearers, in the restart loop, so you only initalize it once.
That also reduces the chance of a race if multiple processes attempt to disable
this barrier in rapid succession.

> +	schedule_work(&eb_ptr->cleanup);
>  }
>  
>  /**
> @@ -265,6 +288,19 @@ static char *eth_addr2str(struct tipc_media_addr *a, char *str_buf, int str_size
>  }
>  
>  /**
> + * do_registration - register TIPC to receive device notifications
> + *
> + * This routine must be invoked from a work queue because it can sleep.
> + */
> +
> +static void do_registration(struct work_struct *dummy)
> +{
> +	notifier.notifier_call = &recv_notification;
> +	notifier.priority = 0;
> +	register_netdevice_notifier(&notifier);
> +}
> +
> +/**
>   * tipc_eth_media_start - activate Ethernet bearer support
>   *
>   * Register Ethernet media type with TIPC bearer code.  Also register
> @@ -291,11 +327,9 @@ int tipc_eth_media_start(void)
>  	if (res)
>  		return res;
>  
> -	notifier.notifier_call = &recv_notification;
> -	notifier.priority = 0;
> -	res = register_netdevice_notifier(&notifier);
> -	if (!res)
> -		eth_started = 1;
> +	INIT_WORK(&reg_notifier, do_registration);
> +	schedule_work(&reg_notifier);
> +	eth_started = 1;
This is racy.  Even though tipc_cfg_do_cmd holds the config_lock, so only one
context can execute this at a time. two requests that execute this code in rapid
succession can re-initalize the reg_notifier work_struct while its sitting on a
work queue, causing an oops if the workqueue task tries to dequeue this entry
while its getting re-initalized.  You need to move this to someplace like the
module_init routine.

>  	return res;
>  }
>  
> @@ -305,22 +339,9 @@ int tipc_eth_media_start(void)
>  
>  void tipc_eth_media_stop(void)
>  {
> -	int i;
> -
>  	if (!eth_started)
>  		return;
>  
>  	unregister_netdevice_notifier(&notifier);
> -	for (i = 0; i < MAX_ETH_BEARERS ; i++) {
> -		if (eth_bearers[i].bearer) {
> -			eth_bearers[i].bearer->blocked = 1;
Where does this now get set when executing a tipc_eth_media_stop?  Don't you
want to block access to all bearers immediately when you call this?

> -			eth_bearers[i].bearer = NULL;
> -		}
> -		if (eth_bearers[i].dev) {
> -			dev_remove_pack(&eth_bearers[i].tipc_packet_type);
> -			dev_put(eth_bearers[i].dev);
> -		}
> -	}
> -	memset(&eth_bearers, 0, sizeof(eth_bearers));
>  	eth_started = 0;
>  }
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>