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

From: Paul Gortmaker <[email-masked]>
Date: Fri, 15 Oct 2010 17:31:16 -0400

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