[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4CB8C824.7090800@windriver.com>
Date: Fri, 15 Oct 2010 17:31:16 -0400
From: Paul Gortmaker <paul.gortmaker@...driver.com>
To: Neil Horman <nhorman@...driver.com>
CC: davem@...emloft.net, netdev@...r.kernel.org,
allan.stephens@...driver.com
Subject: Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic
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
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists