Re: [tipc-discussion] [PATCH net-next v3 02/12] tipc: involve tipc_unregister_media routine

[Jon M. <ma...@do...>]

On 09/30/2013 10:38 AM, Ying Xue wrote:
> When a TIPC media is started, it is registered to be linked into a
> global media_list list, however, the media is not removed from the
> list when it is stopped. But there has a special case that all TIPC
> media instances are declared statically, so it cannot cause serious
> problem like memory leak even if media instance is not removed from
> the media_list list when TIPC module is unloaded.
>
> As the media_list will be protected by RCU lock, without unregister
> routine, we just see how a media instance is linked into media_list,
> but cannot find where the media is removed from the list, which is
> an very weird design. Therefore, to make TIPC code more readable the
> tipc_unregister_media() is introduced to remove media instance from
> media_list when TIPC module is stopped.

See previous comment about getting rid of this list and aggregate
tipc_media into eth/ib_bearer.

If you disagree with this, we should as a minimum rename it to
media[] or media_array[], since the term list is very misleading.

///jon
>
> Signed-off-by: Ying Xue <yin...@wi...>
> ---
>  net/tipc/bearer.c    |   17 +++++++++++++++++
>  net/tipc/bearer.h    |    1 +
>  net/tipc/eth_media.c |    1 +
>  net/tipc/ib_media.c  |    1 +
>  4 files changed, 20 insertions(+)
>
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 17f9824..35e65b5 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -109,6 +109,23 @@ exit:
>  	return res;
>  }
>  
> +void tipc_unregister_media(const struct tipc_media *m_ptr)
> +{
> +	struct tipc_media *m;
> +	u32 i;
> +
> +	write_lock_bh(&tipc_net_lock);
> +	for (i = 0; i < media_count; i++) {
> +		m = media_list[i];
> +		if (m_ptr == m) {
> +			media_list[i] = NULL;
> +			media_count--;
> +			break;
> +		}
> +	}
> +	write_unlock_bh(&tipc_net_lock);
> +}
> +
>  /**
>   * tipc_media_addr_printf - record media address in print buffer
>   */
> diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
> index 8695d4a..280c23d 100644
> --- a/net/tipc/bearer.h
> +++ b/net/tipc/bearer.h
> @@ -160,6 +160,7 @@ extern struct tipc_bearer tipc_bearers[];
>   * TIPC routines available to supported media types
>   */
>  int tipc_register_media(struct tipc_media *m_ptr);
> +void tipc_unregister_media(const struct tipc_media *m_ptr);
>  
>  void tipc_recv_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr);
>  
> diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
> index 32d01e8..5ba58b0 100644
> --- a/net/tipc/eth_media.c
> +++ b/net/tipc/eth_media.c
> @@ -371,5 +371,6 @@ void tipc_eth_media_stop(void)
>  
>  	flush_scheduled_work();
>  	unregister_netdevice_notifier(&notifier);
> +	tipc_unregister_media(&eth_media_info);
>  	eth_started = 0;
>  }
> diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
> index 5241a5b..34b9710 100644
> --- a/net/tipc/ib_media.c
> +++ b/net/tipc/ib_media.c
> @@ -367,5 +367,6 @@ void tipc_ib_media_stop(void)
>  
>  	flush_scheduled_work();
>  	unregister_netdevice_notifier(&notifier);
> +	tipc_unregister_media(&ib_media_info);
>  	ib_started = 0;
>  }