Re: [tipc-discussion] [PATCH net-next v2 02/10] tipc: make bearer variable in media structure synchronous

[Erik H. <eri...@er...>]

On Thu, Aug 08, 2013 at 04:26:16PM +0800, Ying Xue wrote:
> When TIPC media is enabled by bearer, the "bearer" variable in the
> media instance is first initialized with the bearer pointer under
> tipc_net_lock protection; likewise, when media is disabled,
> tipc_net_lock is also first held and its "bearer" variable is then
> set to NULL. But, when the "bearer" variable is accessed in
> recv_msg() and recv_notification(), tipc_net_lock is not taken. As
> the context under which recv_msg() and recv_notification() run is
> different with media enable/disable functions, a stale "bearer"
> might be got by recv_msg() and recv_notification() occasionally.
> Therefore, before the bearer variable is read, tipc_net_lock should
> be held to get rid of the synchronous issue.
> 
> Signed-off-by: Ying Xue <yin...@wi...>
> ---
>  net/tipc/bearer.c    |    2 --
>  net/tipc/eth_media.c |   13 ++++++++++++-
>  net/tipc/ib_media.c  |   13 ++++++++++++-
>  net/tipc/link.c      |    2 --
>  4 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 25045ba..c71da45 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -420,7 +420,6 @@ int tipc_block_bearer(struct tipc_bearer *b_ptr)
>  	struct tipc_link *l_ptr;
>  	struct tipc_link *temp_l_ptr;
>  
> -	read_lock_bh(&tipc_net_lock);
>  	pr_info("Blocking bearer <%s>\n", b_ptr->name);
>  	atomic_set(&b_ptr->blocked, 1);
>  	spin_lock_bh(&b_ptr->lock);
> @@ -432,7 +431,6 @@ int tipc_block_bearer(struct tipc_bearer *b_ptr)
>  		spin_unlock_bh(&n_ptr->lock);
>  	}
>  	spin_unlock_bh(&b_ptr->lock);
> -	read_unlock_bh(&tipc_net_lock);
>  	return 0;
>  }

I'm missing a motivation in the commit message why it's safe do bearer blocking 
outside of tipc_net_lock protection.
By the way, shouldn't this have been done in the last patch from the 
"non controversial" patchset?
tipc: convert 'blocked' flag in struct tipc_bearer to atomic_t

//E