RE: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.

2 messages, 2 authors, 2006-06-26 · open the first message on its own page

RE: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.

From: Stephens, Allan <hidden>
Date: 2006-06-22 20:34:14

Nice observation, James.  As a relative newcomer to the official Linux
kernel development world, I'm impressed that non-TIPC folks are looking
at TIPC changes closely enough to see things like this!

Per, I'll leave it to you to decide if you want to address James's
concern.  But be aware that the link_reset_all() routine is only called
to handle emergency situations when TIPC's multicast link has run into
serious problems and is trying to recover.  Most systems will never
follow this path, so the cost of the unnecessary
local_bh_disable()/local_bh_enable() pairing shouldn't have any real
impact on the overall performance of TIPC.

Regards,
Al Stephens
-----Original Message-----
From: James Morris [mailto:jmorris@namei.org] 
Sent: Thursday, June 22, 2006 2:51 PM
To: Per Liden
Cc: David Miller; netdev@vger.kernel.org; Stephens, Allan
Subject: Re: [PATCH 7/32] [TIPC] Multicast link failure now 
resets all links to "nacking" node.

On Thu, 22 Jun 2006, Per Liden wrote:
quoted
+static void link_reset_all(unsigned long addr) {
+	struct node *n_ptr;
+	char addr_string[16];
+	u32 i;
+
+	read_lock_bh(&tipc_net_lock);
+	n_ptr = tipc_node_find((u32)addr);
+	if (!n_ptr) {
+		read_unlock_bh(&tipc_net_lock);
+		return;	/* node no longer exists */
+	}
+
+	tipc_node_lock(n_ptr);
You already have bh's disabled here, and tipc_node_lock() 
also disables them.

Not sure if it's really worth worrying about but if so, you 
could perhaps implement tipc_node_lock_bh() and tipc_node_lock().



- James
-- 
James Morris
[off-list ref]

RE: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.

From: Per Liden <hidden>
Date: 2006-06-26 11:50:06

On Thu, 22 Jun 2006, Stephens, Allan wrote:

[...]
Per, I'll leave it to you to decide if you want to address James's
concern.  But be aware that the link_reset_all() routine is only called
to handle emergency situations when TIPC's multicast link has run into
serious problems and is trying to recover.  Most systems will never
follow this path, so the cost of the unnecessary
local_bh_disable()/local_bh_enable() pairing shouldn't have any real
impact on the overall performance of TIPC.
I tend to agree with you Allan. Since local_bh_disable()/local_bh_enable() 
supports nesting and this code is only called in extreme situations I 
don't think it's a real problem.

/Per
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help