RE: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.
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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.
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:[EMAIL PROTECTED]
> 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:
>
> > +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
> <[EMAIL PROTECTED]>
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/32] [TIPC] Multicast link failure now resets all links to "nacking" node.
On Thu, 22 Jun 2006, Per Liden wrote:
> +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
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
